2023-06-13 00:12:58

by syzbot

[permalink] [raw]
Subject: [syzbot] [ext4?] UBSAN: shift-out-of-bounds in ext2_fill_super (2)

Hello,

syzbot found the following issue on:

HEAD commit: 908f31f2a05b Merge branch 'for-next/core', remote-tracking..
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci
console output: https://syzkaller.appspot.com/x/log.txt?x=124e9053280000
kernel config: https://syzkaller.appspot.com/x/.config?x=c1058fe68f4b7b2c
dashboard link: https://syzkaller.appspot.com/bug?extid=af5e10f73dbff48f70af
compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
userspace arch: arm64
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10f66595280000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14abde43280000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/87d095820229/disk-908f31f2.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/a1bf67af9675/vmlinux-908f31f2.xz
kernel image: https://storage.googleapis.com/syzbot-assets/7784a88b37e8/Image-908f31f2.gz.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/2816e591e0fa/mount_0.gz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: [email protected]

memfd_create() without MFD_EXEC nor MFD_NOEXEC_SEAL, pid=5969 'syz-executor354'
loop0: detected capacity change from 0 to 512
EXT2-fs (loop0): (no)user_xattr optionsnot supported
================================================================================
UBSAN: shift-out-of-bounds in fs/ext2/super.c:1015:40
shift exponent 63 is too large for 32-bit type 'int'
CPU: 0 PID: 5969 Comm: syz-executor354 Not tainted 6.4.0-rc4-syzkaller-g908f31f2a05b #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/25/2023
Call trace:
dump_backtrace+0x1b8/0x1e4 arch/arm64/kernel/stacktrace.c:233
show_stack+0x2c/0x44 arch/arm64/kernel/stacktrace.c:240
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xd0/0x124 lib/dump_stack.c:106
dump_stack+0x1c/0x28 lib/dump_stack.c:113
ubsan_epilogue lib/ubsan.c:217 [inline]
__ubsan_handle_shift_out_of_bounds+0x2f4/0x36c lib/ubsan.c:387
ext2_fill_super+0x2270/0x2450 fs/ext2/super.c:1015
mount_bdev+0x274/0x370 fs/super.c:1380
ext2_mount+0x44/0x58 fs/ext2/super.c:1491
legacy_get_tree+0xd4/0x16c fs/fs_context.c:610
vfs_get_tree+0x90/0x274 fs/super.c:1510
do_new_mount+0x25c/0x8c4 fs/namespace.c:3039
path_mount+0x590/0xe04 fs/namespace.c:3369
do_mount fs/namespace.c:3382 [inline]
__do_sys_mount fs/namespace.c:3591 [inline]
__se_sys_mount fs/namespace.c:3568 [inline]
__arm64_sys_mount+0x45c/0x594 fs/namespace.c:3568
__invoke_syscall arch/arm64/kernel/syscall.c:38 [inline]
invoke_syscall+0x98/0x2c0 arch/arm64/kernel/syscall.c:52
el0_svc_common+0x138/0x244 arch/arm64/kernel/syscall.c:142
do_el0_svc+0x64/0x198 arch/arm64/kernel/syscall.c:191
el0_svc+0x4c/0x160 arch/arm64/kernel/entry-common.c:647
el0t_64_sync_handler+0x84/0xfc arch/arm64/kernel/entry-common.c:665
el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:591
================================================================================
EXT2-fs (loop0): error: can't find an ext2 filesystem on dev loop0.


---
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 [email protected].

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the bug is already fixed, 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 change bug's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

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

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


2023-06-13 18:09:38

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [syzbot] [ext4?] UBSAN: shift-out-of-bounds in ext2_fill_super (2)

I wonder if we should have a separate syzkaller subsystem for ext2 (as
distinct from ext4)? The syz reproducer seems to know that it should
be mounting using ext2, but also calls it an ext4 file system, which
is a bit weird. I'm guessing there is something specific about the
syzkaller internals which might not make this be practical, but I
thought I should ask.

From the syz reproducer:

syz_mount_image$ext4(&(0x7f0000000100)='ext2\x00', ...)

More generally, there are a series of changes that were made to make
ext4 to make it more robust against maliciously fuzzed superblocks,
but we haven't necessarily made sure the same analogous changes have
been made to ext2. I'm not sure how critical this is in practice,
since most distributions don't actually compile fs/ext2 and instead
use CONFIG_EXT4_USE_FOR_EXT2 instead. However, while we maintain ext2
as a sample "simple" modern file system, I guess we should try to make
sure we do carry those fixes over.

Jan, as the ext2 maintainer, do you have an opinion?

- Ted

On Mon, Jun 12, 2023 at 04:58:57PM -0700, syzbot wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 908f31f2a05b Merge branch 'for-next/core', remote-tracking..
> git tree: git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci
> console output: https://syzkaller.appspot.com/x/log.txt?x=124e9053280000
> kernel config: https://syzkaller.appspot.com/x/.config?x=c1058fe68f4b7b2c
> dashboard link: https://syzkaller.appspot.com/bug?extid=af5e10f73dbff48f70af
> compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
> userspace arch: arm64
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10f66595280000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14abde43280000
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/87d095820229/disk-908f31f2.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/a1bf67af9675/vmlinux-908f31f2.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/7784a88b37e8/Image-908f31f2.gz.xz
> mounted in repro: https://storage.googleapis.com/syzbot-assets/2816e591e0fa/mount_0.gz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: [email protected]
>
> memfd_create() without MFD_EXEC nor MFD_NOEXEC_SEAL, pid=5969 'syz-executor354'
> loop0: detected capacity change from 0 to 512
> EXT2-fs (loop0): (no)user_xattr optionsnot supported
> ================================================================================
> UBSAN: shift-out-of-bounds in fs/ext2/super.c:1015:40
> shift exponent 63 is too large for 32-bit type 'int'
> CPU: 0 PID: 5969 Comm: syz-executor354 Not tainted 6.4.0-rc4-syzkaller-g908f31f2a05b #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/25/2023
> Call trace:
> dump_backtrace+0x1b8/0x1e4 arch/arm64/kernel/stacktrace.c:233
> show_stack+0x2c/0x44 arch/arm64/kernel/stacktrace.c:240
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0xd0/0x124 lib/dump_stack.c:106
> dump_stack+0x1c/0x28 lib/dump_stack.c:113
> ubsan_epilogue lib/ubsan.c:217 [inline]
> __ubsan_handle_shift_out_of_bounds+0x2f4/0x36c lib/ubsan.c:387
> ext2_fill_super+0x2270/0x2450 fs/ext2/super.c:1015
> mount_bdev+0x274/0x370 fs/super.c:1380
> ext2_mount+0x44/0x58 fs/ext2/super.c:1491
> legacy_get_tree+0xd4/0x16c fs/fs_context.c:610
> vfs_get_tree+0x90/0x274 fs/super.c:1510
> do_new_mount+0x25c/0x8c4 fs/namespace.c:3039
> path_mount+0x590/0xe04 fs/namespace.c:3369
> do_mount fs/namespace.c:3382 [inline]
> __do_sys_mount fs/namespace.c:3591 [inline]
> __se_sys_mount fs/namespace.c:3568 [inline]
> __arm64_sys_mount+0x45c/0x594 fs/namespace.c:3568
> __invoke_syscall arch/arm64/kernel/syscall.c:38 [inline]
> invoke_syscall+0x98/0x2c0 arch/arm64/kernel/syscall.c:52
> el0_svc_common+0x138/0x244 arch/arm64/kernel/syscall.c:142
> do_el0_svc+0x64/0x198 arch/arm64/kernel/syscall.c:191
> el0_svc+0x4c/0x160 arch/arm64/kernel/entry-common.c:647
> el0t_64_sync_handler+0x84/0xfc arch/arm64/kernel/entry-common.c:665
> el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:591
> ================================================================================
> EXT2-fs (loop0): error: can't find an ext2 filesystem on dev loop0.
>
>
> ---
> 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 [email protected].
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>
> If the bug is already fixed, 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 change bug's subsystems, reply with:
> #syz set subsystems: new-subsystem
> (See the list of subsystem names on the web dashboard)
>
> If the bug is a duplicate of another bug, reply with:
> #syz dup: exact-subject-of-another-report
>
> If you want to undo deduplication, reply with:
> #syz undup

2023-06-13 21:12:28

by Jan Kara

[permalink] [raw]
Subject: Re: [syzbot] [ext4?] UBSAN: shift-out-of-bounds in ext2_fill_super (2)

On Tue 13-06-23 14:01:03, Theodore Ts'o wrote:
> I wonder if we should have a separate syzkaller subsystem for ext2 (as
> distinct from ext4)? The syz reproducer seems to know that it should
> be mounting using ext2, but also calls it an ext4 file system, which
> is a bit weird. I'm guessing there is something specific about the
> syzkaller internals which might not make this be practical, but I
> thought I should ask.

Yeah, having ext2 driver as a separate subsystem makes sense to me since it
is completely different codebase.

> From the syz reproducer:
>
> syz_mount_image$ext4(&(0x7f0000000100)='ext2\x00', ...)
>
> More generally, there are a series of changes that were made to make
> ext4 to make it more robust against maliciously fuzzed superblocks,
> but we haven't necessarily made sure the same analogous changes have
> been made to ext2. I'm not sure how critical this is in practice,
> since most distributions don't actually compile fs/ext2 and instead
> use CONFIG_EXT4_USE_FOR_EXT2 instead. However, while we maintain ext2
> as a sample "simple" modern file system, I guess we should try to make
> sure we do carry those fixes over.
>
> Jan, as the ext2 maintainer, do you have an opinion?

I agree, I try to fix these problems when syzbot finds them. For this one,
I've already sent a fix [1] (dropping remains of fragments support from ext2).

Honza

[1] https://lore.kernel.org/all/[email protected]

--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-06-15 23:31:35

by Dave Chinner

[permalink] [raw]
Subject: Re: [syzbot] [ext4?] UBSAN: shift-out-of-bounds in ext2_fill_super (2)

On Tue, Jun 13, 2023 at 02:01:03PM -0400, Theodore Ts'o wrote:
> I wonder if we should have a separate syzkaller subsystem for ext2 (as
> distinct from ext4)? The syz reproducer seems to know that it should
> be mounting using ext2, but also calls it an ext4 file system, which
> is a bit weird. I'm guessing there is something specific about the
> syzkaller internals which might not make this be practical, but I
> thought I should ask.
>
> From the syz reproducer:
>
> syz_mount_image$ext4(&(0x7f0000000100)='ext2\x00', ...)
>
> More generally, there are a series of changes that were made to make
> ext4 to make it more robust against maliciously fuzzed superblocks,
> but we haven't necessarily made sure the same analogous changes have
> been made to ext2. I'm not sure how critical this is in practice,
> since most distributions don't actually compile fs/ext2 and instead
> use CONFIG_EXT4_USE_FOR_EXT2 instead. However, while we maintain ext2
> as a sample "simple" modern file system, I guess we should try to make
> sure we do carry those fixes over.

Hmmmm.

Modern filesystems are crash resilient, based on extents and are
using/moving to folios+iomap - calling a non-journalled, indirect
block indexed, bufferhead based code base (that nobody is really
using in production) "modern" seems like a real stretch.

I have my doubts that maintaining fs/ext2 is providing much benefit
to anyone. The code base is in the git history if anyone wants to
study it, so it's not like we have to keep it active in the tree for
it to remain a code base that people can learn from.

Therefore, given the current push to sideline/remove bufferheads
from the kernel, should we simply deprecate fs/ext2 and then remove
it in a year or two like we're doing with reiser to reduce our
future maintenance and/or conversion burden?

Or just remove it right now and simply make CONFIG_FS_EXT2 select
CONFIG_EXT4_USE_FOR_EXT2?

/Devil's Advocate

Cheers,

Dave.
--
Dave Chinner
[email protected]

2023-06-16 16:04:27

by Jan Kara

[permalink] [raw]
Subject: Re: [syzbot] [ext4?] UBSAN: shift-out-of-bounds in ext2_fill_super (2)

On Fri 16-06-23 09:24:12, Dave Chinner wrote:
> On Tue, Jun 13, 2023 at 02:01:03PM -0400, Theodore Ts'o wrote:
> > I wonder if we should have a separate syzkaller subsystem for ext2 (as
> > distinct from ext4)? The syz reproducer seems to know that it should
> > be mounting using ext2, but also calls it an ext4 file system, which
> > is a bit weird. I'm guessing there is something specific about the
> > syzkaller internals which might not make this be practical, but I
> > thought I should ask.
> >
> > From the syz reproducer:
> >
> > syz_mount_image$ext4(&(0x7f0000000100)='ext2\x00', ...)
> >
> > More generally, there are a series of changes that were made to make
> > ext4 to make it more robust against maliciously fuzzed superblocks,
> > but we haven't necessarily made sure the same analogous changes have
> > been made to ext2. I'm not sure how critical this is in practice,
> > since most distributions don't actually compile fs/ext2 and instead
> > use CONFIG_EXT4_USE_FOR_EXT2 instead. However, while we maintain ext2
> > as a sample "simple" modern file system, I guess we should try to make
> > sure we do carry those fixes over.
>
> Hmmmm.
>
> Modern filesystems are crash resilient, based on extents and are
> using/moving to folios+iomap - calling a non-journalled, indirect
> block indexed, bufferhead based code base (that nobody is really
> using in production) "modern" seems like a real stretch.

Yeah, modern is a stretch. But I guess what Ted meant is that we try to
keep ext2 reasonably uptodate with various infrastructure changes. We have
now queued conversion of ext2 direct IO path to iomap and are looking into
converting buffered IO path to iomap as a sample to get experience for ext4
conversion and also conversion of other "simple" filesystems.

> I have my doubts that maintaining fs/ext2 is providing much benefit
> to anyone. The code base is in the git history if anyone wants to
> study it, so it's not like we have to keep it active in the tree for
> it to remain a code base that people can learn from.
>
> Therefore, given the current push to sideline/remove bufferheads
> from the kernel, should we simply deprecate fs/ext2 and then remove
> it in a year or two like we're doing with reiser to reduce our
> future maintenance and/or conversion burden?

That's a fair remark and if there is a general sentiment that ext2 codebase
is not really useful, I certainly won't mind having one less fs to
maintain. I'd just note that having ext2 in git history will not provide
very useful insight into how various current infrastructure changes could
be implemented for simple filesystems.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR