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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 13 Sep 2016 10:05:11 +0800
From:   xiakaixu <xiakaixu@...wei.com>
To:     Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Herbert Xu <herbert@...dor.apana.org.au>
CC:     liushuoran <liushuoran@...wei.com>,
        "David S. Miller" <davem@...emloft.net>,
        "Theodore Ts'o" <tytso@....edu>, Jaegeuk Kim <jaegeuk@...nel.org>,
        "nhorman@...driver.com" <nhorman@...driver.com>,
        "mh1@....fi" <mh1@....fi>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Wangbintian <bintian.wang@...wei.com>,
        Huxinwei <huxinwei@...wei.com>,
        "zhangzhibin (C)" <zhangzhibin.zhang@...wei.com>
Subject: Re: Kernel panic - encryption/decryption failed when open file on
 Arm64

> On 12 September 2016 at 03:16, liushuoran <liushuoran@...wei.com> wrote:
>> Hi Ard,
>>
>> Thanks for the prompt reply. With the patch, there is no panic anymore. But it seems that the encryption/decryption is not successful anyway.
>>
>> As Herbert points out, "If the page allocation fails in blkcipher_walk_next it'll simply switch over to processing it block by block". So does that mean the encryption/decryption should be successful even if the page allocation fails? Please correct me if I misunderstand anything. Thanks in advance.
>>
>
> Perhaps Herbert can explain: I don't see how the 'n = 0' assignment
> results in the correct path being taken; this chunk (blkcipher.c:252)
>
> if (unlikely(n < bsize)) {
>      err = blkcipher_next_slow(desc, walk, bsize, walk->alignmask);
>      goto set_phys_lowmem;
> }
>
> is skipped due to the fact that n == 0 and therefore bsize == 0, and
> so the condition is always false for n == 0
>
> Therefore we end up here (blkcipher.c:257)
>
> walk->nbytes = n;
> if (walk->flags & BLKCIPHER_WALK_COPY) {
>      err = blkcipher_next_copy(walk);
>      goto set_phys_lowmem;
> }
>
> where blkcipher_next_copy() unconditionally calls memcpy() with
> walk->page as destination (even though we ended up here due to the
> fact that walk->page == NULL)
>
> So to me, it seems like we should be taking the blkcipher_next_slow()
> path, which does a kmalloc() and bails with -ENOMEM if that fails.

Hi Ard,

Thanks for such a detailed reply.

According to your reply, I just make a little change to take the
blkcipher_next_slow() path. I test it on arm64 board, there is
no panic anymore and seems the encryption/decryption is successful.

diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c
index 0122bec..5389d40 100644
--- a/crypto/blkcipher.c
+++ b/crypto/blkcipher.c
@@ -240,12 +240,13 @@ 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;
                 }
         }

-       bsize = min(walk->walk_blocksize, n);
+       bsize = walk->walk_blocksize;
         n = scatterwalk_clamp(&walk->in, n);
         n = scatterwalk_clamp(&walk->out, n);

It is just a trial and not sure it makes sense. But anyway, we can do
something here to fix the crash result from the page allocation failure.

What's your opinions, Herbert?

Regards
Kaixu Xia
>
> .
>

Powered by blists - more mailing lists