From: Herbert Xu <herb...@gondor.apana.org.au>

commit 6603523bf5e432c7c8490fb500793bb15d4e5f61 upstream.

There are two problems in crypto_spawn_alg.  First of all it may
return spawn->alg even if spawn->dead is set.  This results in a
double-free as detected by syzbot.

Secondly the setting of the DYING flag is racy because we hold
the read-lock instead of the write-lock.  We should instead call
crypto_shoot_alg in a safe manner by gaining a refcount, dropping
the lock, and then releasing the refcount.

This patch fixes both problems.

Reported-by: syzbot+fc0674cde00b66844...@syzkaller.appspotmail.com
Fixes: 4f87ee118d16 ("crypto: api - Do not zap spawn->alg")
Fixes: 73669cc55646 ("crypto: api - Fix race condition in...")
Cc: <sta...@vger.kernel.org>
Signed-off-by: Herbert Xu <herb...@gondor.apana.org.au>
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>

---
 crypto/algapi.c   |   22 ++++++++++++++++------
 crypto/api.c      |    3 ++-
 crypto/internal.h |    1 +
 3 files changed, 19 insertions(+), 7 deletions(-)

--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -716,17 +716,27 @@ EXPORT_SYMBOL_GPL(crypto_drop_spawn);
 
 static struct crypto_alg *crypto_spawn_alg(struct crypto_spawn *spawn)
 {
-       struct crypto_alg *alg;
+       struct crypto_alg *alg = ERR_PTR(-EAGAIN);
+       struct crypto_alg *target;
+       bool shoot = false;
 
        down_read(&crypto_alg_sem);
-       alg = spawn->alg;
-       if (!spawn->dead && !crypto_mod_get(alg)) {
-               alg->cra_flags |= CRYPTO_ALG_DYING;
-               alg = NULL;
+       if (!spawn->dead) {
+               alg = spawn->alg;
+               if (!crypto_mod_get(alg)) {
+                       target = crypto_alg_get(alg);
+                       shoot = true;
+                       alg = ERR_PTR(-EAGAIN);
+               }
        }
        up_read(&crypto_alg_sem);
 
-       return alg ?: ERR_PTR(-EAGAIN);
+       if (shoot) {
+               crypto_shoot_alg(target);
+               crypto_alg_put(target);
+       }
+
+       return alg;
 }
 
 struct crypto_tfm *crypto_spawn_tfm(struct crypto_spawn *spawn, u32 type,
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -333,12 +333,13 @@ static unsigned int crypto_ctxsize(struc
        return len;
 }
 
-static void crypto_shoot_alg(struct crypto_alg *alg)
+void crypto_shoot_alg(struct crypto_alg *alg)
 {
        down_write(&crypto_alg_sem);
        alg->cra_flags |= CRYPTO_ALG_DYING;
        up_write(&crypto_alg_sem);
 }
+EXPORT_SYMBOL_GPL(crypto_shoot_alg);
 
 struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
                                      u32 mask)
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -65,6 +65,7 @@ void crypto_alg_tested(const char *name,
 void crypto_remove_spawns(struct crypto_alg *alg, struct list_head *list,
                          struct crypto_alg *nalg);
 void crypto_remove_final(struct list_head *list);
+void crypto_shoot_alg(struct crypto_alg *alg);
 struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
                                      u32 mask);
 void *crypto_create_tfm(struct crypto_alg *alg,


Reply via email to