Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1479183 > unrolled thread
| Started by | Herbert Xu <herbert@gondor.apana.org.au> |
|---|---|
| First post | 2016-09-08 14:50 +0200 |
| Last post | 2016-09-09 13:00 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: Kernel panic - encryption/decryption failed when open file on Arm64 Herbert Xu <herbert@gondor.apana.org.au> - 2016-09-08 14:50 +0200
Re: Kernel panic - encryption/decryption failed when open file on Arm64 xiakaixu <xiakaixu@huawei.com> - 2016-09-09 06:10 +0200
Re: Kernel panic - encryption/decryption failed when open file on Arm64 xiakaixu <xiakaixu@huawei.com> - 2016-09-09 12:30 +0200
Re: Kernel panic - encryption/decryption failed when open file on Arm64 Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-09-09 12:40 +0200
Re: Kernel panic - encryption/decryption failed when open file on Arm64 Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-09-09 13:00 +0200
| From | Herbert Xu <herbert@gondor.apana.org.au> |
|---|---|
| Date | 2016-09-08 14:50 +0200 |
| Subject | Re: Kernel panic - encryption/decryption failed when open file on Arm64 |
| Message-ID | <sf6Wf-5Sk-37@gated-at.bofh.it> |
On Thu, Sep 08, 2016 at 08:38:43PM +0800, xiakaixu wrote: > Hi, > > I am using the encryption/decryption feature on arm64 board and a kernel > panic occurs just when open a file. As the memory size of the board > is limited > and there are some page allocation failures before the panic. > > Seems it is a kernel bug from the call trace log. > > ... > - fscrypt_get_encryption_info > - get_crypt_info.part.1 > - validate_user_key.isra.0 > - derive_aes_gcm_key > - crypto_gcm_decrypt > - ablk_decrypt > - ctr_encrypt > - blkcipher_walk_done > - blkcipher_walk_next > - __get_free_pages > ----------------------------------> page allocation failure > ... > - aes_ctr_encrypt > -----------------------------------------> the input parameter is > NULL pointer as the page allocation failure > > > The input parameter of function aes_ctr_encrypt() comes from the > /struct blkcipher_walk// > //walk/, and this variable /walk /is allocated by the function > __get_free_pages(). So if this > page allocate failed, the input parameter of function > aes_ctr_encrypt() will be NULL. The > panic will occurs if we don't check the input parameter. > > Not sure about this and wish to get your opinions! If the page allocation fails in blkcipher_walk_next it'll simply switch over to processing it block by block. so I don't think the warning is related to the crash. Cheers, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[toc] | [next] | [standalone]
| From | xiakaixu <xiakaixu@huawei.com> |
|---|---|
| Date | 2016-09-09 06:10 +0200 |
| Message-ID | <sflix-6w5-3@gated-at.bofh.it> |
| In reply to | #1479183 |
Sorry for resend this email, just add the linux-crypto@vger.kernel.org
and linux-kernel@vger.kernel.org.
Hi,
Firstly, thanks for your reply!
To reproduce this kernel panic, I test the encryption/decryption feature
on arm64 board with more memory. Just add the following
change:
diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c
index 0122bec..10ef3f4 100644
--- a/crypto/blkcipher.c
+++ b/crypto/blkcipher.c
@@ -240,6 +240,7 @@ static int blkcipher_walk_next(struct
blkcipher_desc *desc,
walk->flags |= BLKCIPHER_WALK_COPY;
if (!walk->page) {
walk->page = (void
*)__get_free_page(GFP_ATOMIC);
+ walk->page = NULL;
if (!walk->page)
n = 0;
}
This change just set the walk->page to NULL manually.
I get the same crash when open file with the above change log. So I
think this NULL page failure is not be handled correctly in current code.
Regards
Kaixu Xia
> On Thu, Sep 08, 2016 at 08:38:43PM +0800, xiakaixu wrote:
>> Hi,
>>
>> I am using the encryption/decryption feature on arm64 board and a kernel
>> panic occurs just when open a file. As the memory size of the board
>> is limited
>> and there are some page allocation failures before the panic.
>>
>> Seems it is a kernel bug from the call trace log.
>>
>> ...
>> - fscrypt_get_encryption_info
>> - get_crypt_info.part.1
>> - validate_user_key.isra.0
>> - derive_aes_gcm_key
>> - crypto_gcm_decrypt
>> - ablk_decrypt
>> - ctr_encrypt
>> - blkcipher_walk_done
>> - blkcipher_walk_next
>> - __get_free_pages
>> ----------------------------------> page allocation failure
>> ...
>> - aes_ctr_encrypt
>> -----------------------------------------> the input parameter is
>> NULL pointer as the page allocation failure
>>
>>
>> The input parameter of function aes_ctr_encrypt() comes from the
>> /struct blkcipher_walk//
>> //walk/, and this variable /walk /is allocated by the function
>> __get_free_pages(). So if this
>> page allocate failed, the input parameter of function
>> aes_ctr_encrypt() will be NULL. The
>> panic will occurs if we don't check the input parameter.
>>
>> Not sure about this and wish to get your opinions!
>
> If the page allocation fails in blkcipher_walk_next it'll simply
> switch over to processing it block by block. so I don't think the
> warning is related to the crash.
>
> Cheers,
>
[toc] | [prev] | [next] | [standalone]
| From | xiakaixu <xiakaixu@huawei.com> |
|---|---|
| Date | 2016-09-09 12:30 +0200 |
| Message-ID | <sfrei-1AG-41@gated-at.bofh.it> |
| In reply to | #1479183 |
Hi,
After a deeply research about this crash, seems it is a specific
bug that only exists in armv8 board. And it occurs in this function
in arch/arm64/crypto/aes-glue.c.
static int ctr_encrypt(struct blkcipher_desc *desc, struct scatterlist *dst,
struct scatterlist *src, unsigned int nbytes)
{
...
desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP;
blkcipher_walk_init(&walk, dst, src, nbytes);
err = blkcipher_walk_virt_block(desc, &walk, AES_BLOCK_SIZE); ---> page allocation failed
...
while ((blocks = (walk.nbytes / AES_BLOCK_SIZE))) { ----> walk.nbytes = 0, and skip this loop
aes_ctr_encrypt(walk.dst.virt.addr, walk.src.virt.addr,
(u8 *)ctx->key_enc, rounds, blocks, walk.iv,
first);
...
err = blkcipher_walk_done(desc, &walk,
walk.nbytes % AES_BLOCK_SIZE);
}
if (nbytes) { ----> enter this if() statement
u8 *tdst = walk.dst.virt.addr + blocks * AES_BLOCK_SIZE;
u8 *tsrc = walk.src.virt.addr + blocks * AES_BLOCK_SIZE;
...
aes_ctr_encrypt(tail, tsrc, (u8 *)ctx->key_enc, rounds, ----> the the sencond input parameter is NULL, so crash...
blocks, walk.iv, first);
...
}
...
}
If the page allocation failed in the function blkcipher_walk_virt_block(),
the variable walk.nbytes = 0, so it will skip the while() loop and enter
the if(nbytes) statment. But here the varibale tsrc is NULL and it is also
the sencond input parameter of the function aes_ctr_encrypt()... Kernel Panic...
I have also researched the similar function in other architectures, and
there if(walk.nbytes) is used, not this if(nbytes) statement in the armv8.
so I think this armv8 function ctr_encrypt() should deal with the page
allocation failed situation.
Regards
Kaixu Xia
> On Thu, Sep 08, 2016 at 08:38:43PM +0800, xiakaixu wrote:
>> Hi,
>>
>> I am using the encryption/decryption feature on arm64 board and a kernel
>> panic occurs just when open a file. As the memory size of the board
>> is limited
>> and there are some page allocation failures before the panic.
>>
>> Seems it is a kernel bug from the call trace log.
>>
>> ...
>> - fscrypt_get_encryption_info
>> - get_crypt_info.part.1
>> - validate_user_key.isra.0
>> - derive_aes_gcm_key
>> - crypto_gcm_decrypt
>> - ablk_decrypt
>> - ctr_encrypt
>> - blkcipher_walk_done
>> - blkcipher_walk_next
>> - __get_free_pages
>> ----------------------------------> page allocation failure
>> ...
>> - aes_ctr_encrypt
>> -----------------------------------------> the input parameter is
>> NULL pointer as the page allocation failure
>>
>>
>> The input parameter of function aes_ctr_encrypt() comes from the
>> /struct blkcipher_walk//
>> //walk/, and this variable /walk /is allocated by the function
>> __get_free_pages(). So if this
>> page allocate failed, the input parameter of function
>> aes_ctr_encrypt() will be NULL. The
>> panic will occurs if we don't check the input parameter.
>>
>> Not sure about this and wish to get your opinions!
>
> If the page allocation fails in blkcipher_walk_next it'll simply
> switch over to processing it block by block. so I don't think the
> warning is related to the crash.
>
> Cheers,
>
[toc] | [prev] | [next] | [standalone]
| From | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| Date | 2016-09-09 12:40 +0200 |
| Subject | Re: Kernel panic - encryption/decryption failed when open file on Arm64 |
| Message-ID | <sfrnY-1F2-3@gated-at.bofh.it> |
| In reply to | #1479857 |
On 9 September 2016 at 11:19, xiakaixu <xiakaixu@huawei.com> wrote:
> Hi,
>
> After a deeply research about this crash, seems it is a specific
> bug that only exists in armv8 board. And it occurs in this function
> in arch/arm64/crypto/aes-glue.c.
>
> static int ctr_encrypt(struct blkcipher_desc *desc, struct scatterlist *dst,
> struct scatterlist *src, unsigned int nbytes)
> {
> ...
>
> desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP;
> blkcipher_walk_init(&walk, dst, src, nbytes);
> err = blkcipher_walk_virt_block(desc, &walk, AES_BLOCK_SIZE); --->
> page allocation failed
>
> ...
>
> while ((blocks = (walk.nbytes / AES_BLOCK_SIZE))) { ---->
> walk.nbytes = 0, and skip this loop
> aes_ctr_encrypt(walk.dst.virt.addr, walk.src.virt.addr,
> (u8 *)ctx->key_enc, rounds, blocks, walk.iv,
> first);
> ...
> err = blkcipher_walk_done(desc, &walk,
> walk.nbytes % AES_BLOCK_SIZE);
> }
> if (nbytes) { ---->
> enter this if() statement
> u8 *tdst = walk.dst.virt.addr + blocks * AES_BLOCK_SIZE;
> u8 *tsrc = walk.src.virt.addr + blocks * AES_BLOCK_SIZE;
> ...
>
> aes_ctr_encrypt(tail, tsrc, (u8 *)ctx->key_enc, rounds,
> ----> the the sencond input parameter is NULL, so crash...
> blocks, walk.iv, first);
> ...
> }
> ...
> }
>
>
> If the page allocation failed in the function blkcipher_walk_virt_block(),
> the variable walk.nbytes = 0, so it will skip the while() loop and enter
> the if(nbytes) statment. But here the varibale tsrc is NULL and it is also
> the sencond input parameter of the function aes_ctr_encrypt()... Kernel
> Panic...
>
> I have also researched the similar function in other architectures, and
> there if(walk.nbytes) is used, not this if(nbytes) statement in the armv8.
> so I think this armv8 function ctr_encrypt() should deal with the page
> allocation failed situation.
>
OK, thanks for the report, and for the analysis. I will investigate,
and propose a fix
Thanks,
Ard.
[toc] | [prev] | [next] | [standalone]
| From | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| Date | 2016-09-09 13:00 +0200 |
| Subject | Re: Kernel panic - encryption/decryption failed when open file on Arm64 |
| Message-ID | <sfrHj-1Mf-5@gated-at.bofh.it> |
| In reply to | #1479861 |
On 9 September 2016 at 11:31, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 9 September 2016 at 11:19, xiakaixu <xiakaixu@huawei.com> wrote:
>> Hi,
>>
>> After a deeply research about this crash, seems it is a specific
>> bug that only exists in armv8 board. And it occurs in this function
>> in arch/arm64/crypto/aes-glue.c.
>>
>> static int ctr_encrypt(struct blkcipher_desc *desc, struct scatterlist *dst,
>> struct scatterlist *src, unsigned int nbytes)
>> {
>> ...
>>
>> desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP;
>> blkcipher_walk_init(&walk, dst, src, nbytes);
>> err = blkcipher_walk_virt_block(desc, &walk, AES_BLOCK_SIZE); --->
>> page allocation failed
>>
>> ...
>>
>> while ((blocks = (walk.nbytes / AES_BLOCK_SIZE))) { ---->
>> walk.nbytes = 0, and skip this loop
>> aes_ctr_encrypt(walk.dst.virt.addr, walk.src.virt.addr,
>> (u8 *)ctx->key_enc, rounds, blocks, walk.iv,
>> first);
>> ...
>> err = blkcipher_walk_done(desc, &walk,
>> walk.nbytes % AES_BLOCK_SIZE);
>> }
>> if (nbytes) { ---->
>> enter this if() statement
>> u8 *tdst = walk.dst.virt.addr + blocks * AES_BLOCK_SIZE;
>> u8 *tsrc = walk.src.virt.addr + blocks * AES_BLOCK_SIZE;
>> ...
>>
>> aes_ctr_encrypt(tail, tsrc, (u8 *)ctx->key_enc, rounds,
>> ----> the the sencond input parameter is NULL, so crash...
>> blocks, walk.iv, first);
>> ...
>> }
>> ...
>> }
>>
>>
>> If the page allocation failed in the function blkcipher_walk_virt_block(),
>> the variable walk.nbytes = 0, so it will skip the while() loop and enter
>> the if(nbytes) statment. But here the varibale tsrc is NULL and it is also
>> the sencond input parameter of the function aes_ctr_encrypt()... Kernel
>> Panic...
>>
>> I have also researched the similar function in other architectures, and
>> there if(walk.nbytes) is used, not this if(nbytes) statement in the armv8.
>> so I think this armv8 function ctr_encrypt() should deal with the page
>> allocation failed situation.
>>
Does this solve your problem?
diff --git a/arch/arm64/crypto/aes-glue.c b/arch/arm64/crypto/aes-glue.c
index 5c888049d061..6b2aa0fd6cd0 100644
--- a/arch/arm64/crypto/aes-glue.c
+++ b/arch/arm64/crypto/aes-glue.c
@@ -216,7 +216,7 @@ static int ctr_encrypt(struct blkcipher_desc
*desc, struct scatterlist *dst,
err = blkcipher_walk_done(desc, &walk,
walk.nbytes % AES_BLOCK_SIZE);
}
- if (nbytes) {
+ if (walk.nbytes % AES_BLOCK_SIZE) {
u8 *tdst = walk.dst.virt.addr + blocks * AES_BLOCK_SIZE;
u8 *tsrc = walk.src.virt.addr + blocks * AES_BLOCK_SIZE;
u8 __aligned(8) tail[AES_BLOCK_SIZE];
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web