| 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: <CAOQ4uxhrSSpPijzeuFWRZBrgZvEyk6aLK=q7fBz3rpiZcHZrvg@mail.gmail.com>
Date: Fri, 27 Sep 2024 15:59:11 +0200
From: Amir Goldstein <amir73il@...il.com>
To: Lizhi Xu <lizhi.xu@...driver.com>
Cc: jack@...e.cz, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
phillip@...ashfs.org.uk, squashfs-devel@...ts.sourceforge.net,
syzbot+c679f13773f295d2da53@...kaller.appspotmail.com,
syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH V2] inotify: Fix possible deadlock in fsnotify_destroy_mark
On Fri, Sep 27, 2024 at 3:38 PM Lizhi Xu <lizhi.xu@...driver.com> wrote:
>
> [Syzbot reported]
> WARNING: possible circular locking dependency detected
> 6.11.0-rc4-syzkaller-00019-gb311c1b497e5 #0 Not tainted
> ------------------------------------------------------
> kswapd0/78 is trying to acquire lock:
> ffff88801b8d8930 (&group->mark_mutex){+.+.}-{3:3}, at: fsnotify_group_lock include/linux/fsnotify_backend.h:270 [inline]
> ffff88801b8d8930 (&group->mark_mutex){+.+.}-{3:3}, at: fsnotify_destroy_mark+0x38/0x3c0 fs/notify/mark.c:578
>
> but task is already holding lock:
> ffffffff8ea2fd60 (fs_reclaim){+.+.}-{0:0}, at: balance_pgdat mm/vmscan.c:6841 [inline]
> ffffffff8ea2fd60 (fs_reclaim){+.+.}-{0:0}, at: kswapd+0xbb4/0x35a0 mm/vmscan.c:7223
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (fs_reclaim){+.+.}-{0:0}:
> lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5759
> __fs_reclaim_acquire mm/page_alloc.c:3818 [inline]
> fs_reclaim_acquire+0x88/0x140 mm/page_alloc.c:3832
> might_alloc include/linux/sched/mm.h:334 [inline]
> slab_pre_alloc_hook mm/slub.c:3939 [inline]
> slab_alloc_node mm/slub.c:4017 [inline]
> kmem_cache_alloc_noprof+0x3d/0x2a0 mm/slub.c:4044
> inotify_new_watch fs/notify/inotify/inotify_user.c:599 [inline]
> inotify_update_watch fs/notify/inotify/inotify_user.c:647 [inline]
> __do_sys_inotify_add_watch fs/notify/inotify/inotify_user.c:786 [inline]
> __se_sys_inotify_add_watch+0x72e/0x1070 fs/notify/inotify/inotify_user.c:729
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> -> #0 (&group->mark_mutex){+.+.}-{3:3}:
> check_prev_add kernel/locking/lockdep.c:3133 [inline]
> check_prevs_add kernel/locking/lockdep.c:3252 [inline]
> validate_chain+0x18e0/0x5900 kernel/locking/lockdep.c:3868
> __lock_acquire+0x137a/0x2040 kernel/locking/lockdep.c:5142
> lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5759
> __mutex_lock_common kernel/locking/mutex.c:608 [inline]
> __mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752
> fsnotify_group_lock include/linux/fsnotify_backend.h:270 [inline]
> fsnotify_destroy_mark+0x38/0x3c0 fs/notify/mark.c:578
> fsnotify_destroy_marks+0x14a/0x660 fs/notify/mark.c:934
> fsnotify_inoderemove include/linux/fsnotify.h:264 [inline]
> dentry_unlink_inode+0x2e0/0x430 fs/dcache.c:403
> __dentry_kill+0x20d/0x630 fs/dcache.c:610
> shrink_kill+0xa9/0x2c0 fs/dcache.c:1055
> shrink_dentry_list+0x2c0/0x5b0 fs/dcache.c:1082
> prune_dcache_sb+0x10f/0x180 fs/dcache.c:1163
> super_cache_scan+0x34f/0x4b0 fs/super.c:221
> do_shrink_slab+0x701/0x1160 mm/shrinker.c:435
> shrink_slab+0x1093/0x14d0 mm/shrinker.c:662
> shrink_one+0x43b/0x850 mm/vmscan.c:4815
> shrink_many mm/vmscan.c:4876 [inline]
> lru_gen_shrink_node mm/vmscan.c:4954 [inline]
> shrink_node+0x3799/0x3de0 mm/vmscan.c:5934
> kswapd_shrink_node mm/vmscan.c:6762 [inline]
> balance_pgdat mm/vmscan.c:6954 [inline]
> kswapd+0x1bcd/0x35a0 mm/vmscan.c:7223
> kthread+0x2f0/0x390 kernel/kthread.c:389
> ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
> ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
>
> other info that might help us debug this:
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(fs_reclaim);
> lock(&group->mark_mutex);
> lock(fs_reclaim);
> lock(&group->mark_mutex);
>
> *** DEADLOCK ***
>
> [Analysis]
> The inotify_new_watch() call passes through GFP_KERNEL, use memalloc_nofs_save/
> memalloc_nofs_restore to make sure we don't end up with the fs reclaim dependency.
>
> That any notification group needs to use NOFS allocations to be safe
> against this race so we can just remove FSNOTIFY_GROUP_NOFS and
> unconditionally do memalloc_nofs_save() in fsnotify_group_lock().
>
> Reported-and-tested-by: syzbot+c679f13773f295d2da53@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=c679f13773f295d2da53
> Signed-off-by: Lizhi Xu <lizhi.xu@...driver.com>
> ---
> V1 -> V2: remove FSNOTIFY_GROUP_NOFS in fsnotify_group_lock and unlock
>
> fs/notify/inotify/inotify_user.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index 8be029bc50b1..7b0a2809fc2d 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -268,14 +268,12 @@ struct fsnotify_group {
> static inline void fsnotify_group_lock(struct fsnotify_group *group)
> {
> mutex_lock(&group->mark_mutex);
> - if (group->flags & FSNOTIFY_GROUP_NOFS)
> - group->owner_flags = memalloc_nofs_save();
> + group->owner_flags = memalloc_nofs_save();
> }
>
> static inline void fsnotify_group_unlock(struct fsnotify_group *group)
> {
> - if (group->flags & FSNOTIFY_GROUP_NOFS)
> - memalloc_nofs_restore(group->owner_flags);
> + memalloc_nofs_restore(group->owner_flags);
> mutex_unlock(&group->mark_mutex);
> }
>
You missed several more instances of FSNOTIFY_GROUP_NOFS
that need to be removed, and please also remove the definition of the
flag as well as the nofs_marks_lock lockdep key that is no longer needed.
Thanks,
Amir.
Powered by blists - more mailing lists