[PATCH 5.6 37/43] crypto: api - Fix use-after-free and race in crypto_spawn_alg

From: Greg Kroah-Hartman
Date: Fri Jun 05 2020 - 10:17:50 EST


From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

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+fc0674cde00b66844470@xxxxxxxxxxxxxxxxxxxxxxxxx
Fixes: 4f87ee118d16 ("crypto: api - Do not zap spawn->alg")
Fixes: 73669cc55646 ("crypto: api - Fix race condition in...")
Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

---
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,