linux-kernel - Re: Kernel panic - encryption/decryption failed when open file on Arm64

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 9 Sep 2016 11:56:33 +0100
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     xiakaixu <xiakaixu@...wei.com>
Cc:     Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        "Theodore Ts'o" <tytso@....edu>, Jaegeuk Kim <jaegeuk@...nel.org>,
        nhorman@...driver.com, mh1@....fi,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Bintian <bintian.wang@...wei.com>, liushuoran@...wei.com,
        Huxinwei <huxinwei@...wei.com>, zhangzhibin.zhang@...wei.com
Subject: Re: Kernel panic - encryption/decryption failed when open file on Arm64

On 9 September 2016 at 11:31, Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote:
> On 9 September 2016 at 11:19, xiakaixu <xiakaixu@...wei.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];