* [syzbot] [reiserfs?] possible deadlock in open_xa_dir
@ 2022-12-31 6:35 syzbot
2023-05-05 7:10 ` syzbot
` (2 more replies)
0 siblings, 3 replies; 23+ messages in thread
From: syzbot @ 2022-12-31 6:35 UTC (permalink / raw)
To: linux-kernel, reiserfs-devel, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: 1b929c02afd3 Linux 6.2-rc1
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11da7f8c480000
kernel config: https://syzkaller.appspot.com/x/.config?x=2651619a26b4d687
dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
Unfortunately, I don't have any reproducer for this issue yet.
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/d562ace1a56c/disk-1b929c02.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/39f000fe6b9e/vmlinux-1b929c02.xz
kernel image: https://storage.googleapis.com/syzbot-assets/1c67e48de5a0/bzImage-1b929c02.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
======================================================
WARNING: possible circular locking dependency detected
6.2.0-rc1-syzkaller #0 Not tainted
------------------------------------------------------
syz-executor.1/19784 is trying to acquire lock:
ffff88807d682090 (&sbi->lock){+.+.}-{3:3}, at: reiserfs_write_lock+0x79/0x100 fs/reiserfs/lock.c:27
but task is already holding lock:
ffff8880899d6ce0 (&type->i_mutex_dir_key#9/3){+.+.}-{3:3}, at: inode_lock_nested include/linux/fs.h:791 [inline]
ffff8880899d6ce0 (&type->i_mutex_dir_key#9/3){+.+.}-{3:3}, at: open_xa_root fs/reiserfs/xattr.c:127 [inline]
ffff8880899d6ce0 (&type->i_mutex_dir_key#9/3){+.+.}-{3:3}, at: open_xa_dir+0x127/0x830 fs/reiserfs/xattr.c:152
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&type->i_mutex_dir_key#9/3){+.+.}-{3:3}:
down_write_nested+0x98/0x220 kernel/locking/rwsem.c:1672
inode_lock_nested include/linux/fs.h:791 [inline]
open_xa_root fs/reiserfs/xattr.c:127 [inline]
open_xa_dir+0x127/0x830 fs/reiserfs/xattr.c:152
reiserfs_for_each_xattr+0x1ab/0x9a0 fs/reiserfs/xattr.c:252
reiserfs_delete_xattrs+0x20/0xa0 fs/reiserfs/xattr.c:364
reiserfs_evict_inode+0x2e7/0x540 fs/reiserfs/inode.c:53
evict+0x2ed/0x6b0 fs/inode.c:664
iput_final fs/inode.c:1747 [inline]
iput.part.0+0x59b/0x880 fs/inode.c:1773
iput+0x5c/0x80 fs/inode.c:1763
reiserfs_create+0x65a/0x730 fs/reiserfs/namei.c:688
vfs_create fs/namei.c:3115 [inline]
vfs_create+0x3ed/0x670 fs/namei.c:3101
do_mknodat+0x3c4/0x510 fs/namei.c:3965
__do_sys_mknod fs/namei.c:3998 [inline]
__se_sys_mknod fs/namei.c:3996 [inline]
__x64_sys_mknod+0x11e/0x180 fs/namei.c:3996
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
-> #0 (&sbi->lock){+.+.}-{3:3}:
check_prev_add kernel/locking/lockdep.c:3097 [inline]
check_prevs_add kernel/locking/lockdep.c:3216 [inline]
validate_chain kernel/locking/lockdep.c:3831 [inline]
__lock_acquire+0x2a43/0x56d0 kernel/locking/lockdep.c:5055
lock_acquire kernel/locking/lockdep.c:5668 [inline]
lock_acquire+0x1e3/0x630 kernel/locking/lockdep.c:5633
__mutex_lock_common kernel/locking/mutex.c:603 [inline]
__mutex_lock+0x12f/0x1360 kernel/locking/mutex.c:747
reiserfs_write_lock+0x79/0x100 fs/reiserfs/lock.c:27
reiserfs_mkdir+0x31c/0x990 fs/reiserfs/namei.c:831
xattr_mkdir fs/reiserfs/xattr.c:76 [inline]
open_xa_root fs/reiserfs/xattr.c:136 [inline]
open_xa_dir+0x6a3/0x830 fs/reiserfs/xattr.c:152
xattr_lookup+0x21/0x3d0 fs/reiserfs/xattr.c:395
reiserfs_xattr_set_handle+0xfb/0xb00 fs/reiserfs/xattr.c:533
reiserfs_xattr_set+0x454/0x5b0 fs/reiserfs/xattr.c:633
trusted_set+0xa7/0xd0 fs/reiserfs/xattr_trusted.c:31
__vfs_setxattr+0x173/0x1e0 fs/xattr.c:202
__vfs_setxattr_noperm+0x129/0x5f0 fs/xattr.c:236
__vfs_setxattr_locked+0x1d3/0x260 fs/xattr.c:297
vfs_setxattr+0x143/0x340 fs/xattr.c:323
do_setxattr+0x151/0x190 fs/xattr.c:608
setxattr+0x146/0x160 fs/xattr.c:631
path_setxattr+0x197/0x1c0 fs/xattr.c:650
__do_sys_setxattr fs/xattr.c:666 [inline]
__se_sys_setxattr fs/xattr.c:662 [inline]
__x64_sys_setxattr+0xc4/0x160 fs/xattr.c:662
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&type->i_mutex_dir_key#9/3);
lock(&sbi->lock);
lock(&type->i_mutex_dir_key#9/3);
lock(&sbi->lock);
*** DEADLOCK ***
3 locks held by syz-executor.1/19784:
#0: ffff88801c7fe460 (sb_writers#17){.+.+}-{0:0}, at: path_setxattr+0xb2/0x1c0 fs/xattr.c:648
#1: ffff8880899d7a20 (&type->i_mutex_dir_key#9){++++}-{3:3}, at: inode_lock include/linux/fs.h:756 [inline]
#1: ffff8880899d7a20 (&type->i_mutex_dir_key#9){++++}-{3:3}, at: vfs_setxattr+0x120/0x340 fs/xattr.c:322
#2: ffff8880899d6ce0 (&type->i_mutex_dir_key#9/3){+.+.}-{3:3}, at: inode_lock_nested include/linux/fs.h:791 [inline]
#2: ffff8880899d6ce0 (&type->i_mutex_dir_key#9/3){+.+.}-{3:3}, at: open_xa_root fs/reiserfs/xattr.c:127 [inline]
#2: ffff8880899d6ce0 (&type->i_mutex_dir_key#9/3){+.+.}-{3:3}, at: open_xa_dir+0x127/0x830 fs/reiserfs/xattr.c:152
stack backtrace:
CPU: 1 PID: 19784 Comm: syz-executor.1 Not tainted 6.2.0-rc1-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/26/2022
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xd1/0x138 lib/dump_stack.c:106
check_noncircular+0x25f/0x2e0 kernel/locking/lockdep.c:2177
check_prev_add kernel/locking/lockdep.c:3097 [inline]
check_prevs_add kernel/locking/lockdep.c:3216 [inline]
validate_chain kernel/locking/lockdep.c:3831 [inline]
__lock_acquire+0x2a43/0x56d0 kernel/locking/lockdep.c:5055
lock_acquire kernel/locking/lockdep.c:5668 [inline]
lock_acquire+0x1e3/0x630 kernel/locking/lockdep.c:5633
__mutex_lock_common kernel/locking/mutex.c:603 [inline]
__mutex_lock+0x12f/0x1360 kernel/locking/mutex.c:747
reiserfs_write_lock+0x79/0x100 fs/reiserfs/lock.c:27
reiserfs_mkdir+0x31c/0x990 fs/reiserfs/namei.c:831
xattr_mkdir fs/reiserfs/xattr.c:76 [inline]
open_xa_root fs/reiserfs/xattr.c:136 [inline]
open_xa_dir+0x6a3/0x830 fs/reiserfs/xattr.c:152
xattr_lookup+0x21/0x3d0 fs/reiserfs/xattr.c:395
reiserfs_xattr_set_handle+0xfb/0xb00 fs/reiserfs/xattr.c:533
reiserfs_xattr_set+0x454/0x5b0 fs/reiserfs/xattr.c:633
trusted_set+0xa7/0xd0 fs/reiserfs/xattr_trusted.c:31
__vfs_setxattr+0x173/0x1e0 fs/xattr.c:202
__vfs_setxattr_noperm+0x129/0x5f0 fs/xattr.c:236
__vfs_setxattr_locked+0x1d3/0x260 fs/xattr.c:297
vfs_setxattr+0x143/0x340 fs/xattr.c:323
do_setxattr+0x151/0x190 fs/xattr.c:608
setxattr+0x146/0x160 fs/xattr.c:631
path_setxattr+0x197/0x1c0 fs/xattr.c:650
__do_sys_setxattr fs/xattr.c:666 [inline]
__se_sys_setxattr fs/xattr.c:662 [inline]
__x64_sys_setxattr+0xc4/0x160 fs/xattr.c:662
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7fd78168c0a9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 19 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fd78242a168 EFLAGS: 00000246 ORIG_RAX: 00000000000000bc
RAX: ffffffffffffffda RBX: 00007fd7817abf80 RCX: 00007fd78168c0a9
RDX: 0000000020000440 RSI: 00000000200002c0 RDI: 0000000020000280
RBP: 00007fd7816e7ae9 R08: 0000000000000000 R09: 0000000000000000
R10: 000000000000010c R11: 0000000000000246 R12: 0000000000000000
R13: 00007fff9b71513f R14: 00007fd78242a300 R15: 0000000000022000
</TASK>
REISERFS warning (device loop1): vs-13060 reiserfs_update_sd_size: stat data of object [3 6 0x0 SD] (nlink == 2) not found (pos 5)
REISERFS warning (device loop1): vs-13060 reiserfs_update_sd_size: stat data of object [3 6 0x0 SD] (nlink == 3) not found (pos 3)
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2022-12-31 6:35 [syzbot] [reiserfs?] possible deadlock in open_xa_dir syzbot
@ 2023-05-05 7:10 ` syzbot
2023-05-05 20:51 ` syzbot
2024-03-09 22:20 ` syzbot
2 siblings, 0 replies; 23+ messages in thread
From: syzbot @ 2023-05-05 7:10 UTC (permalink / raw)
To: linux-fsdevel, linux-kernel, reiserfs-devel, syzkaller-bugs
syzbot has found a reproducer for the following issue on:
HEAD commit: 78b421b6a7c6 Merge tag 'linux-watchdog-6.4-rc1' of git://w..
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=150c42a8280000
kernel config: https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
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
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14e2e518280000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17c03112280000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/25168a5d3be8/disk-78b421b6.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/f5af9f784d9d/vmlinux-78b421b6.xz
kernel image: https://storage.googleapis.com/syzbot-assets/7781e4e3e606/bzImage-78b421b6.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/29a59b19c7a8/mount_0.gz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
======================================================
WARNING: possible circular locking dependency detected
6.3.0-syzkaller-13164-g78b421b6a7c6 #0 Not tainted
------------------------------------------------------
syz-executor363/4996 is trying to acquire lock:
ffff888074558980 (&type->i_mutex_dir_key#6/3){+.+.}-{3:3}, at: inode_lock_nested include/linux/fs.h:810 [inline]
ffff888074558980 (&type->i_mutex_dir_key#6/3){+.+.}-{3:3}, at: open_xa_root fs/reiserfs/xattr.c:128 [inline]
ffff888074558980 (&type->i_mutex_dir_key#6/3){+.+.}-{3:3}, at: open_xa_dir+0x136/0x610 fs/reiserfs/xattr.c:153
but task is already holding lock:
ffff8880156f2090 (&sbi->lock){+.+.}-{3:3}, at: reiserfs_write_lock_nested+0x5f/0xd0 fs/reiserfs/lock.c:78
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&sbi->lock){+.+.}-{3:3}:
lock_acquire+0x1e3/0x520 kernel/locking/lockdep.c:5691
__mutex_lock_common+0x1d8/0x2530 kernel/locking/mutex.c:603
__mutex_lock kernel/locking/mutex.c:747 [inline]
mutex_lock_nested+0x1b/0x20 kernel/locking/mutex.c:799
reiserfs_write_lock+0x7a/0xd0 fs/reiserfs/lock.c:27
reiserfs_mkdir+0x321/0x8f0 fs/reiserfs/namei.c:829
xattr_mkdir fs/reiserfs/xattr.c:77 [inline]
open_xa_root fs/reiserfs/xattr.c:137 [inline]
open_xa_dir+0x2cd/0x610 fs/reiserfs/xattr.c:153
xattr_lookup+0x24/0x280 fs/reiserfs/xattr.c:396
reiserfs_xattr_set_handle+0xfc/0xdc0 fs/reiserfs/xattr.c:534
reiserfs_security_write+0x157/0x1d0 fs/reiserfs/xattr_security.c:106
reiserfs_new_inode+0x1631/0x1d40 fs/reiserfs/inode.c:2111
reiserfs_create+0x3e7/0x6e0 fs/reiserfs/namei.c:666
lookup_open fs/namei.c:3492 [inline]
open_last_lookups fs/namei.c:3560 [inline]
path_openat+0x13df/0x3170 fs/namei.c:3788
do_filp_open+0x234/0x490 fs/namei.c:3818
do_sys_openat2+0x13f/0x500 fs/open.c:1356
do_sys_open fs/open.c:1372 [inline]
__do_sys_openat fs/open.c:1388 [inline]
__se_sys_openat fs/open.c:1383 [inline]
__x64_sys_openat+0x247/0x290 fs/open.c:1383
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
-> #0 (&type->i_mutex_dir_key#6/3){+.+.}-{3:3}:
check_prev_add kernel/locking/lockdep.c:3108 [inline]
check_prevs_add kernel/locking/lockdep.c:3227 [inline]
validate_chain+0x166b/0x58e0 kernel/locking/lockdep.c:3842
__lock_acquire+0x1295/0x2000 kernel/locking/lockdep.c:5074
lock_acquire+0x1e3/0x520 kernel/locking/lockdep.c:5691
down_write_nested+0x3d/0x50 kernel/locking/rwsem.c:1689
inode_lock_nested include/linux/fs.h:810 [inline]
open_xa_root fs/reiserfs/xattr.c:128 [inline]
open_xa_dir+0x136/0x610 fs/reiserfs/xattr.c:153
xattr_lookup+0x24/0x280 fs/reiserfs/xattr.c:396
reiserfs_xattr_get+0xfd/0x570 fs/reiserfs/xattr.c:678
__vfs_getxattr+0x436/0x470 fs/xattr.c:424
smk_fetch+0xb1/0x140 security/smack/smack_lsm.c:295
smack_d_instantiate+0x6d9/0xb40 security/smack/smack_lsm.c:3475
security_d_instantiate+0x9b/0xf0 security/security.c:3760
d_instantiate_new+0x65/0x120 fs/dcache.c:2053
reiserfs_create+0x5cf/0x6e0 fs/reiserfs/namei.c:692
lookup_open fs/namei.c:3492 [inline]
open_last_lookups fs/namei.c:3560 [inline]
path_openat+0x13df/0x3170 fs/namei.c:3788
do_filp_open+0x234/0x490 fs/namei.c:3818
do_sys_openat2+0x13f/0x500 fs/open.c:1356
do_sys_open fs/open.c:1372 [inline]
__do_sys_openat fs/open.c:1388 [inline]
__se_sys_openat fs/open.c:1383 [inline]
__x64_sys_openat+0x247/0x290 fs/open.c:1383
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&sbi->lock);
lock(&type->i_mutex_dir_key#6/3);
lock(&sbi->lock);
lock(&type->i_mutex_dir_key#6/3);
*** DEADLOCK ***
3 locks held by syz-executor363/4996:
#0: ffff88807c6be460 (sb_writers#9){.+.+}-{0:0}, at: mnt_want_write+0x3f/0x90 fs/namespace.c:394
#1: ffff8880745582e0 (&type->i_mutex_dir_key#6){+.+.}-{3:3}, at: inode_lock include/linux/fs.h:775 [inline]
#1: ffff8880745582e0 (&type->i_mutex_dir_key#6){+.+.}-{3:3}, at: open_last_lookups fs/namei.c:3557 [inline]
#1: ffff8880745582e0 (&type->i_mutex_dir_key#6){+.+.}-{3:3}, at: path_openat+0x7ba/0x3170 fs/namei.c:3788
#2: ffff8880156f2090 (&sbi->lock){+.+.}-{3:3}, at: reiserfs_write_lock_nested+0x5f/0xd0 fs/reiserfs/lock.c:78
stack backtrace:
CPU: 1 PID: 4996 Comm: syz-executor363 Not tainted 6.3.0-syzkaller-13164-g78b421b6a7c6 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/14/2023
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x1e7/0x2d0 lib/dump_stack.c:106
check_noncircular+0x2fe/0x3b0 kernel/locking/lockdep.c:2188
check_prev_add kernel/locking/lockdep.c:3108 [inline]
check_prevs_add kernel/locking/lockdep.c:3227 [inline]
validate_chain+0x166b/0x58e0 kernel/locking/lockdep.c:3842
__lock_acquire+0x1295/0x2000 kernel/locking/lockdep.c:5074
lock_acquire+0x1e3/0x520 kernel/locking/lockdep.c:5691
down_write_nested+0x3d/0x50 kernel/locking/rwsem.c:1689
inode_lock_nested include/linux/fs.h:810 [inline]
open_xa_root fs/reiserfs/xattr.c:128 [inline]
open_xa_dir+0x136/0x610 fs/reiserfs/xattr.c:153
xattr_lookup+0x24/0x280 fs/reiserfs/xattr.c:396
reiserfs_xattr_get+0xfd/0x570 fs/reiserfs/xattr.c:678
__vfs_getxattr+0x436/0x470 fs/xattr.c:424
smk_fetch+0xb1/0x140 security/smack/smack_lsm.c:295
smack_d_instantiate+0x6d9/0xb40 security/smack/smack_lsm.c:3475
security_d_instantiate+0x9b/0xf0 security/security.c:3760
d_instantiate_new+0x65/0x120 fs/dcache.c:2053
reiserfs_create+0x5cf/0x6e0 fs/reiserfs/namei.c:692
lookup_open fs/namei.c:3492 [inline]
open_last_lookups fs/namei.c:3560 [inline]
path_openat+0x13df/0x3170 fs/namei.c:3788
do_filp_open+0x234/0x490 fs/namei.c:3818
do_sys_openat2+0x13f/0x500 fs/open.c:1356
do_sys_open fs/open.c:1372 [inline]
__do_sys_openat fs/open.c:1388 [inline]
__se_sys_openat fs/open.c:1383 [inline]
__x64_sys_openat+0x247/0x290 fs/open.c:1383
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7fdd867933d9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 b1 14 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fdd867402f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
RAX: ffffffffffffffda RBX: 00007fdd868196c0 RCX: 00007fdd867933d9
RDX: 000000000000275a RSI: 0000000020000080 RDI: 00000000ffffff9c
---
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2022-12-31 6:35 [syzbot] [reiserfs?] possible deadlock in open_xa_dir syzbot
2023-05-05 7:10 ` syzbot
@ 2023-05-05 20:51 ` syzbot
2023-05-05 21:36 ` Paul Moore
` (2 more replies)
2024-03-09 22:20 ` syzbot
2 siblings, 3 replies; 23+ messages in thread
From: syzbot @ 2023-05-05 20:51 UTC (permalink / raw)
To: hdanton, linux-fsdevel, linux-kernel, paul, reiserfs-devel,
roberto.sassu, syzkaller-bugs
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-05-05 20:51 ` syzbot
@ 2023-05-05 21:36 ` Paul Moore
2023-06-01 20:19 ` Roberto Sassu
2023-06-01 20:30 ` Roberto Sassu
2 siblings, 0 replies; 23+ messages in thread
From: Paul Moore @ 2023-05-05 21:36 UTC (permalink / raw)
To: syzbot
Cc: hdanton, linux-fsdevel, linux-kernel, reiserfs-devel,
roberto.sassu, syzkaller-bugs
On Fri, May 5, 2023 at 4:51 PM syzbot
<syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> 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
I don't think Roberto's patch identified above is the actual root
cause of this problem as reiserfs_xattr_set_handle() is called in
reiserfs_security_write() both before and after the patch. However,
due to some bad logic in reiserfs_security_write() which Roberto
corrected, I'm thinking that it is possible this code is being
exercised for the first time and syzbot is starting to trigger a
locking issue in the reiserfs code ... ?
--
paul-moore.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
@ 2023-05-05 21:36 ` Paul Moore
0 siblings, 0 replies; 23+ messages in thread
From: Paul Moore @ 2023-05-05 21:36 UTC (permalink / raw)
To: syzbot
Cc: hdanton, linux-fsdevel, linux-kernel, reiserfs-devel,
roberto.sassu, syzkaller-bugs
On Fri, May 5, 2023 at 4:51 PM syzbot
<syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> 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
I don't think Roberto's patch identified above is the actual root
cause of this problem as reiserfs_xattr_set_handle() is called in
reiserfs_security_write() both before and after the patch. However,
due to some bad logic in reiserfs_security_write() which Roberto
corrected, I'm thinking that it is possible this code is being
exercised for the first time and syzbot is starting to trigger a
locking issue in the reiserfs code ... ?
--
paul-moore.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-05-05 21:36 ` Paul Moore
@ 2023-05-31 9:49 ` Roberto Sassu
-1 siblings, 0 replies; 23+ messages in thread
From: Roberto Sassu @ 2023-05-31 9:49 UTC (permalink / raw)
To: Paul Moore, syzbot
Cc: hdanton, linux-fsdevel, linux-kernel, reiserfs-devel,
roberto.sassu, syzkaller-bugs, peterz, mingo, will
On 5/5/2023 11:36 PM, Paul Moore wrote:
> On Fri, May 5, 2023 at 4:51 PM syzbot
> <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> 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
>
> I don't think Roberto's patch identified above is the actual root
> cause of this problem as reiserfs_xattr_set_handle() is called in
> reiserfs_security_write() both before and after the patch. However,
> due to some bad logic in reiserfs_security_write() which Roberto
> corrected, I'm thinking that it is possible this code is being
> exercised for the first time and syzbot is starting to trigger a
> locking issue in the reiserfs code ... ?
+ Jan, Jeff (which basically restructured the lock)
+ Petr, Ingo, Will
I involve the lockdep experts, to get a bit of help on this.
First of all, the lockdep warning is trivial to reproduce:
# dd if=/dev/zero of=reiserfs.img bs=1M count=100
# losetup -f --show reiserfs.img
/dev/loop0
# mkfs.reiserfs /dev/loop0
# mount /dev/loop0 /mnt/
# touch file0
In the testing system, Smack is the major LSM.
Ok, so the warning here is clear:
https://syzkaller.appspot.com/x/log.txt?x=12403182280000
However, I was looking if that can really happen. From this:
[ 77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
[ 77.753772][ T5418] lock_acquire+0x23e/0x630
[ 77.758792][ T5418] __mutex_lock_common+0x1d8/0x2530
[ 77.764504][ T5418] mutex_lock_nested+0x1b/0x20
[ 77.769868][ T5418] reiserfs_write_lock+0x70/0xc0
[ 77.775321][ T5418] reiserfs_mkdir+0x321/0x870
I see that the lock is taken in reiserfs_write_lock(), while lockdep says:
[ 77.710227][ T5418] but task is already holding lock:
[ 77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at:
reiserfs_write_lock_nested+0x4a/0xb0
which is in a different place, I believe here:
int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
/* Path to the pasted item. */
[...]
depth = reiserfs_write_unlock_nested(sb);
dquot_free_space_nodirty(inode, pasted_size);
reiserfs_write_lock_nested(sb, depth);
return retval;
}
This is called by reiserfs_add_entry(), which is called by
reiserfs_create() (it is in the lockdep trace). After returning to
reiserfs_create(), d_instantiate_new() is called.
I don't know exactly, I take the part that the lock is held. But if it
is held, how d_instantiate_new() can be executed in another task?
static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
struct dentry *dentry, umode_t mode, bool excl)
{
[...]
reiserfs_write_lock(dir->i_sb);
retval = journal_begin(&th, dir->i_sb, jbegin_count);
[...]
d_instantiate_new(dentry, inode);
retval = journal_end(&th);
out_failed:
reiserfs_write_unlock(dir->i_sb);
If the lock is held, the scenario lockdep describes cannot happen. Any
thoughts?
Thanks
Roberto
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
@ 2023-05-31 9:49 ` Roberto Sassu
0 siblings, 0 replies; 23+ messages in thread
From: Roberto Sassu @ 2023-05-31 9:49 UTC (permalink / raw)
To: Paul Moore, syzbot
Cc: hdanton, linux-fsdevel, linux-kernel, reiserfs-devel,
roberto.sassu, syzkaller-bugs, peterz, mingo, will
On 5/5/2023 11:36 PM, Paul Moore wrote:
> On Fri, May 5, 2023 at 4:51 PM syzbot
> <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> 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
>
> I don't think Roberto's patch identified above is the actual root
> cause of this problem as reiserfs_xattr_set_handle() is called in
> reiserfs_security_write() both before and after the patch. However,
> due to some bad logic in reiserfs_security_write() which Roberto
> corrected, I'm thinking that it is possible this code is being
> exercised for the first time and syzbot is starting to trigger a
> locking issue in the reiserfs code ... ?
+ Jan, Jeff (which basically restructured the lock)
+ Petr, Ingo, Will
I involve the lockdep experts, to get a bit of help on this.
First of all, the lockdep warning is trivial to reproduce:
# dd if=/dev/zero of=reiserfs.img bs=1M count=100
# losetup -f --show reiserfs.img
/dev/loop0
# mkfs.reiserfs /dev/loop0
# mount /dev/loop0 /mnt/
# touch file0
In the testing system, Smack is the major LSM.
Ok, so the warning here is clear:
https://syzkaller.appspot.com/x/log.txt?x=12403182280000
However, I was looking if that can really happen. From this:
[ 77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
[ 77.753772][ T5418] lock_acquire+0x23e/0x630
[ 77.758792][ T5418] __mutex_lock_common+0x1d8/0x2530
[ 77.764504][ T5418] mutex_lock_nested+0x1b/0x20
[ 77.769868][ T5418] reiserfs_write_lock+0x70/0xc0
[ 77.775321][ T5418] reiserfs_mkdir+0x321/0x870
I see that the lock is taken in reiserfs_write_lock(), while lockdep says:
[ 77.710227][ T5418] but task is already holding lock:
[ 77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at:
reiserfs_write_lock_nested+0x4a/0xb0
which is in a different place, I believe here:
int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
/* Path to the pasted item. */
[...]
depth = reiserfs_write_unlock_nested(sb);
dquot_free_space_nodirty(inode, pasted_size);
reiserfs_write_lock_nested(sb, depth);
return retval;
}
This is called by reiserfs_add_entry(), which is called by
reiserfs_create() (it is in the lockdep trace). After returning to
reiserfs_create(), d_instantiate_new() is called.
I don't know exactly, I take the part that the lock is held. But if it
is held, how d_instantiate_new() can be executed in another task?
static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
struct dentry *dentry, umode_t mode, bool excl)
{
[...]
reiserfs_write_lock(dir->i_sb);
retval = journal_begin(&th, dir->i_sb, jbegin_count);
[...]
d_instantiate_new(dentry, inode);
retval = journal_end(&th);
out_failed:
reiserfs_write_unlock(dir->i_sb);
If the lock is held, the scenario lockdep describes cannot happen. Any
thoughts?
Thanks
Roberto
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-05-31 9:49 ` Roberto Sassu
(?)
@ 2023-05-31 9:52 ` Roberto Sassu
-1 siblings, 0 replies; 23+ messages in thread
From: Roberto Sassu @ 2023-05-31 9:52 UTC (permalink / raw)
To: Paul Moore, syzbot
Cc: hdanton, linux-fsdevel, linux-kernel, reiserfs-devel,
roberto.sassu, syzkaller-bugs, peterz, mingo, will, Jeff Mahoney,
Jan Kara
On 5/31/2023 11:49 AM, Roberto Sassu wrote:
> On 5/5/2023 11:36 PM, Paul Moore wrote:
>> On Fri, May 5, 2023 at 4:51 PM syzbot
>> <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> 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
>>
>> I don't think Roberto's patch identified above is the actual root
>> cause of this problem as reiserfs_xattr_set_handle() is called in
>> reiserfs_security_write() both before and after the patch. However,
>> due to some bad logic in reiserfs_security_write() which Roberto
>> corrected, I'm thinking that it is possible this code is being
>> exercised for the first time and syzbot is starting to trigger a
>> locking issue in the reiserfs code ... ?
>
> + Jan, Jeff (which basically restructured the lock)
Actually adding Jan and Jeff.
Roberto
> + Petr, Ingo, Will
>
> I involve the lockdep experts, to get a bit of help on this.
>
> First of all, the lockdep warning is trivial to reproduce:
>
> # dd if=/dev/zero of=reiserfs.img bs=1M count=100
> # losetup -f --show reiserfs.img
> /dev/loop0
> # mkfs.reiserfs /dev/loop0
> # mount /dev/loop0 /mnt/
> # touch file0
>
> In the testing system, Smack is the major LSM.
>
> Ok, so the warning here is clear:
>
> https://syzkaller.appspot.com/x/log.txt?x=12403182280000
>
> However, I was looking if that can really happen. From this:
>
> [ 77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
> [ 77.753772][ T5418] lock_acquire+0x23e/0x630
> [ 77.758792][ T5418] __mutex_lock_common+0x1d8/0x2530
> [ 77.764504][ T5418] mutex_lock_nested+0x1b/0x20
> [ 77.769868][ T5418] reiserfs_write_lock+0x70/0xc0
> [ 77.775321][ T5418] reiserfs_mkdir+0x321/0x870
>
> I see that the lock is taken in reiserfs_write_lock(), while lockdep says:
>
> [ 77.710227][ T5418] but task is already holding lock:
> [ 77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at:
> reiserfs_write_lock_nested+0x4a/0xb0
>
> which is in a different place, I believe here:
>
> int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
> /* Path to the pasted item. */
> [...]
>
> depth = reiserfs_write_unlock_nested(sb);
> dquot_free_space_nodirty(inode, pasted_size);
> reiserfs_write_lock_nested(sb, depth);
> return retval;
> }
>
> This is called by reiserfs_add_entry(), which is called by
> reiserfs_create() (it is in the lockdep trace). After returning to
> reiserfs_create(), d_instantiate_new() is called.
>
> I don't know exactly, I take the part that the lock is held. But if it
> is held, how d_instantiate_new() can be executed in another task?
>
> static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
> struct dentry *dentry, umode_t mode, bool excl)
> {
>
> [...]
>
> reiserfs_write_lock(dir->i_sb);
>
> retval = journal_begin(&th, dir->i_sb, jbegin_count);
>
> [...]
>
> d_instantiate_new(dentry, inode);
> retval = journal_end(&th);
>
> out_failed:
> reiserfs_write_unlock(dir->i_sb);
>
> If the lock is held, the scenario lockdep describes cannot happen. Any
> thoughts?
>
> Thanks
>
> Roberto
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-05-05 20:51 ` syzbot
2023-05-05 21:36 ` Paul Moore
@ 2023-06-01 20:19 ` Roberto Sassu
2023-06-01 20:38 ` syzbot
2023-06-01 20:30 ` Roberto Sassu
2 siblings, 1 reply; 23+ messages in thread
From: Roberto Sassu @ 2023-06-01 20:19 UTC (permalink / raw)
To: syzbot, hdanton, linux-fsdevel, linux-kernel, paul,
reiserfs-devel, roberto.sassu, syzkaller-bugs, Jan Kara,
Jeff Mahoney
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
--- 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);
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-05-05 20:51 ` syzbot
2023-05-05 21:36 ` Paul Moore
2023-06-01 20:19 ` Roberto Sassu
@ 2023-06-01 20:30 ` Roberto Sassu
2023-06-01 20:57 ` syzbot
2 siblings, 1 reply; 23+ messages in thread
From: Roberto Sassu @ 2023-06-01 20:30 UTC (permalink / raw)
To: syzbot, hdanton, linux-fsdevel, linux-kernel, paul,
reiserfs-devel, roberto.sassu, syzkaller-bugs
[-- 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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-06-01 20:19 ` Roberto Sassu
@ 2023-06-01 20:38 ` syzbot
0 siblings, 0 replies; 23+ messages in thread
From: syzbot @ 2023-06-01 20:38 UTC (permalink / raw)
To: hdanton, jack, jeffm, linux-fsdevel, linux-kernel, paul,
reiserfs-devel, roberto.sassu, roberto.sassu, syzkaller-bugs
Hello,
syzbot tried to test the proposed patch but the build/boot failed:
failed to apply patch:
checking file fs/reiserfs/namei.c
patch: **** unexpected end of file in patch
Tested on:
commit: 929ed21d Merge tag '6.4-rc4-smb3-client-fixes' of git:..
git tree: upstream
dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
compiler:
patch: https://syzkaller.appspot.com/x/patch.diff?x=10efc079280000
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-06-01 20:30 ` Roberto Sassu
@ 2023-06-01 20:57 ` syzbot
0 siblings, 0 replies; 23+ messages in thread
From: syzbot @ 2023-06-01 20:57 UTC (permalink / raw)
To: hdanton, linux-fsdevel, linux-kernel, paul, reiserfs-devel,
roberto.sassu, roberto.sassu, syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-and-tested-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
Tested on:
commit: 929ed21d Merge tag '6.4-rc4-smb3-client-fixes' of git:..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16c69fed280000
kernel config: https://syzkaller.appspot.com/x/.config?x=162cf2103e4a7453
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=1159c149280000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-05-31 9:49 ` Roberto Sassu
(?)
(?)
@ 2023-06-01 21:22 ` Jeff Mahoney
2023-06-02 7:20 ` Roberto Sassu
-1 siblings, 1 reply; 23+ messages in thread
From: Jeff Mahoney @ 2023-06-01 21:22 UTC (permalink / raw)
To: Roberto Sassu, Paul Moore, syzbot
Cc: hdanton, linux-fsdevel, linux-kernel, reiserfs-devel,
roberto.sassu, syzkaller-bugs, peterz, mingo, will
On 5/31/23 05:49, Roberto Sassu wrote:
> On 5/5/2023 11:36 PM, Paul Moore wrote:
>> On Fri, May 5, 2023 at 4:51 PM syzbot
>> <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> 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
>>
>> I don't think Roberto's patch identified above is the actual root
>> cause of this problem as reiserfs_xattr_set_handle() is called in
>> reiserfs_security_write() both before and after the patch. However,
>> due to some bad logic in reiserfs_security_write() which Roberto
>> corrected, I'm thinking that it is possible this code is being
>> exercised for the first time and syzbot is starting to trigger a
>> locking issue in the reiserfs code ... ?
>
> + Jan, Jeff (which basically restructured the lock)
>
> + Petr, Ingo, Will
>
> I involve the lockdep experts, to get a bit of help on this.
Yep, looks like that's been broken since it was added in 2009. Since
there can't be any users of it, it'd make sense to drop the security
xattr support from reiserfs entirely.
> First of all, the lockdep warning is trivial to reproduce:
>
> # dd if=/dev/zero of=reiserfs.img bs=1M count=100
> # losetup -f --show reiserfs.img
> /dev/loop0
> # mkfs.reiserfs /dev/loop0
> # mount /dev/loop0 /mnt/
> # touch file0
>
> In the testing system, Smack is the major LSM.
>
> Ok, so the warning here is clear:
>
> https://syzkaller.appspot.com/x/log.txt?x=12403182280000
>
> However, I was looking if that can really happen. From this:
>
> [ 77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
> [ 77.753772][ T5418] lock_acquire+0x23e/0x630
> [ 77.758792][ T5418] __mutex_lock_common+0x1d8/0x2530
> [ 77.764504][ T5418] mutex_lock_nested+0x1b/0x20
> [ 77.769868][ T5418] reiserfs_write_lock+0x70/0xc0
> [ 77.775321][ T5418] reiserfs_mkdir+0x321/0x870
>
> I see that the lock is taken in reiserfs_write_lock(), while lockdep says:
>
> [ 77.710227][ T5418] but task is already holding lock:
> [ 77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at:
> reiserfs_write_lock_nested+0x4a/0xb0
>
> which is in a different place, I believe here:
>
> int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
> /* Path to the pasted item. */
> [...]
>
> depth = reiserfs_write_unlock_nested(sb);
> dquot_free_space_nodirty(inode, pasted_size);
> reiserfs_write_lock_nested(sb, depth);
> return retval;
> }
>
> This is called by reiserfs_add_entry(), which is called by
> reiserfs_create() (it is in the lockdep trace). After returning to
> reiserfs_create(), d_instantiate_new() is called.
>
> I don't know exactly, I take the part that the lock is held. But if it
> is held, how d_instantiate_new() can be executed in another task?
>
> static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
> struct dentry *dentry, umode_t mode, bool excl)
> {
>
> [...]
>
> reiserfs_write_lock(dir->i_sb);
>
> retval = journal_begin(&th, dir->i_sb, jbegin_count);
>
> [...]
>
> d_instantiate_new(dentry, inode);
> retval = journal_end(&th);
>
> out_failed:
> reiserfs_write_unlock(dir->i_sb);
>
> If the lock is held, the scenario lockdep describes cannot happen. Any
> thoughts?
It's important to understand that the reiserfs write lock was added as a
subsystem-specific replacement for the BKL. Given that reiserfs was
dying already back then, it made more sense from a time management
perspective to emulate that behavior internally rather than use new
locking when practically nobody cared anymore.
See reiserfs_write_unlock_nested and reiserfs_write_lock_nested paired
throughout the code. It drops the lock when it passes a point where
it's likely to schedule, just like the BKL would have.
Yes, it's a mess. Just let it die quietly.
-Jeff
--
Jeff Mahoney
VP Engineering, Linux Systems
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-06-01 21:22 ` Jeff Mahoney
@ 2023-06-02 7:20 ` Roberto Sassu
0 siblings, 0 replies; 23+ messages in thread
From: Roberto Sassu @ 2023-06-02 7:20 UTC (permalink / raw)
To: Jeff Mahoney, Paul Moore, syzbot
Cc: hdanton, linux-fsdevel, linux-kernel, reiserfs-devel,
roberto.sassu, syzkaller-bugs, peterz, mingo, will, Jan Kara
On Thu, 2023-06-01 at 17:22 -0400, Jeff Mahoney wrote:
> On 5/31/23 05:49, Roberto Sassu wrote:
> > On 5/5/2023 11:36 PM, Paul Moore wrote:
> > > On Fri, May 5, 2023 at 4:51 PM syzbot
> > > <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> 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
> > >
> > > I don't think Roberto's patch identified above is the actual root
> > > cause of this problem as reiserfs_xattr_set_handle() is called in
> > > reiserfs_security_write() both before and after the patch. However,
> > > due to some bad logic in reiserfs_security_write() which Roberto
> > > corrected, I'm thinking that it is possible this code is being
> > > exercised for the first time and syzbot is starting to trigger a
> > > locking issue in the reiserfs code ... ?
> >
> > + Jan, Jeff (which basically restructured the lock)
> >
> > + Petr, Ingo, Will
Peter, clearly (sorry!)
> I involve the lockdep experts, to get a bit of help on this.
>
> Yep, looks like that's been broken since it was added in 2009. Since
> there can't be any users of it, it'd make sense to drop the security
> xattr support from reiserfs entirely.
Thanks, Jeff. Will make a patch to implement your suggestion.
Meanwhile, I learned how to read lockdep a bit better. The following
format could have helped me to understand it more quickly. The proposal
is simply to change #n to CPU#n at the top of the trace, define labels
L#n for the locks, and add them where effectively are held.
[ 77.746561][ T5418] -> CPU1 (&sbi->lock){+.+.}-{3:3}:
[ 77.753772][ T5418] lock_acquire+0x23e/0x630
[ 77.758792][ T5418] __mutex_lock_common+0x1d8/0x2530
[ 77.764504][ T5418] (L3) mutex_lock_nested+0x1b/0x20
[ 77.769868][ T5418] reiserfs_write_lock+0x70/0xc0
[ 77.775321][ T5418] reiserfs_mkdir+0x321/0x870
[ 77.780509][ T5418] (L2) open_xa_dir+0x259/0x540
[ 77.785440][ T5418] xattr_lookup+0x17/0x210
[ 77.790378][ T5418] reiserfs_xattr_set_handle+0xda/0xc80
[ 77.796448][ T5418] reiserfs_security_write+0x134/0x190
[ 77.802416][ T5418] reiserfs_new_inode+0x13bf/0x1a90
[ 77.808124][ T5418] reiserfs_create+0x3b1/0x680
[ 77.813399][ T5418] path_openat+0xf1e/0x2c10
[ 77.818415][ T5418] do_filp_open+0x22a/0x440
[ 77.823433][ T5418] do_sys_openat2+0x10f/0x430
[ 77.828624][ T5418] __x64_sys_creat+0x11e/0x160
[ 77.833905][ T5418] do_syscall_64+0x41/0xc0
[ 77.838926][ T5418] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 77.845514][ T5418]
[ 77.845514][ T5418] -> CPU0 (&type->i_mutex_dir_key#8/3){+.+.}-{3:3}:
[ 77.854118][ T5418] validate_chain+0x166b/0x58e0
[ 77.859488][ T5418] __lock_acquire+0x125b/0x1f80
[ 77.864853][ T5418] lock_acquire+0x23e/0x630
[ 77.869909][ T5418] (L4) down_write_nested+0x3d/0x50
[ 77.875186][ T5418] open_xa_dir+0x134/0x540
[ 77.880117][ T5418] xattr_lookup+0x17/0x210
[ 77.885050][ T5418] reiserfs_xattr_get+0xe1/0x4a0
[ 77.890501][ T5418] __vfs_getxattr+0x2fe/0x350
[ 77.895802][ T5418] smk_fetch+0x98/0xf0
[ 77.900382][ T5418] smack_d_instantiate+0x5d5/0xa20
[ 77.906018][ T5418] security_d_instantiate+0x6b/0xb0
[ 77.911736][ T5418] d_instantiate_new+0x5e/0xe0
[ 77.917013][ T5418] (L1) reiserfs_create+0x5ee/0x680
[ 77.922293][ T5418] path_openat+0xf1e/0x2c10
[ 77.927308][ T5418] do_filp_open+0x22a/0x440
[ 77.932330][ T5418] do_sys_openat2+0x10f/0x430
[ 77.937515][ T5418] __x64_sys_creat+0x11e/0x160
[ 77.942874][ T5418] do_syscall_64+0x41/0xc0
[ 77.947796][ T5418] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 77.954200][ T5418]
[ 77.954200][ T5418] other info that might help us debug this:
[ 77.954200][ T5418]
[ 77.964508][ T5418] Possible unsafe locking scenario:
[ 77.964508][ T5418]
[ 77.972034][ T5418] CPU0 CPU1
[ 77.977394][ T5418] ---- ----
[ 77.982748][ T5418] L1: lock(&sbi->lock);
[ 77.986726][ T5418] L2: lock(&type->i_mutex_dir_key#8/3);
[ 77.994618][ T5418] L3: lock(&sbi->lock);
[ 78.001118][ T5418] L4: lock(&type->i_mutex_dir_key#8/3);
Thanks
Roberto
> > First of all, the lockdep warning is trivial to reproduce:
> >
> > # dd if=/dev/zero of=reiserfs.img bs=1M count=100
> > # losetup -f --show reiserfs.img
> > /dev/loop0
> > # mkfs.reiserfs /dev/loop0
> > # mount /dev/loop0 /mnt/
> > # touch file0
> >
> > In the testing system, Smack is the major LSM.
> >
> > Ok, so the warning here is clear:
> >
> > https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> >
> > However, I was looking if that can really happen. From this:
> >
> > [ 77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
> > [ 77.753772][ T5418] lock_acquire+0x23e/0x630
> > [ 77.758792][ T5418] __mutex_lock_common+0x1d8/0x2530
> > [ 77.764504][ T5418] mutex_lock_nested+0x1b/0x20
> > [ 77.769868][ T5418] reiserfs_write_lock+0x70/0xc0
> > [ 77.775321][ T5418] reiserfs_mkdir+0x321/0x870
> >
> > I see that the lock is taken in reiserfs_write_lock(), while lockdep says:
> >
> > [ 77.710227][ T5418] but task is already holding lock:
> > [ 77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at:
> > reiserfs_write_lock_nested+0x4a/0xb0
> >
> > which is in a different place, I believe here:
> >
> > int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
> > /* Path to the pasted item. */
> > [...]
> >
> > depth = reiserfs_write_unlock_nested(sb);
> > dquot_free_space_nodirty(inode, pasted_size);
> > reiserfs_write_lock_nested(sb, depth);
> > return retval;
> > }
> >
> > This is called by reiserfs_add_entry(), which is called by
> > reiserfs_create() (it is in the lockdep trace). After returning to
> > reiserfs_create(), d_instantiate_new() is called.
> >
> > I don't know exactly, I take the part that the lock is held. But if it
> > is held, how d_instantiate_new() can be executed in another task?
> >
> > static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
> > struct dentry *dentry, umode_t mode, bool excl)
> > {
> >
> > [...]
> >
> > reiserfs_write_lock(dir->i_sb);
> >
> > retval = journal_begin(&th, dir->i_sb, jbegin_count);
> >
> > [...]
> >
> > d_instantiate_new(dentry, inode);
> > retval = journal_end(&th);
> >
> > out_failed:
> > reiserfs_write_unlock(dir->i_sb);
> >
> > If the lock is held, the scenario lockdep describes cannot happen. Any
> > thoughts?
>
> It's important to understand that the reiserfs write lock was added as a
> subsystem-specific replacement for the BKL. Given that reiserfs was
> dying already back then, it made more sense from a time management
> perspective to emulate that behavior internally rather than use new
> locking when practically nobody cared anymore.
>
> See reiserfs_write_unlock_nested and reiserfs_write_lock_nested paired
> throughout the code. It drops the lock when it passes a point where
> it's likely to schedule, just like the BKL would have.
>
> Yes, it's a mess. Just let it die quietly.
>
> -Jeff
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
@ 2023-06-02 7:20 ` Roberto Sassu
0 siblings, 0 replies; 23+ messages in thread
From: Roberto Sassu @ 2023-06-02 7:20 UTC (permalink / raw)
To: Jeff Mahoney, Paul Moore, syzbot
Cc: hdanton, linux-fsdevel, linux-kernel, reiserfs-devel,
roberto.sassu, syzkaller-bugs, peterz, mingo, will, Jan Kara
On Thu, 2023-06-01 at 17:22 -0400, Jeff Mahoney wrote:
> On 5/31/23 05:49, Roberto Sassu wrote:
> > On 5/5/2023 11:36 PM, Paul Moore wrote:
> > > On Fri, May 5, 2023 at 4:51 PM syzbot
> > > <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> 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
> > >
> > > I don't think Roberto's patch identified above is the actual root
> > > cause of this problem as reiserfs_xattr_set_handle() is called in
> > > reiserfs_security_write() both before and after the patch. However,
> > > due to some bad logic in reiserfs_security_write() which Roberto
> > > corrected, I'm thinking that it is possible this code is being
> > > exercised for the first time and syzbot is starting to trigger a
> > > locking issue in the reiserfs code ... ?
> >
> > + Jan, Jeff (which basically restructured the lock)
> >
> > + Petr, Ingo, Will
Peter, clearly (sorry!)
> I involve the lockdep experts, to get a bit of help on this.
>
> Yep, looks like that's been broken since it was added in 2009. Since
> there can't be any users of it, it'd make sense to drop the security
> xattr support from reiserfs entirely.
Thanks, Jeff. Will make a patch to implement your suggestion.
Meanwhile, I learned how to read lockdep a bit better. The following
format could have helped me to understand it more quickly. The proposal
is simply to change #n to CPU#n at the top of the trace, define labels
L#n for the locks, and add them where effectively are held.
[ 77.746561][ T5418] -> CPU1 (&sbi->lock){+.+.}-{3:3}:
[ 77.753772][ T5418] lock_acquire+0x23e/0x630
[ 77.758792][ T5418] __mutex_lock_common+0x1d8/0x2530
[ 77.764504][ T5418] (L3) mutex_lock_nested+0x1b/0x20
[ 77.769868][ T5418] reiserfs_write_lock+0x70/0xc0
[ 77.775321][ T5418] reiserfs_mkdir+0x321/0x870
[ 77.780509][ T5418] (L2) open_xa_dir+0x259/0x540
[ 77.785440][ T5418] xattr_lookup+0x17/0x210
[ 77.790378][ T5418] reiserfs_xattr_set_handle+0xda/0xc80
[ 77.796448][ T5418] reiserfs_security_write+0x134/0x190
[ 77.802416][ T5418] reiserfs_new_inode+0x13bf/0x1a90
[ 77.808124][ T5418] reiserfs_create+0x3b1/0x680
[ 77.813399][ T5418] path_openat+0xf1e/0x2c10
[ 77.818415][ T5418] do_filp_open+0x22a/0x440
[ 77.823433][ T5418] do_sys_openat2+0x10f/0x430
[ 77.828624][ T5418] __x64_sys_creat+0x11e/0x160
[ 77.833905][ T5418] do_syscall_64+0x41/0xc0
[ 77.838926][ T5418] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 77.845514][ T5418]
[ 77.845514][ T5418] -> CPU0 (&type->i_mutex_dir_key#8/3){+.+.}-{3:3}:
[ 77.854118][ T5418] validate_chain+0x166b/0x58e0
[ 77.859488][ T5418] __lock_acquire+0x125b/0x1f80
[ 77.864853][ T5418] lock_acquire+0x23e/0x630
[ 77.869909][ T5418] (L4) down_write_nested+0x3d/0x50
[ 77.875186][ T5418] open_xa_dir+0x134/0x540
[ 77.880117][ T5418] xattr_lookup+0x17/0x210
[ 77.885050][ T5418] reiserfs_xattr_get+0xe1/0x4a0
[ 77.890501][ T5418] __vfs_getxattr+0x2fe/0x350
[ 77.895802][ T5418] smk_fetch+0x98/0xf0
[ 77.900382][ T5418] smack_d_instantiate+0x5d5/0xa20
[ 77.906018][ T5418] security_d_instantiate+0x6b/0xb0
[ 77.911736][ T5418] d_instantiate_new+0x5e/0xe0
[ 77.917013][ T5418] (L1) reiserfs_create+0x5ee/0x680
[ 77.922293][ T5418] path_openat+0xf1e/0x2c10
[ 77.927308][ T5418] do_filp_open+0x22a/0x440
[ 77.932330][ T5418] do_sys_openat2+0x10f/0x430
[ 77.937515][ T5418] __x64_sys_creat+0x11e/0x160
[ 77.942874][ T5418] do_syscall_64+0x41/0xc0
[ 77.947796][ T5418] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 77.954200][ T5418]
[ 77.954200][ T5418] other info that might help us debug this:
[ 77.954200][ T5418]
[ 77.964508][ T5418] Possible unsafe locking scenario:
[ 77.964508][ T5418]
[ 77.972034][ T5418] CPU0 CPU1
[ 77.977394][ T5418] ---- ----
[ 77.982748][ T5418] L1: lock(&sbi->lock);
[ 77.986726][ T5418] L2: lock(&type->i_mutex_dir_key#8/3);
[ 77.994618][ T5418] L3: lock(&sbi->lock);
[ 78.001118][ T5418] L4: lock(&type->i_mutex_dir_key#8/3);
Thanks
Roberto
> > First of all, the lockdep warning is trivial to reproduce:
> >
> > # dd if=/dev/zero of=reiserfs.img bs=1M count=100
> > # losetup -f --show reiserfs.img
> > /dev/loop0
> > # mkfs.reiserfs /dev/loop0
> > # mount /dev/loop0 /mnt/
> > # touch file0
> >
> > In the testing system, Smack is the major LSM.
> >
> > Ok, so the warning here is clear:
> >
> > https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> >
> > However, I was looking if that can really happen. From this:
> >
> > [ 77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
> > [ 77.753772][ T5418] lock_acquire+0x23e/0x630
> > [ 77.758792][ T5418] __mutex_lock_common+0x1d8/0x2530
> > [ 77.764504][ T5418] mutex_lock_nested+0x1b/0x20
> > [ 77.769868][ T5418] reiserfs_write_lock+0x70/0xc0
> > [ 77.775321][ T5418] reiserfs_mkdir+0x321/0x870
> >
> > I see that the lock is taken in reiserfs_write_lock(), while lockdep says:
> >
> > [ 77.710227][ T5418] but task is already holding lock:
> > [ 77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at:
> > reiserfs_write_lock_nested+0x4a/0xb0
> >
> > which is in a different place, I believe here:
> >
> > int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
> > /* Path to the pasted item. */
> > [...]
> >
> > depth = reiserfs_write_unlock_nested(sb);
> > dquot_free_space_nodirty(inode, pasted_size);
> > reiserfs_write_lock_nested(sb, depth);
> > return retval;
> > }
> >
> > This is called by reiserfs_add_entry(), which is called by
> > reiserfs_create() (it is in the lockdep trace). After returning to
> > reiserfs_create(), d_instantiate_new() is called.
> >
> > I don't know exactly, I take the part that the lock is held. But if it
> > is held, how d_instantiate_new() can be executed in another task?
> >
> > static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
> > struct dentry *dentry, umode_t mode, bool excl)
> > {
> >
> > [...]
> >
> > reiserfs_write_lock(dir->i_sb);
> >
> > retval = journal_begin(&th, dir->i_sb, jbegin_count);
> >
> > [...]
> >
> > d_instantiate_new(dentry, inode);
> > retval = journal_end(&th);
> >
> > out_failed:
> > reiserfs_write_unlock(dir->i_sb);
> >
> > If the lock is held, the scenario lockdep describes cannot happen. Any
> > thoughts?
>
> It's important to understand that the reiserfs write lock was added as a
> subsystem-specific replacement for the BKL. Given that reiserfs was
> dying already back then, it made more sense from a time management
> perspective to emulate that behavior internally rather than use new
> locking when practically nobody cared anymore.
>
> See reiserfs_write_unlock_nested and reiserfs_write_lock_nested paired
> throughout the code. It drops the lock when it passes a point where
> it's likely to schedule, just like the BKL would have.
>
> Yes, it's a mess. Just let it die quietly.
>
> -Jeff
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-06-02 7:20 ` Roberto Sassu
(?)
@ 2023-06-02 8:56 ` Roberto Sassu
2023-06-02 9:17 ` syzbot
-1 siblings, 1 reply; 23+ messages in thread
From: Roberto Sassu @ 2023-06-02 8:56 UTC (permalink / raw)
To: Jeff Mahoney, Paul Moore, syzbot
Cc: hdanton, linux-fsdevel, linux-kernel, reiserfs-devel,
roberto.sassu, syzkaller-bugs, peterz, mingo, will, Jan Kara
[-- Attachment #1: Type: text/plain, Size: 2798 bytes --]
On Fri, 2023-06-02 at 09:20 +0200, Roberto Sassu wrote:
> On Thu, 2023-06-01 at 17:22 -0400, Jeff Mahoney wrote:
> > On 5/31/23 05:49, Roberto Sassu wrote:
> > > On 5/5/2023 11:36 PM, Paul Moore wrote:
> > > > On Fri, May 5, 2023 at 4:51 PM syzbot
> > > > <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> 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
> > > >
> > > > I don't think Roberto's patch identified above is the actual root
> > > > cause of this problem as reiserfs_xattr_set_handle() is called in
> > > > reiserfs_security_write() both before and after the patch. However,
> > > > due to some bad logic in reiserfs_security_write() which Roberto
> > > > corrected, I'm thinking that it is possible this code is being
> > > > exercised for the first time and syzbot is starting to trigger a
> > > > locking issue in the reiserfs code ... ?
> > >
> > > + Jan, Jeff (which basically restructured the lock)
> > >
> > > + Petr, Ingo, Will
>
> Peter, clearly (sorry!)
>
> > I involve the lockdep experts, to get a bit of help on this.
> >
> > Yep, looks like that's been broken since it was added in 2009. Since
> > there can't be any users of it, it'd make sense to drop the security
> > xattr support from reiserfs entirely.
>
> Thanks, Jeff. Will make a patch to implement your suggestion.
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git next
[-- Attachment #2: 0001-reiserfs-Disable-security-xattr-initialization-since.patch --]
[-- Type: text/x-patch, Size: 2210 bytes --]
From a31f5b09e39e5a964457b0a52aa9c437a0bf7f63 Mon Sep 17 00:00:00 2001
From: Roberto Sassu <roberto.sassu@huawei.com>
Date: Fri, 2 Jun 2023 10:10:28 +0200
Subject: [PATCH] reiserfs: Disable security xattr initialization 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.
Since the bug in security xattr initialization was introduced since the
beginning, there cannot be any user of this feature. Instead of trying to
fix the locking dependency, which was already challenging to convert from
BLK, just disable the feature.
However, still keep the security xattr handler, since it was introduced
earlier, and users might have manually added xattrs.
Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
Suggested-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
fs/reiserfs/super.c | 2 ++
fs/reiserfs/xattr_security.c | 3 +++
2 files changed, 5 insertions(+)
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 929acce6e73..262041b87cd 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -1654,6 +1654,8 @@ 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");
+ reiserfs_warning(NULL, "", "initializing security xattrs never worked, disable it");
+
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..4037a998dbf 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;
+ /* See warning in read_super_block(). */
+ return 0;
+
/* Don't add selinux attributes on xattrs - they'll never get used */
if (IS_PRIVATE(dir))
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-06-02 8:56 ` Roberto Sassu
@ 2023-06-02 9:17 ` syzbot
2023-06-02 16:18 ` Roberto Sassu
0 siblings, 1 reply; 23+ messages in thread
From: syzbot @ 2023-06-02 9:17 UTC (permalink / raw)
To: hdanton, jack, jeffm, linux-fsdevel, linux-kernel, mingo, paul,
peterz, reiserfs-devel, roberto.sassu, roberto.sassu,
syzkaller-bugs, will
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-and-tested-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-06-02 9:17 ` syzbot
@ 2023-06-02 16:18 ` Roberto Sassu
2023-06-02 16:39 ` syzbot
0 siblings, 1 reply; 23+ messages in thread
From: Roberto Sassu @ 2023-06-02 16:18 UTC (permalink / raw)
To: syzbot, hdanton, jack, jeffm, linux-fsdevel, linux-kernel, mingo,
paul, peterz, reiserfs-devel, roberto.sassu, syzkaller-bugs,
will
[-- Attachment #1: Type: text/plain, Size: 932 bytes --]
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@syzkaller.appspotmail.com
>
> 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
[-- Attachment #2: 0001-reiserfs-Disable-by-default-security-xattr-init-sinc.patch --]
[-- Type: text/x-patch, Size: 2884 bytes --]
From 73bb02eb7a751c447af43d7cac7c191329b6dd55 Mon Sep 17 00:00:00 2001
From: Roberto Sassu <roberto.sassu@huawei.com>
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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
Suggested-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-06-02 16:18 ` Roberto Sassu
@ 2023-06-02 16:39 ` syzbot
0 siblings, 0 replies; 23+ messages in thread
From: syzbot @ 2023-06-02 16:39 UTC (permalink / raw)
To: hdanton, jack, jeffm, linux-fsdevel, linux-kernel, mingo, paul,
peterz, reiserfs-devel, roberto.sassu, roberto.sassu,
syzkaller-bugs, will
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-and-tested-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
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=16b47dd1280000
kernel config: https://syzkaller.appspot.com/x/.config?x=178a8c28652084e1
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=102f6ab6280000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2023-06-02 7:20 ` Roberto Sassu
@ 2023-06-02 16:46 ` Roberto Sassu
-1 siblings, 0 replies; 23+ messages in thread
From: Roberto Sassu @ 2023-06-02 16:46 UTC (permalink / raw)
To: Jeff Mahoney, Paul Moore, syzbot
Cc: hdanton, linux-fsdevel, linux-kernel, reiserfs-devel,
roberto.sassu, syzkaller-bugs, peterz, mingo, will, Jan Kara
On Fri, 2023-06-02 at 09:20 +0200, Roberto Sassu wrote:
> On Thu, 2023-06-01 at 17:22 -0400, Jeff Mahoney wrote:
> > On 5/31/23 05:49, Roberto Sassu wrote:
> > > On 5/5/2023 11:36 PM, Paul Moore wrote:
> > > > On Fri, May 5, 2023 at 4:51 PM syzbot
> > > > <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> 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
> > > >
> > > > I don't think Roberto's patch identified above is the actual root
> > > > cause of this problem as reiserfs_xattr_set_handle() is called in
> > > > reiserfs_security_write() both before and after the patch. However,
> > > > due to some bad logic in reiserfs_security_write() which Roberto
> > > > corrected, I'm thinking that it is possible this code is being
> > > > exercised for the first time and syzbot is starting to trigger a
> > > > locking issue in the reiserfs code ... ?
> > >
> > > + Jan, Jeff (which basically restructured the lock)
> > >
> > > + Petr, Ingo, Will
>
> Peter, clearly (sorry!)
>
> > I involve the lockdep experts, to get a bit of help on this.
> >
> > Yep, looks like that's been broken since it was added in 2009. Since
> > there can't be any users of it, it'd make sense to drop the security
> > xattr support from reiserfs entirely.
>
> Thanks, Jeff. Will make a patch to implement your suggestion.
Ok, I tried first to disable security xattr initialization and keep the
xattr handler.
Setting the security xattr manually triggers a lockdep warning. Even
worse, setting a trusted xattr manually triggers that too. So, not sure
how we should proceed.
Have you looked at:
https://lore.kernel.org/linux-kernel/8a48ede1-3a45-7c3c-39e9-36001ac09283@huaweicloud.com/
That silences the lockdep warning, but I'm far from saying that it
won't have any side effect...
Thanks
Roberto
> Meanwhile, I learned how to read lockdep a bit better. The following
> format could have helped me to understand it more quickly. The proposal
> is simply to change #n to CPU#n at the top of the trace, define labels
> L#n for the locks, and add them where effectively are held.
>
> [ 77.746561][ T5418] -> CPU1 (&sbi->lock){+.+.}-{3:3}:
> [ 77.753772][ T5418] lock_acquire+0x23e/0x630
> [ 77.758792][ T5418] __mutex_lock_common+0x1d8/0x2530
> [ 77.764504][ T5418] (L3) mutex_lock_nested+0x1b/0x20
> [ 77.769868][ T5418] reiserfs_write_lock+0x70/0xc0
> [ 77.775321][ T5418] reiserfs_mkdir+0x321/0x870
> [ 77.780509][ T5418] (L2) open_xa_dir+0x259/0x540
> [ 77.785440][ T5418] xattr_lookup+0x17/0x210
> [ 77.790378][ T5418] reiserfs_xattr_set_handle+0xda/0xc80
> [ 77.796448][ T5418] reiserfs_security_write+0x134/0x190
> [ 77.802416][ T5418] reiserfs_new_inode+0x13bf/0x1a90
> [ 77.808124][ T5418] reiserfs_create+0x3b1/0x680
> [ 77.813399][ T5418] path_openat+0xf1e/0x2c10
> [ 77.818415][ T5418] do_filp_open+0x22a/0x440
> [ 77.823433][ T5418] do_sys_openat2+0x10f/0x430
> [ 77.828624][ T5418] __x64_sys_creat+0x11e/0x160
> [ 77.833905][ T5418] do_syscall_64+0x41/0xc0
> [ 77.838926][ T5418] entry_SYSCALL_64_after_hwframe+0x63/0xcd
> [ 77.845514][ T5418]
> [ 77.845514][ T5418] -> CPU0 (&type->i_mutex_dir_key#8/3){+.+.}-{3:3}:
> [ 77.854118][ T5418] validate_chain+0x166b/0x58e0
> [ 77.859488][ T5418] __lock_acquire+0x125b/0x1f80
> [ 77.864853][ T5418] lock_acquire+0x23e/0x630
> [ 77.869909][ T5418] (L4) down_write_nested+0x3d/0x50
> [ 77.875186][ T5418] open_xa_dir+0x134/0x540
> [ 77.880117][ T5418] xattr_lookup+0x17/0x210
> [ 77.885050][ T5418] reiserfs_xattr_get+0xe1/0x4a0
> [ 77.890501][ T5418] __vfs_getxattr+0x2fe/0x350
> [ 77.895802][ T5418] smk_fetch+0x98/0xf0
> [ 77.900382][ T5418] smack_d_instantiate+0x5d5/0xa20
> [ 77.906018][ T5418] security_d_instantiate+0x6b/0xb0
> [ 77.911736][ T5418] d_instantiate_new+0x5e/0xe0
> [ 77.917013][ T5418] (L1) reiserfs_create+0x5ee/0x680
> [ 77.922293][ T5418] path_openat+0xf1e/0x2c10
> [ 77.927308][ T5418] do_filp_open+0x22a/0x440
> [ 77.932330][ T5418] do_sys_openat2+0x10f/0x430
> [ 77.937515][ T5418] __x64_sys_creat+0x11e/0x160
> [ 77.942874][ T5418] do_syscall_64+0x41/0xc0
> [ 77.947796][ T5418] entry_SYSCALL_64_after_hwframe+0x63/0xcd
> [ 77.954200][ T5418]
> [ 77.954200][ T5418] other info that might help us debug this:
> [ 77.954200][ T5418]
> [ 77.964508][ T5418] Possible unsafe locking scenario:
> [ 77.964508][ T5418]
> [ 77.972034][ T5418] CPU0 CPU1
> [ 77.977394][ T5418] ---- ----
> [ 77.982748][ T5418] L1: lock(&sbi->lock);
> [ 77.986726][ T5418] L2: lock(&type->i_mutex_dir_key#8/3);
> [ 77.994618][ T5418] L3: lock(&sbi->lock);
> [ 78.001118][ T5418] L4: lock(&type->i_mutex_dir_key#8/3);
>
> Thanks
>
> Roberto
>
> > > First of all, the lockdep warning is trivial to reproduce:
> > >
> > > # dd if=/dev/zero of=reiserfs.img bs=1M count=100
> > > # losetup -f --show reiserfs.img
> > > /dev/loop0
> > > # mkfs.reiserfs /dev/loop0
> > > # mount /dev/loop0 /mnt/
> > > # touch file0
> > >
> > > In the testing system, Smack is the major LSM.
> > >
> > > Ok, so the warning here is clear:
> > >
> > > https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> > >
> > > However, I was looking if that can really happen. From this:
> > >
> > > [ 77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
> > > [ 77.753772][ T5418] lock_acquire+0x23e/0x630
> > > [ 77.758792][ T5418] __mutex_lock_common+0x1d8/0x2530
> > > [ 77.764504][ T5418] mutex_lock_nested+0x1b/0x20
> > > [ 77.769868][ T5418] reiserfs_write_lock+0x70/0xc0
> > > [ 77.775321][ T5418] reiserfs_mkdir+0x321/0x870
> > >
> > > I see that the lock is taken in reiserfs_write_lock(), while lockdep says:
> > >
> > > [ 77.710227][ T5418] but task is already holding lock:
> > > [ 77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at:
> > > reiserfs_write_lock_nested+0x4a/0xb0
> > >
> > > which is in a different place, I believe here:
> > >
> > > int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
> > > /* Path to the pasted item. */
> > > [...]
> > >
> > > depth = reiserfs_write_unlock_nested(sb);
> > > dquot_free_space_nodirty(inode, pasted_size);
> > > reiserfs_write_lock_nested(sb, depth);
> > > return retval;
> > > }
> > >
> > > This is called by reiserfs_add_entry(), which is called by
> > > reiserfs_create() (it is in the lockdep trace). After returning to
> > > reiserfs_create(), d_instantiate_new() is called.
> > >
> > > I don't know exactly, I take the part that the lock is held. But if it
> > > is held, how d_instantiate_new() can be executed in another task?
> > >
> > > static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
> > > struct dentry *dentry, umode_t mode, bool excl)
> > > {
> > >
> > > [...]
> > >
> > > reiserfs_write_lock(dir->i_sb);
> > >
> > > retval = journal_begin(&th, dir->i_sb, jbegin_count);
> > >
> > > [...]
> > >
> > > d_instantiate_new(dentry, inode);
> > > retval = journal_end(&th);
> > >
> > > out_failed:
> > > reiserfs_write_unlock(dir->i_sb);
> > >
> > > If the lock is held, the scenario lockdep describes cannot happen. Any
> > > thoughts?
> >
> > It's important to understand that the reiserfs write lock was added as a
> > subsystem-specific replacement for the BKL. Given that reiserfs was
> > dying already back then, it made more sense from a time management
> > perspective to emulate that behavior internally rather than use new
> > locking when practically nobody cared anymore.
> >
> > See reiserfs_write_unlock_nested and reiserfs_write_lock_nested paired
> > throughout the code. It drops the lock when it passes a point where
> > it's likely to schedule, just like the BKL would have.
> >
> > Yes, it's a mess. Just let it die quietly.
> >
> > -Jeff
> >
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
@ 2023-06-02 16:46 ` Roberto Sassu
0 siblings, 0 replies; 23+ messages in thread
From: Roberto Sassu @ 2023-06-02 16:46 UTC (permalink / raw)
To: Jeff Mahoney, Paul Moore, syzbot
Cc: hdanton, linux-fsdevel, linux-kernel, reiserfs-devel,
roberto.sassu, syzkaller-bugs, peterz, mingo, will, Jan Kara
On Fri, 2023-06-02 at 09:20 +0200, Roberto Sassu wrote:
> On Thu, 2023-06-01 at 17:22 -0400, Jeff Mahoney wrote:
> > On 5/31/23 05:49, Roberto Sassu wrote:
> > > On 5/5/2023 11:36 PM, Paul Moore wrote:
> > > > On Fri, May 5, 2023 at 4:51 PM syzbot
> > > > <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> 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
> > > >
> > > > I don't think Roberto's patch identified above is the actual root
> > > > cause of this problem as reiserfs_xattr_set_handle() is called in
> > > > reiserfs_security_write() both before and after the patch. However,
> > > > due to some bad logic in reiserfs_security_write() which Roberto
> > > > corrected, I'm thinking that it is possible this code is being
> > > > exercised for the first time and syzbot is starting to trigger a
> > > > locking issue in the reiserfs code ... ?
> > >
> > > + Jan, Jeff (which basically restructured the lock)
> > >
> > > + Petr, Ingo, Will
>
> Peter, clearly (sorry!)
>
> > I involve the lockdep experts, to get a bit of help on this.
> >
> > Yep, looks like that's been broken since it was added in 2009. Since
> > there can't be any users of it, it'd make sense to drop the security
> > xattr support from reiserfs entirely.
>
> Thanks, Jeff. Will make a patch to implement your suggestion.
Ok, I tried first to disable security xattr initialization and keep the
xattr handler.
Setting the security xattr manually triggers a lockdep warning. Even
worse, setting a trusted xattr manually triggers that too. So, not sure
how we should proceed.
Have you looked at:
https://lore.kernel.org/linux-kernel/8a48ede1-3a45-7c3c-39e9-36001ac09283@huaweicloud.com/
That silences the lockdep warning, but I'm far from saying that it
won't have any side effect...
Thanks
Roberto
> Meanwhile, I learned how to read lockdep a bit better. The following
> format could have helped me to understand it more quickly. The proposal
> is simply to change #n to CPU#n at the top of the trace, define labels
> L#n for the locks, and add them where effectively are held.
>
> [ 77.746561][ T5418] -> CPU1 (&sbi->lock){+.+.}-{3:3}:
> [ 77.753772][ T5418] lock_acquire+0x23e/0x630
> [ 77.758792][ T5418] __mutex_lock_common+0x1d8/0x2530
> [ 77.764504][ T5418] (L3) mutex_lock_nested+0x1b/0x20
> [ 77.769868][ T5418] reiserfs_write_lock+0x70/0xc0
> [ 77.775321][ T5418] reiserfs_mkdir+0x321/0x870
> [ 77.780509][ T5418] (L2) open_xa_dir+0x259/0x540
> [ 77.785440][ T5418] xattr_lookup+0x17/0x210
> [ 77.790378][ T5418] reiserfs_xattr_set_handle+0xda/0xc80
> [ 77.796448][ T5418] reiserfs_security_write+0x134/0x190
> [ 77.802416][ T5418] reiserfs_new_inode+0x13bf/0x1a90
> [ 77.808124][ T5418] reiserfs_create+0x3b1/0x680
> [ 77.813399][ T5418] path_openat+0xf1e/0x2c10
> [ 77.818415][ T5418] do_filp_open+0x22a/0x440
> [ 77.823433][ T5418] do_sys_openat2+0x10f/0x430
> [ 77.828624][ T5418] __x64_sys_creat+0x11e/0x160
> [ 77.833905][ T5418] do_syscall_64+0x41/0xc0
> [ 77.838926][ T5418] entry_SYSCALL_64_after_hwframe+0x63/0xcd
> [ 77.845514][ T5418]
> [ 77.845514][ T5418] -> CPU0 (&type->i_mutex_dir_key#8/3){+.+.}-{3:3}:
> [ 77.854118][ T5418] validate_chain+0x166b/0x58e0
> [ 77.859488][ T5418] __lock_acquire+0x125b/0x1f80
> [ 77.864853][ T5418] lock_acquire+0x23e/0x630
> [ 77.869909][ T5418] (L4) down_write_nested+0x3d/0x50
> [ 77.875186][ T5418] open_xa_dir+0x134/0x540
> [ 77.880117][ T5418] xattr_lookup+0x17/0x210
> [ 77.885050][ T5418] reiserfs_xattr_get+0xe1/0x4a0
> [ 77.890501][ T5418] __vfs_getxattr+0x2fe/0x350
> [ 77.895802][ T5418] smk_fetch+0x98/0xf0
> [ 77.900382][ T5418] smack_d_instantiate+0x5d5/0xa20
> [ 77.906018][ T5418] security_d_instantiate+0x6b/0xb0
> [ 77.911736][ T5418] d_instantiate_new+0x5e/0xe0
> [ 77.917013][ T5418] (L1) reiserfs_create+0x5ee/0x680
> [ 77.922293][ T5418] path_openat+0xf1e/0x2c10
> [ 77.927308][ T5418] do_filp_open+0x22a/0x440
> [ 77.932330][ T5418] do_sys_openat2+0x10f/0x430
> [ 77.937515][ T5418] __x64_sys_creat+0x11e/0x160
> [ 77.942874][ T5418] do_syscall_64+0x41/0xc0
> [ 77.947796][ T5418] entry_SYSCALL_64_after_hwframe+0x63/0xcd
> [ 77.954200][ T5418]
> [ 77.954200][ T5418] other info that might help us debug this:
> [ 77.954200][ T5418]
> [ 77.964508][ T5418] Possible unsafe locking scenario:
> [ 77.964508][ T5418]
> [ 77.972034][ T5418] CPU0 CPU1
> [ 77.977394][ T5418] ---- ----
> [ 77.982748][ T5418] L1: lock(&sbi->lock);
> [ 77.986726][ T5418] L2: lock(&type->i_mutex_dir_key#8/3);
> [ 77.994618][ T5418] L3: lock(&sbi->lock);
> [ 78.001118][ T5418] L4: lock(&type->i_mutex_dir_key#8/3);
>
> Thanks
>
> Roberto
>
> > > First of all, the lockdep warning is trivial to reproduce:
> > >
> > > # dd if=/dev/zero of=reiserfs.img bs=1M count=100
> > > # losetup -f --show reiserfs.img
> > > /dev/loop0
> > > # mkfs.reiserfs /dev/loop0
> > > # mount /dev/loop0 /mnt/
> > > # touch file0
> > >
> > > In the testing system, Smack is the major LSM.
> > >
> > > Ok, so the warning here is clear:
> > >
> > > https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> > >
> > > However, I was looking if that can really happen. From this:
> > >
> > > [ 77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
> > > [ 77.753772][ T5418] lock_acquire+0x23e/0x630
> > > [ 77.758792][ T5418] __mutex_lock_common+0x1d8/0x2530
> > > [ 77.764504][ T5418] mutex_lock_nested+0x1b/0x20
> > > [ 77.769868][ T5418] reiserfs_write_lock+0x70/0xc0
> > > [ 77.775321][ T5418] reiserfs_mkdir+0x321/0x870
> > >
> > > I see that the lock is taken in reiserfs_write_lock(), while lockdep says:
> > >
> > > [ 77.710227][ T5418] but task is already holding lock:
> > > [ 77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at:
> > > reiserfs_write_lock_nested+0x4a/0xb0
> > >
> > > which is in a different place, I believe here:
> > >
> > > int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
> > > /* Path to the pasted item. */
> > > [...]
> > >
> > > depth = reiserfs_write_unlock_nested(sb);
> > > dquot_free_space_nodirty(inode, pasted_size);
> > > reiserfs_write_lock_nested(sb, depth);
> > > return retval;
> > > }
> > >
> > > This is called by reiserfs_add_entry(), which is called by
> > > reiserfs_create() (it is in the lockdep trace). After returning to
> > > reiserfs_create(), d_instantiate_new() is called.
> > >
> > > I don't know exactly, I take the part that the lock is held. But if it
> > > is held, how d_instantiate_new() can be executed in another task?
> > >
> > > static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
> > > struct dentry *dentry, umode_t mode, bool excl)
> > > {
> > >
> > > [...]
> > >
> > > reiserfs_write_lock(dir->i_sb);
> > >
> > > retval = journal_begin(&th, dir->i_sb, jbegin_count);
> > >
> > > [...]
> > >
> > > d_instantiate_new(dentry, inode);
> > > retval = journal_end(&th);
> > >
> > > out_failed:
> > > reiserfs_write_unlock(dir->i_sb);
> > >
> > > If the lock is held, the scenario lockdep describes cannot happen. Any
> > > thoughts?
> >
> > It's important to understand that the reiserfs write lock was added as a
> > subsystem-specific replacement for the BKL. Given that reiserfs was
> > dying already back then, it made more sense from a time management
> > perspective to emulate that behavior internally rather than use new
> > locking when practically nobody cared anymore.
> >
> > See reiserfs_write_unlock_nested and reiserfs_write_lock_nested paired
> > throughout the code. It drops the lock when it passes a point where
> > it's likely to schedule, just like the BKL would have.
> >
> > Yes, it's a mess. Just let it die quietly.
> >
> > -Jeff
> >
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
2022-12-31 6:35 [syzbot] [reiserfs?] possible deadlock in open_xa_dir syzbot
2023-05-05 7:10 ` syzbot
2023-05-05 20:51 ` syzbot
@ 2024-03-09 22:20 ` syzbot
2 siblings, 0 replies; 23+ messages in thread
From: syzbot @ 2024-03-09 22:20 UTC (permalink / raw)
To: axboe, brauner, hdanton, jack, jeffm, linux-fsdevel,
linux-kernel, mingo, paul, peterz, reiserfs-devel, roberto.sassu,
roberto.sassu, syzkaller-bugs, will
syzbot suspects this issue was fixed by commit:
commit 6f861765464f43a71462d52026fbddfc858239a5
Author: Jan Kara <jack@suse.cz>
Date: Wed Nov 1 17:43:10 2023 +0000
fs: Block writes to mounted block devices
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=11c28556180000
start commit: 5eff55d725a4 Merge tag 'platform-drivers-x86-v6.7-7' of gi..
git tree: upstream
kernel config: https://syzkaller.appspot.com/x/.config?x=655f8abe9fe69b3b
dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12d80b99e80000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=148cccdee80000
If the result looks correct, please mark the issue as fixed by replying with:
#syz fix: fs: Block writes to mounted block devices
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir
[not found] <20230505075628.4150-1-hdanton@sina.com>
@ 2023-05-05 8:32 ` syzbot
0 siblings, 0 replies; 23+ messages in thread
From: syzbot @ 2023-05-05 8:32 UTC (permalink / raw)
To: hdanton, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
WARNING in reiserfs_check_lock_depth
REISERFS (device loop0): checking transaction log (loop0)
REISERFS (device loop0): Using r5 hash to sort names
REISERFS (device loop0): Created .reiserfs_priv - reserved for xattr storage.
------------[ cut here ]------------
WARNING: CPU: 1 PID: 5415 at fs/reiserfs/lock.c:91 reiserfs_check_lock_depth+0x71/0x90
Modules linked in:
CPU: 1 PID: 5415 Comm: syz-executor.0 Not tainted 6.3.0-syzkaller-13164-g78b421b6a7c6-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/14/2023
RIP: 0010:reiserfs_check_lock_depth+0x71/0x90 fs/reiserfs/lock.c:91
Code: 03 42 0f b6 04 38 84 c0 75 24 41 8b 1e 31 ff 89 de e8 a3 4f 58 ff 85 db 78 0b e8 ea 4b 58 ff 5b 41 5e 41 5f c3 e8 df 4b 58 ff <0f> 0b eb f1 44 89 f1 80 e1 07 80 c1 03 38 c1 7c cf 4c 89 f7 e8 86
RSP: 0018:ffffc9000581f520 EFLAGS: 00010293
RAX: ffffffff82334431 RBX: 00000000ffffffff RCX: ffff888025ed0000
RDX: 0000000000000000 RSI: 00000000ffffffff RDI: 0000000000000000
RBP: ffffc9000581f7b0 R08: ffffffff8233441d R09: ffffed100e877044
R10: 0000000000000000 R11: dffffc0000000001 R12: 1ffff92000b03f05
R13: dffffc0000000000 R14: ffff88807783b0c0 R15: dffffc0000000000
FS: 00007f9491058700(0000) GS:ffff8880b9900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f62321d1ff8 CR3: 000000002a3c7000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
do_journal_end+0x20f/0x4770 fs/reiserfs/journal.c:4018
reiserfs_create+0x68e/0x710 fs/reiserfs/namei.c:694
lookup_open fs/namei.c:3492 [inline]
open_last_lookups fs/namei.c:3560 [inline]
path_openat+0x13df/0x3170 fs/namei.c:3788
do_filp_open+0x234/0x490 fs/namei.c:3818
do_sys_openat2+0x13f/0x500 fs/open.c:1356
do_sys_open fs/open.c:1372 [inline]
__do_sys_openat fs/open.c:1388 [inline]
__se_sys_openat fs/open.c:1383 [inline]
__x64_sys_openat+0x247/0x290 fs/open.c:1383
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7f949028c169
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 19 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f9491058168 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
RAX: ffffffffffffffda RBX: 00007f94903abf80 RCX: 00007f949028c169
RDX: 000000000000275a RSI: 0000000020000080 RDI: ffffffffffffff9c
RBP: 00007f94902e7ca1 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fff4aaf8b4f R14: 00007f9491058300 R15: 0000000000022000
</TASK>
Tested on:
commit: 78b421b6 Merge tag 'linux-watchdog-6.4-rc1' of git://w..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=1271004c280000
kernel config: https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
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=1194d2f4280000
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2024-03-09 22:20 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-31 6:35 [syzbot] [reiserfs?] possible deadlock in open_xa_dir syzbot
2023-05-05 7:10 ` syzbot
2023-05-05 20:51 ` syzbot
2023-05-05 21:36 ` Paul Moore
2023-05-05 21:36 ` Paul Moore
2023-05-31 9:49 ` Roberto Sassu
2023-05-31 9:49 ` Roberto Sassu
2023-05-31 9:52 ` Roberto Sassu
2023-06-01 21:22 ` Jeff Mahoney
2023-06-02 7:20 ` Roberto Sassu
2023-06-02 7:20 ` Roberto Sassu
2023-06-02 8:56 ` Roberto Sassu
2023-06-02 9:17 ` syzbot
2023-06-02 16:18 ` Roberto Sassu
2023-06-02 16:39 ` syzbot
2023-06-02 16:46 ` Roberto Sassu
2023-06-02 16:46 ` Roberto Sassu
2023-06-01 20:19 ` Roberto Sassu
2023-06-01 20:38 ` syzbot
2023-06-01 20:30 ` Roberto Sassu
2023-06-01 20:57 ` syzbot
2024-03-09 22:20 ` syzbot
[not found] <20230505075628.4150-1-hdanton@sina.com>
2023-05-05 8:32 ` syzbot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.