Skip to content

Commit 6603523

Browse files
committed
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: 4f87ee1 ("crypto: api - Do not zap spawn->alg") Fixes: 73669cc ("crypto: api - Fix race condition in...") Cc: <stable@vger.kernel.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
1 parent eebac67 commit 6603523

File tree

3 files changed

+19
-7
lines changed

3 files changed

+19
-7
lines changed

crypto/algapi.c

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -716,17 +716,27 @@ EXPORT_SYMBOL_GPL(crypto_drop_spawn);
716716

717717
static struct crypto_alg *crypto_spawn_alg(struct crypto_spawn *spawn)
718718
{
719-
struct crypto_alg *alg;
719+
struct crypto_alg *alg = ERR_PTR(-EAGAIN);
720+
struct crypto_alg *target;
721+
bool shoot = false;
720722

721723
down_read(&crypto_alg_sem);
722-
alg = spawn->alg;
723-
if (!spawn->dead && !crypto_mod_get(alg)) {
724-
alg->cra_flags |= CRYPTO_ALG_DYING;
725-
alg = NULL;
724+
if (!spawn->dead) {
725+
alg = spawn->alg;
726+
if (!crypto_mod_get(alg)) {
727+
target = crypto_alg_get(alg);
728+
shoot = true;
729+
alg = ERR_PTR(-EAGAIN);
730+
}
726731
}
727732
up_read(&crypto_alg_sem);
728733

729-
return alg ?: ERR_PTR(-EAGAIN);
734+
if (shoot) {
735+
crypto_shoot_alg(target);
736+
crypto_alg_put(target);
737+
}
738+
739+
return alg;
730740
}
731741

732742
struct crypto_tfm *crypto_spawn_tfm(struct crypto_spawn *spawn, u32 type,

crypto/api.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,12 +333,13 @@ static unsigned int crypto_ctxsize(struct crypto_alg *alg, u32 type, u32 mask)
333333
return len;
334334
}
335335

336-
static void crypto_shoot_alg(struct crypto_alg *alg)
336+
void crypto_shoot_alg(struct crypto_alg *alg)
337337
{
338338
down_write(&crypto_alg_sem);
339339
alg->cra_flags |= CRYPTO_ALG_DYING;
340340
up_write(&crypto_alg_sem);
341341
}
342+
EXPORT_SYMBOL_GPL(crypto_shoot_alg);
342343

343344
struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
344345
u32 mask)

crypto/internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ void crypto_alg_tested(const char *name, int err);
6565
void crypto_remove_spawns(struct crypto_alg *alg, struct list_head *list,
6666
struct crypto_alg *nalg);
6767
void crypto_remove_final(struct list_head *list);
68+
void crypto_shoot_alg(struct crypto_alg *alg);
6869
struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
6970
u32 mask);
7071
void *crypto_create_tfm(struct crypto_alg *alg,

0 commit comments

Comments
 (0)