| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250225104923.2802-1-hdanton@sina.com>
Date: Tue, 25 Feb 2025 18:49:22 +0800
From: Hillf Danton <hdanton@...a.com>
To: Herbert Xu <herbert@...dor.apana.org.au>
Cc: syzbot <syzbot+1a517ccfcbc6a7ab0f82@...kaller.appspotmail.com>,
linux-crypto@...r.kernel.org,
linux-kernel@...r.kernel.org,
syzkaller-bugs@...glegroups.com,
Yosry Ahmed <yosry.ahmed@...ux.dev>,
linux-mm@...ck.org
Subject: Re: mm: zswap: fix crypto_free_acomp deadlock in zswap_cpu_comp_dead
On Tue, 25 Feb 2025 16:53:58 +0800 Herbert Xu wrote:
> 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@...kaller.appspotmail.com
> Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>
>
> 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;
> }
>
> --
> Email: Herbert Xu <herbert@...dor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
>
[snippet of the syz report]
>>-> #1 (fs_reclaim){+.+.}-{0:0}:
>> __fs_reclaim_acquire mm/page_alloc.c:3853 [inline]
>> fs_reclaim_acquire+0x102/0x150 mm/page_alloc.c:3867
>> might_alloc include/linux/sched/mm.h:318 [inline]
>> slab_pre_alloc_hook mm/slub.c:4066 [inline]
>> slab_alloc_node mm/slub.c:4144 [inline]
>> __kmalloc_cache_node_noprof+0x54/0x420 mm/slub.c:4333
>> kmalloc_node_noprof include/linux/slab.h:924 [inline]
>> __get_vm_area_node+0x101/0x2f0 mm/vmalloc.c:3127
>> __vmalloc_node_range_noprof+0x26a/0x1530 mm/vmalloc.c:3806
>> __vmalloc_node_noprof mm/vmalloc.c:3911 [inline]
>> vmalloc_node_noprof+0x6f/0x90 mm/vmalloc.c:4022
>> crypto_scomp_alloc_scratches crypto/scompress.c:86 [inline]
>> crypto_scomp_init_tfm+0x122/0x270 crypto/scompress.c:107
>> crypto_create_tfm_node+0x100/0x320 crypto/api.c:539
>> crypto_create_tfm crypto/internal.h:120 [inline]
>> crypto_init_scomp_ops_async+0x5d/0x1d0 crypto/scompress.c:217
>> crypto_acomp_init_tfm+0x240/0x2e0 crypto/acompress.c:70
>> crypto_create_tfm_node+0x100/0x320 crypto/api.c:539
>> crypto_alloc_tfm_node+0x102/0x260 crypto/api.c:640
>> zswap_cpu_comp_prepare+0xe2/0x420 mm/zswap.c:834
>> cpuhp_invoke_callback+0x20c/0xa10 kernel/cpu.c:204
>> cpuhp_issue_call+0x1c0/0x980 kernel/cpu.c:2376
>> __cpuhp_state_add_instance_cpuslocked+0x1a4/0x3c0 kernel/cpu.c:2438
>> __cpuhp_state_add_instance+0xd7/0x2e0 kernel/cpu.c:2459
>> cpuhp_state_add_instance include/linux/cpuhotplug.h:387 [inline]
>> zswap_pool_create+0x59a/0x7b0 mm/zswap.c:291
Given mutex_init() [1], nobody should take the lock, so the report
sounds false positive.
[1] https://elixir.bootlin.com/linux/v6.14-rc3/source/mm/zswap.c#L289
>> __zswap_pool_create_fallback mm/zswap.c:359 [inline]
>> zswap_setup+0x402/0x810 mm/zswap.c:1814
>> zswap_init+0x2c/0x40 mm/zswap.c:1850
>> do_one_initcall+0x128/0x700 init/main.c:1257
>> do_initcall_level init/main.c:1319 [inline]
>> do_initcalls init/main.c:1335 [inline]
>> do_basic_setup init/main.c:1354 [inline]
>> kernel_init_freeable+0x5c7/0x900 init/main.c:1568
>> kernel_init+0x1c/0x2b0 init/main.c:1457
>> ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:148
>> ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
>>
>>-> #0 (scomp_lock){+.+.}-{4:4}:
Powered by blists - more mailing lists