Received-SPF: pass (google.com: domain of linux-kernel+bounces-81230-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: Hillf Danton <hdanton@sina.com>
To: Boqun Feng <boqun.feng@gmail.com>
Cc: syzbot <syzbot+c2ada45c23d98d646118@syzkaller.appspotmail.com>,
	almaz.alexandrovich@paragon-software.com,
	linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	ntfs3@lists.linux.dev,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	syzkaller-bugs@googlegroups.com
Subject: Re: [syzbot] [ntfs3?] possible deadlock in ntfs_set_state (2)
Date: Mon, 26 Feb 2024 18:55:06 +0800
Message-Id: <20240226105506.1398-1-hdanton@sina.com>
In-Reply-To: <ZdwSXCaTrzq7mm7Z@boqun-archlinux>
References: <000000000000998cff06113e1d91@google.com> <20240213114151.982-1-hdanton@sina.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

On Sun, 25 Feb 2024 20:23:56 -0800 Boqun Feng wrote:
> On Tue, Feb 13, 2024 at 07:41:50PM +0800, Hillf Danton wrote:
> > On Mon, 12 Feb 2024 23:12:22 -0800
> > > HEAD commit:    716f4aaa7b48 Merge tag 'vfs-6.8-rc5.fixes' of git://git.ke..
> > > git tree:       upstream
> > > console+strace: https://syzkaller.appspot.com/x/log.txt?x=100fd062180000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=1d7c92dd8d5c7a1e
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=c2ada45c23d98d646118
> > > compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11fcbd48180000
> > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17f6e642180000
> > > 
> > > ============================================
> > > WARNING: possible recursive locking detected
> > > 6.8.0-rc4-syzkaller-00003-g716f4aaa7b48 #0 Not tainted
> > > --------------------------------------------
> > > syz-executor354/5071 is trying to acquire lock:
> > > ffff888070ee0100 (&ni->ni_lock#3){+.+.}-{3:3}, at: ntfs_set_state+0x1ff/0x6c0 fs/ntfs3/fsntfs.c:947
> 
> this is a mutex_lock_nested() with a subkey 0.
> 
> > > 
> > > but task is already holding lock:
> > > ffff888070de3c00 (&ni->ni_lock#3){+.+.}-{3:3}, at: ni_trylock fs/ntfs3/ntfs_fs.h:1141 [inline]
> > > ffff888070de3c00 (&ni->ni_lock#3){+.+.}-{3:3}, at: ni_write_inode+0x1bc/0x1010 fs/ntfs3/frecord.c:3265
> 
> These two are try locks.
> 
> > > 
> > This report looks false positive but raises the question -- what made lockedp
> 
> This is not a false positive by lockdep locking rules, basically it
> reported deadlock cases as the follow:
> 
> 	mutex_trylock(A1);
> 	mutex_trylock(A2);
> 	mutex_lock(A1 /* or A2 */);
> 
> Two things to notice here: 1) these two trylock()s not resulting in
> real deadlock cases must be because they are on different lock
> instances, 2) deadlock detectors work on lock classes, so although the
> mutex_lock() above may be on a different instance (say A3), currently
> there is no way for lockdep to tell that. In this case, users need to
> use subkeys to tell lockdep mutex_lock() and mutex_trylock() are on
> different sets of instannces (i.e. sub classes). Note that subkey == 0
> means the main class.

Test non-zero subkey.

#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git  716f4aaa7b48

--- x/fs/ntfs3/fsntfs.c
+++ y/fs/ntfs3/fsntfs.c
@@ -944,7 +944,7 @@ int ntfs_set_state(struct ntfs_sb_info *
 	if (!ni)
 		return -EINVAL;
 
-	mutex_lock_nested(&ni->ni_lock, NTFS_INODE_MUTEX_DIRTY);
+	mutex_lock_nested(&ni->ni_lock, NTFS_INODE_MUTEX_NORMAL);
 
 	attr = ni_find_attr(ni, NULL, NULL, ATTR_VOL_INFO, NULL, 0, NULL, &mi);
 	if (!attr) {
--