linux-kernel - Re: [PATCH] crypto: pcrypt - Delay write to padata->info

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] [day] [month] [year] [list]

Message-ID: <20211029131204.GE12278@gondor.apana.org.au>
Date:   Fri, 29 Oct 2021 21:12:04 +0800
From:   Herbert Xu <herbert@...dor.apana.org.au>
To:     Daniel Jordan <daniel.m.jordan@...cle.com>
Cc:     Steffen Klassert <steffen.klassert@...unet.com>,
        linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] crypto: pcrypt - Delay write to padata->info

On Thu, Oct 21, 2021 at 02:30:28PM -0400, Daniel Jordan wrote:
> These three events can race when pcrypt is used multiple times in a
> template ("pcrypt(pcrypt(...))"):
> 
>   1.  [taskA] The caller makes the crypto request via crypto_aead_encrypt()
>   2.  [kworkerB] padata serializes the inner pcrypt request
>   3.  [kworkerC] padata serializes the outer pcrypt request
> 
> 3 might finish before the call to crypto_aead_encrypt() returns in 1,
> resulting in two possible issues.
> 
> First, a use-after-free of the crypto request's memory when, for
> example, taskA writes to the outer pcrypt request's padata->info in
> pcrypt_aead_enc() after kworkerC completes the request.
> 
> Second, the outer pcrypt request overwrites the inner pcrypt request's
> return code with -EINPROGRESS, making a successful request appear to
> fail.  For instance, kworkerB writes the outer pcrypt request's
> padata->info in pcrypt_aead_done() and then taskA overwrites it
> in pcrypt_aead_enc().
> 
> Avoid both situations by delaying the write of padata->info until after
> the inner crypto request's return code is checked.  This prevents the
> use-after-free by not touching the crypto request's memory after the
> next-inner crypto request is made, and stops padata->info from being
> overwritten.
> 
> Fixes: 5068c7a883d16 ("crypto: pcrypt - Add pcrypt crypto parallelization wrapper")
> Reported-by: syzbot+b187b77c8474f9648fae@...kaller.appspotmail.com
> Signed-off-by: Daniel Jordan <daniel.m.jordan@...cle.com>
> ---
> 
> So, pcrypt(pcrypt(...)) doesn't make all that much sense, but apparently
> there's not an easy way to prevent it [1], so I'm going with the minimal
> fix.
> 
> [1] https://lkml.kernel.org/20171230083744.vuclnbs677tj7pi2@gauss3.secunet.de/
> 
>  crypto/pcrypt.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt