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

[Roberto Sassu]

On Fri, 2023-06-02 at 02:17 -0700, syzbot wrote:
> Hello,
> 
> syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> 
> Reported-and-tested-by: syzbot+8fb64a61fdd96b50f3b8@xxxxxxxxxxxxxxxxxxxxxxxxx
> 
> Tested on:
> 
> commit:         4432b507 lsm: fix a number of misspellings
> git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git next
> console output: https://syzkaller.appspot.com/x/log.txt?x=166c541d280000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=38526bf24c8d961b
> dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
> compiler:       Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
> patch:          https://syzkaller.appspot.com/x/patch.diff?x=1095cd79280000
> 
> Note: testing is done by a robot and is best-effort only.

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git next
From 73bb02eb7a751c447af43d7cac7c191329b6dd55 Mon Sep 17 00:00:00 2001
From: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
Date: Fri, 2 Jun 2023 10:10:28 +0200
Subject: [PATCH] reiserfs: Disable by default security xattr init since it
 never worked

Commit d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in
reiserfs_security_write()"), while fixed the security xattr initialization,
it also revealed a circular locking dependency between the reiserfs write
lock and the inode lock.

Add the new config option CONFIG_REISERFS_FS_SECURITY_INIT to
enable/disable the feature. Also, since the bug in security xattr
initialization was introduced since the beginning, disable it by default.

Reported-and-tested-by: syzbot+8fb64a61fdd96b50f3b8@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
Suggested-by: Jeff Mahoney <jeffm@xxxxxxxx>
Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
---
 fs/reiserfs/Kconfig          | 15 +++++++++++++++
 fs/reiserfs/super.c          |  3 +++
 fs/reiserfs/xattr_security.c |  3 +++
 3 files changed, 21 insertions(+)

diff --git a/fs/reiserfs/Kconfig b/fs/reiserfs/Kconfig
index 4d22ecfe0fa..a618d0bda7b 100644
--- a/fs/reiserfs/Kconfig
+++ b/fs/reiserfs/Kconfig
@@ -88,3 +88,18 @@ config REISERFS_FS_SECURITY
 
 	  If you are not using a security module that requires using
 	  extended attributes for file security labels, say N.
+
+config REISERFS_FS_SECURITY_INIT
+	bool "ReiserFS Security Labels initialization"
+	depends on REISERFS_FS_XATTR
+	default false
+	help
+	  Init new inodes with security labels provided by LSMs.
+
+	  It was broken from the beginning, since the xattr name was
+	  missing the 'security.' prefix.
+
+	  Enabling this option might cause lockdep warnings and
+	  ultimately deadlocks.
+
+	  If unsure, say N.
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 929acce6e73..b427d03d0ea 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -1654,6 +1654,9 @@ static int read_super_block(struct super_block *s, int offset)
 
 	reiserfs_warning(NULL, "", "reiserfs filesystem is deprecated and "
 		"scheduled to be removed from the kernel in 2025");
+	if (IS_ENABLED(CONFIG_REISERFS_FS_SECURITY_INIT))
+		reiserfs_warning(NULL, "", "initializing security xattrs can cause deadlocks");
+
 	SB_BUFFER_WITH_SB(s) = bh;
 	SB_DISK_SUPER_BLOCK(s) = rs;
 
diff --git a/fs/reiserfs/xattr_security.c b/fs/reiserfs/xattr_security.c
index 078dd8cc312..d82c4507803 100644
--- a/fs/reiserfs/xattr_security.c
+++ b/fs/reiserfs/xattr_security.c
@@ -69,6 +69,9 @@ int reiserfs_security_init(struct inode *dir, struct inode *inode,
 	sec->value = NULL;
 	sec->length = 0;
 
+	if (!IS_ENABLED(CONFIG_REISERFS_FS_SECURITY_INIT))
+		return 0;
+
 	/* Don't add selinux attributes on xattrs - they'll never get used */
 	if (IS_PRIVATE(dir))
 		return 0;
-- 
2.25.1