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

Roberto Sassu <[email protected]>
Newsgroups gmane.comp.file-systems.reiserfs.general,gmane.linux.file-systems,gmane.linux.kernel
Message-ID <[email protected]>
On 5/5/2023 10:51 PM, syzbot wrote:
> syzbot has bisected this issue to:
> 
> commit d82dcd9e21b77d338dc4875f3d4111f0db314a7c
> Author: Roberto Sassu <[email protected]>
> 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: [email protected]
> 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
0001-reiserfs-Move-d_instantiate_new-out-of-the-write-loc.patch (text/plain, 2.7 KB)
From cf5445afc351bbc55a0080f1bc408ff496aeb879 Mon Sep 17 00:00:00 2001
From: Roberto Sassu <[email protected]>
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: [email protected]
Signed-off-by: Roberto Sassu <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.