diff mbox series

mm: zswap: fix crypto_free_acomp deadlock in zswap_cpu_comp_dead

Message ID Z72FJnbA39zWh4zS@gondor.apana.org.au
State New
Headers show
Series mm: zswap: fix crypto_free_acomp deadlock in zswap_cpu_comp_dead | expand

Commit Message

Herbert Xu Feb. 25, 2025, 8:53 a.m. UTC 
On Mon, Feb 24, 2025 at 01:53:21PM -0800, syzbot wrote:
> 
> syzbot found the following issue on:
> 
> HEAD commit:    e9a8cac0bf89 Merge tag 'v6.14-rc3-smb3-client-fixes' of gi..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=17b667f8580000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=61cbf5ac8a063ad4
> dashboard link: https://syzkaller.appspot.com/bug?extid=1a517ccfcbc6a7ab0f82
> compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> 
> Unfortunately, I don't have any reproducer for this issue yet.
> 
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/8441f1b50402/disk-e9a8cac0.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/65b1f8d2f790/vmlinux-e9a8cac0.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/1d6f6d8c3d6b/bzImage-e9a8cac0.xz

---8<---
Call crypto_free_acomp outside of the mutex in zswap_cpu_comp_dead
as otherwise this could dead-lock as the allocation path may lead
back into zswap while holding the same lock.  Zap the pointers to
acomp and buffer after freeing.

Also move the NULL check on acomp_ctx so that it takes place before
the mutex dereference.

Fixes: 12dcb0ef5406 ("mm: zswap: properly synchronize freeing resources during CPU hotunplug")
Reported-by: syzbot+1a517ccfcbc6a7ab0f82@syzkaller.appspotmail.com
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff mbox series

Patch

diff --git a/mm/zswap.c b/mm/zswap.c
index 6504174fbc6a..24d36266a791 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -881,18 +881,23 @@  static int zswap_cpu_comp_dead(unsigned int cpu, struct hlist_node *node)
 {
 	struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
 	struct crypto_acomp_ctx *acomp_ctx = per_cpu_ptr(pool->acomp_ctx, cpu);
+	struct crypto_acomp *acomp = NULL;
+
+	if (IS_ERR_OR_NULL(acomp_ctx))
+		return 0;
 
 	mutex_lock(&acomp_ctx->mutex);
-	if (!IS_ERR_OR_NULL(acomp_ctx)) {
-		if (!IS_ERR_OR_NULL(acomp_ctx->req))
-			acomp_request_free(acomp_ctx->req);
-		acomp_ctx->req = NULL;
-		if (!IS_ERR_OR_NULL(acomp_ctx->acomp))
-			crypto_free_acomp(acomp_ctx->acomp);
-		kfree(acomp_ctx->buffer);
-	}
+	if (!IS_ERR_OR_NULL(acomp_ctx->req))
+		acomp_request_free(acomp_ctx->req);
+	acomp_ctx->req = NULL;
+	acomp = acomp_ctx->acomp;
+	acomp_ctx->acomp = NULL;
+	kfree(acomp_ctx->buffer);
+	acomp_ctx->buffer = NULL;
 	mutex_unlock(&acomp_ctx->mutex);
 
+	crypto_free_acomp(acomp);
+
 	return 0;
 }