linux-kernel - [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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 21 Oct 2021 14:30:28 -0400
From:   Daniel Jordan <daniel.m.jordan@...cle.com>
To:     Herbert Xu <herbert@...dor.apana.org.au>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        Daniel Jordan <daniel.m.jordan@...cle.com>
Cc:     linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] crypto: pcrypt - Delay write to padata->info

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(-)

diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
index d569c7ed6c80..9d10b846ccf7 100644
--- a/crypto/pcrypt.c
+++ b/crypto/pcrypt.c
@@ -78,12 +78,14 @@ static void pcrypt_aead_enc(struct padata_priv *padata)
 {
 	struct pcrypt_request *preq = pcrypt_padata_request(padata);
 	struct aead_request *req = pcrypt_request_ctx(preq);
+	int ret;

-	padata->info = crypto_aead_encrypt(req);
+	ret = crypto_aead_encrypt(req);

-	if (padata->info == -EINPROGRESS)
+	if (ret == -EINPROGRESS)
 		return;

+	padata->info = ret;
 	padata_do_serial(padata);
 }

@@ -123,12 +125,14 @@ static void pcrypt_aead_dec(struct padata_priv *padata)
 {
 	struct pcrypt_request *preq = pcrypt_padata_request(padata);
 	struct aead_request *req = pcrypt_request_ctx(preq);
+	int ret;

-	padata->info = crypto_aead_decrypt(req);
+	ret = crypto_aead_decrypt(req);

-	if (padata->info == -EINPROGRESS)
+	if (ret == -EINPROGRESS)
 		return;

+	padata->info = ret;
 	padata_do_serial(padata);
 }

-- 
2.27.0