crypto: api - Fix use-after-free and race in crypto_spawn_alg

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+fc0674cde00b66844470@syzkaller.appspotmail.com
Fixes: 4f87ee118d16 ("crypto: api - Do not zap spawn->alg")
Fixes: 73669cc55646 ("crypto: api - Fix race condition in...")
Cc: <stable@vger.kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/algapi.c b/crypto/algapi.c
index 849254d..f1e6cca 100644 (file)
--- 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,

diff --git a/crypto/api.c b/crypto/api.c
index 7d71a9b..edcf690 100644 (file)
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -333,12 +333,13 @@ static unsigned int crypto_ctxsize(struct crypto_alg *alg, u32 type, u32 mask)
        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)

diff --git a/crypto/internal.h b/crypto/internal.h
index d5ebc60..ff06a3b 100644 (file)
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -65,6 +65,7 @@ void crypto_alg_tested(const char *name, int err);
 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,