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 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: [email protected] > > 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
0001-reiserfs-Disable-by-default-security-xattr-init-sinc.patch
(text/x-patch, 2.8 KB)
From 73bb02eb7a751c447af43d7cac7c191329b6dd55 Mon Sep 17 00:00:00 2001 From: Roberto Sassu <[email protected]> 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: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8 Suggested-by: Jeff Mahoney <[email protected]> Signed-off-by: Roberto Sassu <[email protected]> --- 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