[syzbot] [udf?] WARNING in udf_rmdir (2)

[syzbot] [udf?] WARNING in udf_rmdir (2)

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    228a1157fb9f Merge tag '6.13-rc-part1-SMB3-client-fixes' o..
git tree:       upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=122c0778580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=402159daa216c89d
dashboard link: https://syzkaller.appspot.com/bug?extid=5df2d3fa14f2d3e49305
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14b476e8580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=162c0778580000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/d32a8e8c5aae/disk-228a1157.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/28d5c070092e/vmlinux-228a1157.xz
kernel image: https://storage.googleapis.com/syzbot-assets/45af4bfd9e8e/bzImage-228a1157.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/b7209c2fe94f/mount_0.gz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+5df2d3fa14f2d3e49305@syzkaller.appspotmail.com

UDF-fs: warning (device loop0): udf_rmdir: empty directory has nlink != 2 (0)
UDF-fs: warning (device loop0): udf_rmdir: empty directory has nlink != 2 (0)
------------[ cut here ]------------
WARNING: CPU: 1 PID: 5834 at fs/inode.c:407 drop_nlink+0xc4/0x110 fs/inode.c:407
Modules linked in:
CPU: 1 UID: 0 PID: 5834 Comm: syz-executor338 Not tainted 6.12.0-syzkaller-08446-g228a1157fb9f #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
RIP: 0010:drop_nlink+0xc4/0x110 fs/inode.c:407
Code: bb 70 07 00 00 be 08 00 00 00 e8 27 d8 e5 ff f0 48 ff 83 70 07 00 00 5b 41 5c 41 5e 41 5f 5d c3 cc cc cc cc e8 9d 40 7e ff 90 <0f> 0b 90 eb 83 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 5c ff ff ff
RSP: 0018:ffffc90003f47ad0 EFLAGS: 00010293
RAX: ffffffff82170c43 RBX: 1ffff1100ed1c100 RCX: ffff88802d9c0000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: ffffffff82170bc3 R09: 1ffff920007e8ed8
R10: dffffc0000000000 R11: fffff520007e8ed9 R12: ffff8880768e0800
R13: ffff8880768e07b8 R14: ffff8880768e07b8 R15: dffffc0000000000
FS:  000055556e49b380(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000066c7e0 CR3: 000000003468a000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 inode_dec_link_count include/linux/fs.h:2521 [inline]
 udf_rmdir+0x3b8/0x6f0 fs/udf/namei.c:520
 vfs_rmdir+0x3a3/0x510 fs/namei.c:4394
 do_rmdir+0x3b5/0x580 fs/namei.c:4453
 __do_sys_rmdir fs/namei.c:4472 [inline]
 __se_sys_rmdir fs/namei.c:4470 [inline]
 __x64_sys_rmdir+0x47/0x50 fs/namei.c:4470
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f6beb678a39
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 61 17 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:00007fffba3588d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000054
RAX: ffffffffffffffda RBX: 000000000000003d RCX: 00007f6beb678a39
RDX: 00007f6beb678a39 RSI: 00007f6beb678a39 RDI: 0000000020000000
RBP: 00007f6beb6ec610 R08: 00007fffba358aa8 R09: 00007fffba358aa8
R10: 00007fffba358aa8 R11: 0000000000000246 R12: 0000000000000001
R13: 00007fffba358a98 R14: 0000000000000001 R15: 0000000000000001
 </TASK>


---
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.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

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.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

[PATCH] udf: Prevent rmdir of deleted directories

[Leo Stone]

The syzbot reproducer mounts a UDF image with the undelete option
enabled. It then creates a directory, and eventually attempts to call
rmdir on it 65 times. Because the undelete option is set, the
directory still gets found in udf_fiiter_find_entry(), and the link
count of its parent directory is decremented until it triggers the
warning in drop_nlink().

Prevent directories with the FID_FILE_CHAR_DELETED flag set from being
deleted again.

#syz test

Reported-by: syzbot+5df2d3fa14f2d3e49305@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5df2d3fa14f2d3e49305
Signed-off-by: Leo Stone <leocstone@gmail.com>
---
 fs/udf/namei.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 78a603129dd5..0a577b7459de 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -504,6 +504,12 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)
 	if (ret)
 		goto out;
 
+	if (iter.fi.fileCharacteristics & FID_FILE_CHAR_DELETED) {
+		udf_warn(inode->i_sb,
+			 "tried to rmdir a directory that was already deleted\n");
+		ret = -ENOENT;
+		goto end_rmdir;
+	}
 	ret = -EFSCORRUPTED;
 	tloc = lelb_to_cpu(iter.fi.icb.extLocation);
 	if (udf_get_lb_pblock(dir->i_sb, &tloc, 0) != inode->i_ino)
-- 
2.43.0

Re: [syzbot] [udf?] WARNING in udf_rmdir (2)

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+5df2d3fa14f2d3e49305@syzkaller.appspotmail.com
Tested-by: syzbot+5df2d3fa14f2d3e49305@syzkaller.appspotmail.com

Tested on:

commit:         40384c84 Linux 6.13-rc1
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=135195e8580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=55aba29dc49cabd6
dashboard link: https://syzkaller.appspot.com/bug?extid=5df2d3fa14f2d3e49305
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=17faa0df980000

Note: testing is done by a robot and is best-effort only.

Re: [PATCH] udf: Prevent rmdir of deleted directories

[Jan Kara]

On Sun 01-12-24 16:41:02, Leo Stone wrote:
> The syzbot reproducer mounts a UDF image with the undelete option
> enabled. It then creates a directory, and eventually attempts to call
> rmdir on it 65 times. Because the undelete option is set, the
> directory still gets found in udf_fiiter_find_entry(), and the link
> count of its parent directory is decremented until it triggers the
> warning in drop_nlink().
> 
> Prevent directories with the FID_FILE_CHAR_DELETED flag set from being
> deleted again.
> 
> #syz test
> 
> Reported-by: syzbot+5df2d3fa14f2d3e49305@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5df2d3fa14f2d3e49305
> Signed-off-by: Leo Stone <leocstone@gmail.com>

Thanks for looking into this but the fix is already queued into my tree and
I'll send it to Linus today.

								Honza

> ---
>  fs/udf/namei.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index 78a603129dd5..0a577b7459de 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -504,6 +504,12 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)
>  	if (ret)
>  		goto out;
>  
> +	if (iter.fi.fileCharacteristics & FID_FILE_CHAR_DELETED) {
> +		udf_warn(inode->i_sb,
> +			 "tried to rmdir a directory that was already deleted\n");
> +		ret = -ENOENT;
> +		goto end_rmdir;
> +	}
>  	ret = -EFSCORRUPTED;
>  	tloc = lelb_to_cpu(iter.fi.icb.extLocation);
>  	if (udf_get_lb_pblock(dir->i_sb, &tloc, 0) != inode->i_ino)
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [syzbot] [udf?] WARNING in udf_rmdir (2)

[syzbot]

syzbot suspects this issue was fixed by commit:

commit c5566903af56dd1abb092f18dcb0c770d6cd8dcb
Author: Jan Kara <jack@suse.cz>
Date:   Tue Nov 26 11:46:00 2024 +0000

    udf: Skip parent dir link count update if corrupted

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=12bc6a18580000
start commit:   228a1157fb9f Merge tag '6.13-rc-part1-SMB3-client-fixes' o..
git tree:       upstream
kernel config:  https://syzkaller.appspot.com/x/.config?x=402159daa216c89d
dashboard link: https://syzkaller.appspot.com/bug?extid=5df2d3fa14f2d3e49305
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14b476e8580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=162c0778580000

If the result looks correct, please mark the issue as fixed by replying with:

#syz fix: udf: Skip parent dir link count update if corrupted

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

Re: [syzbot] [udf?] WARNING in udf_rmdir (2)

[Jan Kara]

On Tue 14-01-25 19:53:04, syzbot wrote:
> syzbot suspects this issue was fixed by commit:
> 
> commit c5566903af56dd1abb092f18dcb0c770d6cd8dcb
> Author: Jan Kara <jack@suse.cz>
> Date:   Tue Nov 26 11:46:00 2024 +0000
> 
>     udf: Skip parent dir link count update if corrupted
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=12bc6a18580000
> start commit:   228a1157fb9f Merge tag '6.13-rc-part1-SMB3-client-fixes' o..
> git tree:       upstream
> kernel config:  https://syzkaller.appspot.com/x/.config?x=402159daa216c89d
> dashboard link: https://syzkaller.appspot.com/bug?extid=5df2d3fa14f2d3e49305
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14b476e8580000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=162c0778580000
> 
> If the result looks correct, please mark the issue as fixed by replying with:

Makes sense:
 
#syz fix: udf: Skip parent dir link count update if corrupted

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR