[lvc-project] [PATCH] jffs2: silence lockdep warning on evict path

[Ilya Shchipletsov]

Syzkaller detected a possible deadlock in jffs2_do_clear_inode that happens
in kswapd's evict path. This is however a false positive because in
jffs2_evict_inode we are the only holder of inode and nobody else should be
touching any locks of such inode.

WARNING: possible circular locking dependency detected
6.1.128-syzkaller-00157-gf31f96bd278e #0 Not tainted
------------------------------------------------------
kswapd0/72 is trying to acquire lock:
ffff8880945d6998 (&f->sem){+.+.}-{3:3}, at: jffs2_do_clear_inode+0x56/0x570 fs/jffs2/readinode.c:1419

but task is already holding lock:
ffffffff8a68b100 (fs_reclaim){+.+.}-{0:0}, at: balance_pgdat+0xa15/0x1510 mm/vmscan.c:7173

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (fs_reclaim){+.+.}-{0:0}:
       __fs_reclaim_acquire mm/page_alloc.c:4719 [inline]
       fs_reclaim_acquire+0x100/0x150 mm/page_alloc.c:4733
       might_alloc include/linux/sched/mm.h:271 [inline]
       slab_pre_alloc_hook mm/slab.h:710 [inline]
       slab_alloc_node mm/slub.c:3318 [inline]
       slab_alloc mm/slub.c:3406 [inline]
       __kmem_cache_alloc_lru mm/slub.c:3413 [inline]
       kmem_cache_alloc+0x43/0x360 mm/slub.c:3422
       jffs2_do_read_inode+0x300/0x510 fs/jffs2/readinode.c:1372
       jffs2_iget+0x1bb/0xcb0 fs/jffs2/fs.c:276
       jffs2_do_fill_super+0x449/0xa60 fs/jffs2/fs.c:575
       jffs2_fill_super+0x27e/0x370 fs/jffs2/super.c:290
       mtd_get_sb+0x16f/0x220 drivers/mtd/mtdsuper.c:80
       mtd_get_sb_by_nr drivers/mtd/mtdsuper.c:111 [inline]
       get_tree_mtd+0x5ff/0x750 drivers/mtd/mtdsuper.c:164
       vfs_get_tree+0x8e/0x300 fs/super.c:1573
       do_new_mount fs/namespace.c:3056 [inline]
       path_mount+0x6a6/0x1e90 fs/namespace.c:3386
       do_mount fs/namespace.c:3399 [inline]
       __do_sys_mount fs/namespace.c:3607 [inline]
       __se_sys_mount fs/namespace.c:3584 [inline]
       __x64_sys_mount+0x283/0x300 fs/namespace.c:3584
       do_syscall_x64 arch/x86/entry/common.c:51 [inline]
       do_syscall_64+0x35/0x80 arch/x86/entry/common.c:81
       entry_SYSCALL_64_after_hwframe+0x6e/0xd8

-> #0 (&f->sem){+.+.}-{3:3}:
       check_prev_add kernel/locking/lockdep.c:3090 [inline]
       check_prevs_add kernel/locking/lockdep.c:3209 [inline]
       validate_chain kernel/locking/lockdep.c:3825 [inline]
       __lock_acquire+0x2a29/0x5320 kernel/locking/lockdep.c:5049
       lock_acquire kernel/locking/lockdep.c:5662 [inline]
       lock_acquire+0x194/0x4b0 kernel/locking/lockdep.c:5627
       __mutex_lock_common kernel/locking/mutex.c:603 [inline]
       __mutex_lock+0x14c/0x19f0 kernel/locking/mutex.c:747
       jffs2_do_clear_inode+0x56/0x570 fs/jffs2/readinode.c:1419
       evict+0x32c/0x810 fs/inode.c:705
       dispose_list+0xd7/0x1a0 fs/inode.c:738
       prune_icache_sb+0xe7/0x150 fs/inode.c:941
       super_cache_scan+0x38a/0x590 fs/super.c:106
       do_shrink_slab+0x412/0xa00 mm/vmscan.c:853
       shrink_slab+0x178/0x670 mm/vmscan.c:1013
       shrink_node_memcgs mm/vmscan.c:6147 [inline]
       shrink_node+0x957/0x1fb0 mm/vmscan.c:6176
       kswapd_shrink_node mm/vmscan.c:6968 [inline]
       balance_pgdat+0x8ed/0x1510 mm/vmscan.c:7158
       kswapd+0x5d4/0xb80 mm/vmscan.c:7418
       kthread+0x2e1/0x3a0 kernel/kthread.c:376
       ret_from_fork+0x22/0x30 arch/x86/entry/entry_64.S:295

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(fs_reclaim);
                               lock(&f->sem);
                               lock(fs_reclaim);
  lock(&f->sem);

 *** DEADLOCK ***

Fix this false positive by using mutex_trylock instead of mutex_lock to
avoid creating a false locking dependency.

jffs2_do_crccheck_inode also calls this function, with local mutex,
which should be safe, but to be extremely sure and to make code more
future-proof WARN_ON_ONCE was used.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Co-developed-by: Nikita Marushkin <hfggklm at gmail.com>
Signed-off-by: Nikita Marushkin <hfggklm at gmail.com>
Signed-off-by: Ilya Shchipletsov <rabbelkin at mail.ru>
---
 fs/jffs2/readinode.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/jffs2/readinode.c b/fs/jffs2/readinode.c
index 03b4f99614be..3d2b2e5fc2c5 100644
--- a/fs/jffs2/readinode.c
+++ b/fs/jffs2/readinode.c
@@ -1416,7 +1416,17 @@ void jffs2_do_clear_inode(struct jffs2_sb_info *c, struct jffs2_inode_info *f)
 	int deleted;
 
 	jffs2_xattr_delete_inode(c, f->inocache);
-	mutex_lock(&f->sem);
+
+	/*
+	 * We should be the only ones having a reference to this struct
+	 * jffs2_inode_info. So the locking is actually unnecessary. Besides,
+	 * lockdep triggers a false-positive warning on &f->sem here about
+	 * reclaim circular dependency. Play it safe and bump a warning if
+	 * this doesn't hold true.
+	 */
+	if (WARN_ON_ONCE(!mutex_trylock(&f->sem)))
+		return;
+
 	deleted = f->inocache && !f->inocache->pino_nlink;
 
 	if (f->inocache && f->inocache->state != INO_STATE_CHECKING)
-- 
2.43.0