linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [syzbot] [ext4?] kernel BUG in ext4_update_inline_data
@ 2025-07-07 16:32 syzbot
  0 siblings, 0 replies; 9+ messages in thread
From: syzbot @ 2025-07-07 16:32 UTC (permalink / raw)
  To: adilger.kernel, linux-ext4, linux-kernel, syzkaller-bugs, tytso

Hello,

syzbot found the following issue on:

HEAD commit:    772b78c2abd8 Merge tag 'sched_urgent_for_v6.16_rc5' of git..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1747ef70580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=930b74448cb4593a
dashboard link: https://syzkaller.appspot.com/bug?extid=544248a761451c0df72f
compiler:       Debian clang version 20.1.7 (++20250616065708+6146a88f6049-1~exp1~20250616065826.132), Debian LLD 20.1.7
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=120e828c580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=160e828c580000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/d900f083ada3/non_bootable_disk-772b78c2.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/8a1257ea2c00/vmlinux-772b78c2.xz
kernel image: https://storage.googleapis.com/syzbot-assets/ff01d2c78ebd/bzImage-772b78c2.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/f97118969515/mount_0.gz
  fsck result: OK (log: https://syzkaller.appspot.com/x/fsck.log?x=1581628c580000)

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

EXT4-fs (loop0): mounted filesystem 00000000-0000-0000-0000-000000000000 r/w without journal. Quota mode: writeback.
fscrypt: AES-256-XTS using implementation "xts-aes-aesni-avx"
loop0: detected capacity change from 512 to 64
------------[ cut here ]------------
kernel BUG at fs/ext4/inline.c:357!
Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
CPU: 0 UID: 0 PID: 5499 Comm: syz.0.16 Not tainted 6.16.0-rc4-syzkaller-00348-g772b78c2abd8 #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
RIP: 0010:ext4_update_inline_data+0x4e8/0x4f0 fs/ext4/inline.c:357
Code: ff ff ff 48 8b 4c 24 18 80 e1 07 fe c1 38 c1 0f 8c 32 ff ff ff 48 8b 7c 24 18 e8 33 59 b1 ff e9 23 ff ff ff e8 e9 d5 4d ff 90 <0f> 0b 66 0f 1f 44 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90
RSP: 0018:ffffc90002abf4a0 EFLAGS: 00010293
RAX: ffffffff82725017 RBX: ffff8880123cf558 RCX: ffff88800020c880
RDX: 0000000000000000 RSI: 00000000ffffffc3 RDI: 0000000000000000
RBP: ffffc90002abf5f0 R08: ffff88800020c880 R09: 0000000000000002
R10: 00000000ffffffc3 R11: 0000000000000000 R12: 00000000ffffffc3
R13: 000000000000004a R14: ffffc90002abf500 R15: ffffc90002abf528
FS:  0000555558bce500(0000) GS:ffff88808d21d000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055795b748950 CR3: 000000003f98a000 CR4: 0000000000352ef0
Call Trace:
 <TASK>
 ext4_prepare_inline_data+0x141/0x1d0 fs/ext4/inline.c:415
 ext4_generic_write_inline_data+0x207/0xc90 fs/ext4/inline.c:692
 ext4_try_to_write_inline_data+0x80/0xa0 fs/ext4/inline.c:763
 ext4_write_begin+0x2d8/0x1680 fs/ext4/inode.c:1281
 generic_perform_write+0x2c7/0x910 mm/filemap.c:4112
 ext4_buffered_write_iter+0xce/0x3a0 fs/ext4/file.c:299
 ext4_file_write_iter+0x298/0x1bc0 fs/ext4/file.c:-1
 new_sync_write fs/read_write.c:593 [inline]
 vfs_write+0x548/0xa90 fs/read_write.c:686
 ksys_write+0x145/0x250 fs/read_write.c:738
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f32a778e929
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fffd1fdeb38 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007f32a79b5fa0 RCX: 00007f32a778e929
RDX: 000000000000004a RSI: 0000000000000000 RDI: 0000000000000007
RBP: 00007f32a7810b39 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f32a79b5fa0 R14: 00007f32a79b5fa0 R15: 0000000000000003
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:ext4_update_inline_data+0x4e8/0x4f0 fs/ext4/inline.c:357
Code: ff ff ff 48 8b 4c 24 18 80 e1 07 fe c1 38 c1 0f 8c 32 ff ff ff 48 8b 7c 24 18 e8 33 59 b1 ff e9 23 ff ff ff e8 e9 d5 4d ff 90 <0f> 0b 66 0f 1f 44 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90
RSP: 0018:ffffc90002abf4a0 EFLAGS: 00010293
RAX: ffffffff82725017 RBX: ffff8880123cf558 RCX: ffff88800020c880
RDX: 0000000000000000 RSI: 00000000ffffffc3 RDI: 0000000000000000
RBP: ffffc90002abf5f0 R08: ffff88800020c880 R09: 0000000000000002
R10: 00000000ffffffc3 R11: 0000000000000000 R12: 00000000ffffffc3
R13: 000000000000004a R14: ffffc90002abf500 R15: ffffc90002abf528
FS:  0000555558bce500(0000) GS:ffff88808d21d000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000563842271138 CR3: 000000003f98a000 CR4: 0000000000352ef0


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [syzbot] [ext4?] kernel BUG in ext4_update_inline_data
@ 2025-07-10  8:01 Moon Hee Lee
  2025-07-10  8:23 ` syzbot
  2025-07-17 14:59 ` [PATCH] ext4: do not BUG when INLINE_DATA_FL lacks system.data xattr Theodore Ts'o
  0 siblings, 2 replies; 9+ messages in thread
From: Moon Hee Lee @ 2025-07-10  8:01 UTC (permalink / raw)
  To: syzbot+544248a761451c0df72f
  Cc: adilger.kernel, linux-ext4, linux-kernel, syzkaller-bugs, tytso

[-- Attachment #1: Type: text/plain, Size: 10 bytes --]

#syz test

[-- Attachment #2: 0001-ext4-bail-out-when-INLINE_DATA_FL-lacks-system.data-.patch --]
[-- Type: text/x-patch, Size: 2885 bytes --]

From 4c910ac989e7a6d97565a67677a1ee88e2d1a9ad Mon Sep 17 00:00:00 2001
From: Moon Hee Lee <moonhee.lee.ca@gmail.com>
Date: Thu, 10 Jul 2025 00:36:59 -0700
Subject: [PATCH] ext4: bail out when INLINE_DATA_FL lacks system.data xattr

A syzbot fuzzed image triggered a BUG_ON in ext4_update_inline_data()
when an inode had the INLINE_DATA_FL flag set but was missing the
system.data extended attribute.

ext4_prepare_inline_data() now checks for the presence of that xattr
and returns -EFSCORRUPTED if it is missing, preventing corrupted inodes
from reaching the update path and triggering a crash.

Proof from e2fsck on the fuzzed image:

    $ e2fsck -fn mount_0
    e2fsck 1.47.0 (5-Feb-2023)
    One or more block group descriptor checksums are invalid.  Fix? no

    Group descriptor 0 checksum is 0x8245, should be 0x353a.  IGNORED.
    Pass 1: Checking inodes, blocks, and sizes
    Inode 12 has INLINE_DATA_FL flag but extended attribute not found.  Truncate? no

    Inode 16, i_blocks is 3298534883346, should be 18.  Fix? no

    Inode 17, i_blocks is 17592186044416, should be 0.  Fix? no

    Pass 2: Checking directory structure
    Symlink /file0/file1 (inode #14) is invalid.
    Clear? no

    Entry 'file1' in /file0 (12) has an incorrect filetype (was 7, should be 0).
    Fix? no

    Directory inode 11, block #5, offset 0: directory corrupted
    Salvage? no

    e2fsck: aborted

    syzkaller: ********** WARNING: Filesystem still has errors **********

Signed-off-by: Moon Hee Lee <moonhee.lee.ca@gmail.com>
---
 fs/ext4/inline.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index a1bbcdf40824..d9dcb0b09e5c 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -399,6 +399,13 @@ static int ext4_update_inline_data(handle_t *handle, struct inode *inode,
 static int ext4_prepare_inline_data(handle_t *handle, struct inode *inode,
 				    loff_t len)
 {
+	struct ext4_xattr_ibody_find is = {
+		.s = { .not_found = -ENODATA, },
+	};
+	struct ext4_xattr_info i = {
+		.name_index = EXT4_XATTR_INDEX_SYSTEM,
+		.name = EXT4_XATTR_SYSTEM_DATA,
+	};
 	int ret, size, no_expand;
 	struct ext4_inode_info *ei = EXT4_I(inode);
 
@@ -409,6 +416,19 @@ static int ext4_prepare_inline_data(handle_t *handle, struct inode *inode,
 	if (size < len)
 		return -ENOSPC;
 
+	ret = ext4_get_inode_loc(inode, &is.iloc);
+	if (ret)
+		goto out;
+
+	ret = ext4_xattr_ibody_find(inode, &i, &is);
+	if (ret)
+		goto out;
+
+	if (is.s.not_found) {
+		ret = -EFSCORRUPTED;
+		goto out;
+	}
+
 	ext4_write_lock_xattr(inode, &no_expand);
 
 	if (ei->i_inline_off)
@@ -417,6 +437,8 @@ static int ext4_prepare_inline_data(handle_t *handle, struct inode *inode,
 		ret = ext4_create_inline_data(handle, inode, len);
 
 	ext4_write_unlock_xattr(inode, &no_expand);
+out:
+	brelse(is.iloc.bh);
 	return ret;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [syzbot] [ext4?] kernel BUG in ext4_update_inline_data
  2025-07-10  8:01 [syzbot] [ext4?] kernel BUG in ext4_update_inline_data Moon Hee Lee
@ 2025-07-10  8:23 ` syzbot
  2025-07-17 14:59 ` [PATCH] ext4: do not BUG when INLINE_DATA_FL lacks system.data xattr Theodore Ts'o
  1 sibling, 0 replies; 9+ messages in thread
From: syzbot @ 2025-07-10  8:23 UTC (permalink / raw)
  To: adilger.kernel, linux-ext4, linux-kernel, moonhee.lee.ca,
	syzkaller-bugs, tytso

Hello,

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

Reported-by: syzbot+544248a761451c0df72f@syzkaller.appspotmail.com
Tested-by: syzbot+544248a761451c0df72f@syzkaller.appspotmail.com

Tested on:

commit:         8c2e52eb eventpoll: don't decrement ep refcount while ..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14ff7582580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=b309c907eaab29da
dashboard link: https://syzkaller.appspot.com/bug?extid=544248a761451c0df72f
compiler:       Debian clang version 20.1.7 (++20250616065708+6146a88f6049-1~exp1~20250616065826.132), Debian LLD 20.1.7
patch:          https://syzkaller.appspot.com/x/patch.diff?x=15ef7582580000

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] ext4: do not BUG when INLINE_DATA_FL lacks system.data xattr
  2025-07-10  8:01 [syzbot] [ext4?] kernel BUG in ext4_update_inline_data Moon Hee Lee
  2025-07-10  8:23 ` syzbot
@ 2025-07-17 14:59 ` Theodore Ts'o
  2025-07-17 16:59   ` Moon Hee Lee
  1 sibling, 1 reply; 9+ messages in thread
From: Theodore Ts'o @ 2025-07-17 14:59 UTC (permalink / raw)
  To: Moon Hee Lee
  Cc: syzbot+544248a761451c0df72f, adilger.kernel, linux-ext4,
	linux-kernel, syzkaller-bugs

On Thu, Jul 10, 2025 at 01:01:04AM -0700, Moon Hee Lee wrote:
> From 4c910ac989e7a6d97565a67677a1ee88e2d1a9ad Mon Sep 17 00:00:00 2001
> From: Moon Hee Lee <moonhee.lee.ca@gmail.com>
> Date: Thu, 10 Jul 2025 00:36:59 -0700
> Subject: [PATCH] ext4: bail out when INLINE_DATA_FL lacks system.data xattr
> 
> A syzbot fuzzed image triggered a BUG_ON in ext4_update_inline_data()
> when an inode had the INLINE_DATA_FL flag set but was missing the
> system.data extended attribute.
> 
> ext4_prepare_inline_data() now checks for the presence of that xattr
> and returns -EFSCORRUPTED if it is missing, preventing corrupted inodes
> from reaching the update path and triggering a crash.

Thanks ofor the patch!  However, instead of doing an xattr lookup in
ext4_prepare_inline_data(), we can more simply and more efficiently
just not BUG in ext4_update_inline_data, like this:


From eedfada9eb51541fe72f19350503890da522212d Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <tytso@mit.edu>
Date: Thu, 17 Jul 2025 10:54:34 -0400
Subject: [PATCH] ext4: do not BUG when INLINE_DATA_FL lacks system.data xattr

A syzbot fuzzed image triggered a BUG_ON in ext4_update_inline_data()
when an inode had the INLINE_DATA_FL flag set but was missing the
system.data extended attribute.

Since this can happen due to a maiciouly fuzzed file system, we
shouldn't BUG, but rather, report it as a corrupted file system.

Reported-by: syzbot+544248a761451c0df72f@syzkaller.appspotmail.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/inline.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index d5b32d242495..424c40c768de 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -354,6 +354,12 @@ static int ext4_update_inline_data(handle_t *handle, struct inode *inode,
 	if (error)
 		goto out;
 
+	if (is.s.not_found) {
+		EXT4_ERROR_INODE(inode, "missing inline data xattr");
+		error = -EFSCORRUPTED;
+		goto out;
+	}
+
 	BUG_ON(is.s.not_found);
 
 	len -= EXT4_MIN_INLINE_DATA_SIZE;
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] ext4: do not BUG when INLINE_DATA_FL lacks system.data xattr
  2025-07-17 14:59 ` [PATCH] ext4: do not BUG when INLINE_DATA_FL lacks system.data xattr Theodore Ts'o
@ 2025-07-17 16:59   ` Moon Hee Lee
  2025-07-17 19:53     ` Moon Hee Lee
  2025-07-18  1:05     ` Theodore Ts'o
  0 siblings, 2 replies; 9+ messages in thread
From: Moon Hee Lee @ 2025-07-17 16:59 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: syzbot+544248a761451c0df72f, adilger.kernel, linux-ext4,
	linux-kernel, syzkaller-bugs

>
> Thanks ofor the patch!  However, instead of doing an xattr lookup in
> ext4_prepare_inline_data(), we can more simply and more efficiently
> just not BUG in ext4_update_inline_data, like this:

Thanks for the response and for taking the time to address the issue.

Just to clarify the intent behind the earlier patch [1]: it was meant to
catch the missing system.data xattr early in ext4_prepare_inline_data(),
before branching into paths that assume the xattr is present.

> @@ -354,6 +354,12 @@ static int ext4_update_inline_data(handle_t *handle, struct inode *inode,
>         if (error)
>                 goto out;
>
> +       if (is.s.not_found) {
> +               EXT4_ERROR_INODE(inode, "missing inline data xattr");
> +               error = -EFSCORRUPTED;
> +               goto out;
> +       }
> +
>         BUG_ON(is.s.not_found);

The current patch addresses ext4_update_inline_data() directly, but the
same condition also leads to a BUG_ON in ext4_create_inline_data() [2],
which the earlier approach intended to prevent as well.

Later, a third instance was found in ext4_inline_data_truncate() [3],
which also contains a similar BUG_ON and might need the same kind of
check.

Reducing duplicated checks across these sites would be beneficial, though
fixing each case directly also looks reasonable and straightforward.

[1] https://lore.kernel.org/all/20250710175837.29822-2-moonhee.lee.ca@gmail.com/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ext4/inline.c?h=v6.16-rc6#n306
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ext4/inline.c?h=v6.16-rc6#n1906

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ext4: do not BUG when INLINE_DATA_FL lacks system.data xattr
  2025-07-17 16:59   ` Moon Hee Lee
@ 2025-07-17 19:53     ` Moon Hee Lee
  2025-07-18  1:05     ` Theodore Ts'o
  1 sibling, 0 replies; 9+ messages in thread
From: Moon Hee Lee @ 2025-07-17 19:53 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: syzbot+544248a761451c0df72f, adilger.kernel, linux-ext4,
	linux-kernel, syzkaller-bugs

On Thu, Jul 17, 2025 at 9:59 AM Moon Hee Lee <moonhee.lee.ca@gmail.com> wrote:

Just a quick follow-up to close the loop:

>
> The current patch addresses ext4_update_inline_data() directly, but the
> same condition also leads to a BUG_ON in ext4_create_inline_data() [2],
> which the earlier approach intended to prevent as well.

I missed that ext4_create_inline_data expects the xattr to be absent at
that point, since it's about to create it. The BUG_ON(!is.s.not_found)
enforces that expectation.

The patch I sent earlier didn’t account for this correctly. Returning an
error when the xattr was not found would have broken valid behavior in
the create path.

Thanks again for resolving this with a simple and correct fix. Per-site
handling makes sense here, as each path has different expectations about
the xattr state.

Best regards,
Moonhee

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ext4: do not BUG when INLINE_DATA_FL lacks system.data xattr
  2025-07-17 16:59   ` Moon Hee Lee
  2025-07-17 19:53     ` Moon Hee Lee
@ 2025-07-18  1:05     ` Theodore Ts'o
  2025-07-21 12:49       ` Jan Kara
  1 sibling, 1 reply; 9+ messages in thread
From: Theodore Ts'o @ 2025-07-18  1:05 UTC (permalink / raw)
  To: Moon Hee Lee
  Cc: syzbot+544248a761451c0df72f, adilger.kernel, linux-ext4,
	linux-kernel, syzkaller-bugs, Jan Kara, Jens Axboe, linux-block

On Thu, Jul 17, 2025 at 09:59:13AM -0700, Moon Hee Lee wrote:
> The current patch addresses ext4_update_inline_data() directly, but the
> same condition also leads to a BUG_ON in ext4_create_inline_data() [2],
> which the earlier approach intended to prevent as well.

Actually, the two conditions are opposite to each other.  The one in
ext4_update_inline_data() was:

         BUG_ON(is.s.not_found);

while te one in ext4_create_inline_data() was:

	BUG_ON(!is.s.not_found);

So your patch would not only cause an extra xattr lookup in
ext4_prepare_inline_data(), but it would actually cause problems by
causing spurious failures when first writing to an inline data file.
(Which makes me suspect that you hadn't run other test on your patich
other than just vaidating that the syzkaller reproduce was no longer
reproducing.)   

Also, having taking a closer look at te code paths, I became
suspicious that there is something about the syzkaller reproducer is
doing which might be a bit sus.  That's because whether we call
ext4_update_inline_data() or ext4_create_inline_data() is based on
whether i_inline off is set or not:

	if (ei->i_inline_off)
		ret = ext4_update_inline_data(handle, inode, len);
	else
		ret = ext4_create_inline_data(handle, inode, len);


But how is ei->i_inline_off set?  It's set from a former call to
ext4_xattr_ibody_find():

	error = ext4_xattr_ibody_find(inode, &i, &is);
	if (error)
		goto out;

	if (!is.s.not_found) {
		if (is.s.here->e_value_inum) {
			EXT4_ERROR_INODE(inode, "inline data xattr refers "
					 "to an external xattr inode");
			error = -EFSCORRUPTED;
			goto out;
		}
		EXT4_I(inode)->i_inline_off = (u16)((void *)is.s.here -
					(void *)ext4_raw_inode(&is.iloc));
		EXT4_I(inode)->i_inline_size = EXT4_MIN_INLINE_DATA_SIZE +
				le32_to_cpu(is.s.here->e_value_size);
	}

So the whole *reason* why i_inline_off exists is because we're caching
the result of calling ext4_xattr_ibody_find().  So if i_inline_off is
non-zero, and then when we call ext4_ibody_find() later on, and we
find that xattr has suddenly disappeared, there is something weird
going on.   That's why the BUG_ON was added orginally.

When I took a look at the reproduer, I found that indeed, it is
calling LOOP_CLR_FD and LOOP_SET_STATUS64 to reconfigure the loop
device out from under the mounted file system.  This is smashing the
file system, and is therefore corrupting the block device.  As it
turns out, Jan Kara recently sent out a patch, and it has been
accepted in the block tree, to prevent a similar Syzkaller issue using
LOOP_SET_BLOCK_SIZE[1].

[1] https://lore.kernel.org/r/20250711163202.19623-2-jack@suse.cz

We need to do something similar for LOOP_CLR_FD, LOOP_SET_STATUS,
LOOP_SET_STATUS64, LOOP_CHANGE_FD, and LOOP_SET_CAPACITY ioctls.

Cheers,

						- Ted

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ext4: do not BUG when INLINE_DATA_FL lacks system.data xattr
  2025-07-18  1:05     ` Theodore Ts'o
@ 2025-07-21 12:49       ` Jan Kara
  2025-07-21 13:51         ` Theodore Ts'o
  0 siblings, 1 reply; 9+ messages in thread
From: Jan Kara @ 2025-07-21 12:49 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Moon Hee Lee, syzbot+544248a761451c0df72f, adilger.kernel,
	linux-ext4, linux-kernel, syzkaller-bugs, Jan Kara, Jens Axboe,
	linux-block

On Thu 17-07-25 21:05:21, Theodore Ts'o wrote:
> On Thu, Jul 17, 2025 at 09:59:13AM -0700, Moon Hee Lee wrote:
> > The current patch addresses ext4_update_inline_data() directly, but the
> > same condition also leads to a BUG_ON in ext4_create_inline_data() [2],
> > which the earlier approach intended to prevent as well.
> 
> Actually, the two conditions are opposite to each other.  The one in
> ext4_update_inline_data() was:
> 
>          BUG_ON(is.s.not_found);
> 
> while te one in ext4_create_inline_data() was:
> 
> 	BUG_ON(!is.s.not_found);
> 
> So your patch would not only cause an extra xattr lookup in
> ext4_prepare_inline_data(), but it would actually cause problems by
> causing spurious failures when first writing to an inline data file.
> (Which makes me suspect that you hadn't run other test on your patich
> other than just vaidating that the syzkaller reproduce was no longer
> reproducing.)   
> 
> Also, having taking a closer look at te code paths, I became
> suspicious that there is something about the syzkaller reproducer is
> doing which might be a bit sus.  That's because whether we call
> ext4_update_inline_data() or ext4_create_inline_data() is based on
> whether i_inline off is set or not:
> 
> 	if (ei->i_inline_off)
> 		ret = ext4_update_inline_data(handle, inode, len);
> 	else
> 		ret = ext4_create_inline_data(handle, inode, len);
> 
> 
> But how is ei->i_inline_off set?  It's set from a former call to
> ext4_xattr_ibody_find():
> 
> 	error = ext4_xattr_ibody_find(inode, &i, &is);
> 	if (error)
> 		goto out;
> 
> 	if (!is.s.not_found) {
> 		if (is.s.here->e_value_inum) {
> 			EXT4_ERROR_INODE(inode, "inline data xattr refers "
> 					 "to an external xattr inode");
> 			error = -EFSCORRUPTED;
> 			goto out;
> 		}
> 		EXT4_I(inode)->i_inline_off = (u16)((void *)is.s.here -
> 					(void *)ext4_raw_inode(&is.iloc));
> 		EXT4_I(inode)->i_inline_size = EXT4_MIN_INLINE_DATA_SIZE +
> 				le32_to_cpu(is.s.here->e_value_size);
> 	}
> 
> So the whole *reason* why i_inline_off exists is because we're caching
> the result of calling ext4_xattr_ibody_find().  So if i_inline_off is
> non-zero, and then when we call ext4_ibody_find() later on, and we
> find that xattr has suddenly disappeared, there is something weird
> going on.   That's why the BUG_ON was added orginally.
> 
> When I took a look at the reproduer, I found that indeed, it is
> calling LOOP_CLR_FD and LOOP_SET_STATUS64 to reconfigure the loop
> device out from under the mounted file system.  This is smashing the
> file system, and is therefore corrupting the block device.  As it
> turns out, Jan Kara recently sent out a patch, and it has been
> accepted in the block tree, to prevent a similar Syzkaller issue using
> LOOP_SET_BLOCK_SIZE[1].
> 
> [1] https://lore.kernel.org/r/20250711163202.19623-2-jack@suse.cz
> 
> We need to do something similar for LOOP_CLR_FD, LOOP_SET_STATUS,
> LOOP_SET_STATUS64, LOOP_CHANGE_FD, and LOOP_SET_CAPACITY ioctls.

Well, careful here. Changing loop device underneath mounted filesystem is a
valid usecase in active use (similarly as changing DM device underneath a
filesystem). So don't think we can play similar tricks as with
LOOP_SET_BLOCK_SIZE where changing block device block size just doesn't
make sense while the device is in use. Similarly LOOP_CLR_FD is an
equivalent of device going away. LOOP_CHANGE_FD is a legacy of the past but
it was *designed* to be used to swap backing file under a life filesystem
(old days of Wild West :)) during boot. We may get away with dropping that
these days but so far I'm not convinced it's worth the risk. So in this case
I don't see anything here that couldn't happen with say DM device and thus
I wouldn't really restrict the loop device functionality...

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ext4: do not BUG when INLINE_DATA_FL lacks system.data xattr
  2025-07-21 12:49       ` Jan Kara
@ 2025-07-21 13:51         ` Theodore Ts'o
  0 siblings, 0 replies; 9+ messages in thread
From: Theodore Ts'o @ 2025-07-21 13:51 UTC (permalink / raw)
  To: Jan Kara
  Cc: Moon Hee Lee, syzbot+544248a761451c0df72f, adilger.kernel,
	linux-ext4, linux-kernel, syzkaller-bugs, Jens Axboe, linux-block

On Mon, Jul 21, 2025 at 02:49:57PM +0200, Jan Kara wrote:
> > We need to do something similar for LOOP_CLR_FD, LOOP_SET_STATUS,
> > LOOP_SET_STATUS64, LOOP_CHANGE_FD, and LOOP_SET_CAPACITY ioctls.
> 
> Well, careful here. Changing loop device underneath mounted filesystem is a
> valid usecase in active use (similarly as changing DM device underneath a
> filesystem). So don't think we can play similar tricks as with
> LOOP_SET_BLOCK_SIZE where changing block device block size just doesn't
> make sense while the device is in use. Similarly LOOP_CLR_FD is an
> equivalent of device going away. LOOP_CHANGE_FD is a legacy of the past but
> it was *designed* to be used to swap backing file under a life filesystem
> (old days of Wild West :)) during boot. We may get away with dropping that
> these days but so far I'm not convinced it's worth the risk. So in this case
> I don't see anything here that couldn't happen with say DM device and thus
> I wouldn't really restrict the loop device functionality...

Sure, and LOOP_SET_CAPACITY might be used to grow a file system image
which the file system could then grow into.  Fair.

This is related to BLK_DEV_WRITE_MOUNTED=n which the syzkaller folks
have agreed to use to prevent noisy syzkaller reports.  We're seeing a
bunch of syzkaller reports now that syzkaller has been taught how to
use these loop ioctls and so we're seeing loop device hijinks.  Which
is fine; I can just start filtering any syzkaller report that uses
loop device ioctls as false positive noise and call it a day.
Unfortunately, that won't help deal with researchers that are taking
the syzkaller code and then sending reports without any dashboards or
reproducers.  :-(

However, I do think that if the file system has advertised that they
don't support random underlying block device hijinks, such as XFS for
example, we should honor this and disallow those "wild west" loop
device operations.  And perhaps we should similarly disallow them if
BLK_DEV_WRITE_MOUNTED=n.

						- Ted


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-07-21 13:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-10  8:01 [syzbot] [ext4?] kernel BUG in ext4_update_inline_data Moon Hee Lee
2025-07-10  8:23 ` syzbot
2025-07-17 14:59 ` [PATCH] ext4: do not BUG when INLINE_DATA_FL lacks system.data xattr Theodore Ts'o
2025-07-17 16:59   ` Moon Hee Lee
2025-07-17 19:53     ` Moon Hee Lee
2025-07-18  1:05     ` Theodore Ts'o
2025-07-21 12:49       ` Jan Kara
2025-07-21 13:51         ` Theodore Ts'o
  -- strict thread matches above, loose matches on Subject: below --
2025-07-07 16:32 [syzbot] [ext4?] kernel BUG in ext4_update_inline_data syzbot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).