Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[Roberto Sassu <roberto.sassu@huaweicloud.com>]

[-- Attachment #1: Type: text/plain, Size: 1285 bytes --]

On 5/5/2023 10:51 PM, syzbot wrote:
> syzbot has bisected this issue to:
> 
> commit d82dcd9e21b77d338dc4875f3d4111f0db314a7c
> Author: Roberto Sassu <roberto.sassu@huawei.com>
> Date:   Fri Mar 31 12:32:18 2023 +0000
> 
>      reiserfs: Add security prefix to xattr name in reiserfs_security_write()
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=14403182280000
> start commit:   3c4aa4434377 Merge tag 'ceph-for-6.4-rc1' of https://githu..
> git tree:       upstream
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=16403182280000
> console output: https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
> dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12442414280000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=176a7318280000
> 
> Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
> Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in reiserfs_security_write()")
> 
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

[-- Attachment #2: 0001-reiserfs-Move-d_instantiate_new-out-of-the-write-loc.patch --]
[-- Type: text/plain, Size: 2724 bytes --]

From cf5445afc351bbc55a0080f1bc408ff496aeb879 Mon Sep 17 00:00:00 2001
From: Roberto Sassu <roberto.sassu@huawei.com>
Date: Thu, 1 Jun 2023 20:36:37 +0200
Subject: [RFC][PATCH] reiserfs: Move d_instantiate_new() out of the write lock

Commit 4c05141df57f ("reiserfs: locking, push write lock out of xattr
code") moved xattr operations outside the write lock. The problem is that
not all xattr operations are outside that lock.  For example, the write
lock is not released when d_instantiate_new() is called. At that time,
active LSMs likely fetch the content from their xattrs.

Mixing the two cases (xattr operations without and with a write lock)
could cause a deadlock. For example, a deadlock could happen due to the
following circular dependencies:

write lock (task A) -> inode lock (task B) ->write lock (task B)
-> inode lock (task A)

Make sure that all xattr operations are outside the write lock, by
wrapping all d_instantiate_new() calls with reiserfs_write_unlock() and
reiserfs_write_lock().

Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in reiserfs_security_write()")
Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/reiserfs/namei.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index 52240cc891c..3508bf1a75e 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -689,7 +689,9 @@ static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
 	reiserfs_update_inode_transaction(inode);
 	reiserfs_update_inode_transaction(dir);
 
+	reiserfs_write_unlock(dir->i_sb);
 	d_instantiate_new(dentry, inode);
+	reiserfs_write_lock(dir->i_sb);
 	retval = journal_end(&th);
 
 out_failed:
@@ -773,7 +775,9 @@ static int reiserfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
 		goto out_failed;
 	}
 
+	reiserfs_write_unlock(dir->i_sb);
 	d_instantiate_new(dentry, inode);
+	reiserfs_write_lock(dir->i_sb);
 	retval = journal_end(&th);
 
 out_failed:
@@ -874,7 +878,9 @@ static int reiserfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 	/* the above add_entry did not update dir's stat data */
 	reiserfs_update_sd(&th, dir);
 
+	reiserfs_write_unlock(dir->i_sb);
 	d_instantiate_new(dentry, inode);
+	reiserfs_write_lock(dir->i_sb);
 	retval = journal_end(&th);
 out_failed:
 	reiserfs_write_unlock(dir->i_sb);
@@ -1191,7 +1197,9 @@ static int reiserfs_symlink(struct mnt_idmap *idmap,
 		goto out_failed;
 	}
 
+	reiserfs_write_unlock(parent_dir->i_sb);
 	d_instantiate_new(dentry, inode);
+	reiserfs_write_lock(parent_dir->i_sb);
 	retval = journal_end(&th);
 out_failed:
 	reiserfs_write_unlock(parent_dir->i_sb);
-- 
2.25.1