linux-kernel - Re: [syzbot] [hardening?] [mm?] BUG: bad usercopy in vfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGudoHFszbdKkPibU_ynbj_PSyHewxTxWT_-hR3=8GHa46ebaQ@mail.gmail.com>
Date: Tue, 4 Feb 2025 16:58:10 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: Kees Cook <kees@...nel.org>
Cc: syzbot <syzbot+48a99e426f29859818c0@...kaller.appspotmail.com>, 
	"Theodore Ts'o" <tytso@....edu>, akpm@...ux-foundation.org, brauner@...nel.org, 
	gustavoars@...nel.org, linux-hardening@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, 
	syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [hardening?] [mm?] BUG: bad usercopy in vfs_readlink

On Tue, Feb 4, 2025 at 4:30 PM Kees Cook <kees@...nel.org> wrote:
>
> On Tue, Feb 04, 2025 at 12:38:30PM +0100, Mateusz Guzik wrote:
> > On Tue, Feb 4, 2025 at 10:46 AM syzbot
> > <syzbot+48a99e426f29859818c0@...kaller.appspotmail.com> wrote:
> > >
> > > Hello,
> > >
> > > syzbot found the following issue on:
> > >
> > > HEAD commit:    69b8923f5003 Merge tag 'for-linus-6.14-ofs4' of git://git...
> > > git tree:       upstream
> > > console+strace: https://syzkaller.appspot.com/x/log.txt?x=1258aeb0580000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=57ab43c279fa614d
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=48a99e426f29859818c0
> > > 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=15825724580000
> > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1658aeb0580000
> > >
> > > Downloadable assets:
> > > disk image: https://storage.googleapis.com/syzbot-assets/ea84ac864e92/disk-69b8923f.raw.xz
> > > vmlinux: https://storage.googleapis.com/syzbot-assets/6a465997b4e0/vmlinux-69b8923f.xz
> > > kernel image: https://storage.googleapis.com/syzbot-assets/d72b67b2bd15/bzImage-69b8923f.xz
> > > mounted in repro: https://storage.googleapis.com/syzbot-assets/7c2919610764/mount_0.gz
> > >
> > > The issue was bisected to:
> > >
> > > commit bae80473f7b0b25772619e7692019b1549d4a82c
> > > Author: Mateusz Guzik <mjguzik@...il.com>
> > > Date:   Wed Nov 20 11:20:35 2024 +0000
> > >
> > >     ext4: use inode_set_cached_link()
> > >
> >
> > This looks like a case of a deliberately corrupted image.
> >
> > I added back a debug printk I used when writing the patch before. For
> > correct cases it reports:
> >
> > [   19.574861] __ext4_iget: inode ff18d9bec05977c8 [/etc/environment]
> > i_size 16 strlen 16
> > [   19.585987] __ext4_iget: inode ff18d9bed6f25c68
> > [/etc/alternatives/awk] i_size 21 strlen 21
> > [   19.590419] __ext4_iget: inode ff18d9bed6f24108 [/usr/bin/gawk]
> > i_size 13 strlen 13
> > [   19.592199] __ext4_iget: inode ff18d9bed6abeea8
> > [libassuan.so.0.8.5] i_size 18 strlen 18
> > [   19.593804] __ext4_iget: inode ff18d9bed6f29368
> > [libsigsegv.so.2.0.7] i_size 19 strlen 19
> >
> > etc.
> >
> > However, the bogus case is:
> > [   52.161476] __ext4_iget: inode ff18d9bed1cc2a38
> > [/tmp/syz-imagegen43743633/file0/file0] i_size 131109
> > strlen 37
> >
> > That is i_size is out of sync with strlen.
> >
> > Prior to my patch this happened to work because the copying was in
> > fact issuing strlen to find the size every time.
> >
> > Given this code in fs/ext4/inode.c:
> >   5008                 } else if (ext4_inode_is_fast_symlink(inode)) {
> >   5009                         inode->i_op = &ext4_fast_symlink_inode_operations;
> >   5010                         nd_terminate_link(ei->i_data, inode->i_size,
> >   5011                                 sizeof(ei->i_data) - 1);
> >
> > To me this looks like a pre-existing bug in ext4 which just happened
>
> This gets clamped to i_data size, though, so I don't see a bug. Is there
> still a code path where i_size is getting used? It looks like the buffer
> overflow was introduced with bae80473f7b0 ("ext4: use inode_set_cached_link()"),
> more details below...
>
> > to get exposed with:
> >
> >   5012                         inode_set_cached_link(inode, (char *)ei->i_data,
> >   5013                                               inode->i_size);
>
> The sanity checker said:
> > usercopy: Kernel memory exposure attempt detected from SLUB object 'ext4_inode_cache' (offset 0, size 176)!
>
> The cache was created to make only the i_data field visible:
>
>         ext4_inode_cachep = kmem_cache_create_usercopy("ext4_inode_cache",
>                                 sizeof(struct ext4_inode_info), 0,
>                                 SLAB_RECLAIM_ACCOUNT | SLAB_ACCOUNT,
>                                 offsetof(struct ext4_inode_info, i_data),
>                                 sizeof_field(struct ext4_inode_info, i_data),
>                                 init_once);
>
> which is 15 bytes, at offset 0:
>
> struct ext4_inode_info {
>         __le32  i_data[15];     /* unconverted */
>         __u32   i_dtime;
>
> So, yes, this seems like a buffer overflow caught by usercopy, where the
> bug was as described above. I don't think there was an existing flaw in
> ext4, though?
>

I don't follow. Are you claiming the cached symlinks can only be up to
15 sizeof(i_data) in size?

In the long above you can see lengths bigger than that.

I had the vm still running, dmesg shows some more printks with lengths
above that:
[  695.648078] __ext4_iget: inode ff18d9bed33c3c78
[../Pacific/Pago_Pago] i_size 20 strlen 20
[  695.650647] __ext4_iget: inode ff18d9bed33c60f8
[../America/Los_Angeles] i_size 22 strlen 22
[  695.653160] __ext4_iget: inode ff18d9bec8be8128 [../America/Denver]
i_size 17 strlen 17
[  695.656716] __ext4_iget: inode ff18d9bed325dc68
[../America/Detroit] i_size 18 strlen 18
[  695.660450] __ext4_iget: inode ff18d9bed325ca28
[../America/Indiana/Knox] i_size 23 strlen 23
[  695.664270] __ext4_iget: inode ff18d9bed325c108
[../Pacific/Honolulu] i_size 19 strlen 19
[  695.666569] __ext4_iget: inode ff18d9bed325aec8
[../America/Indiana/Indianapolis] i_size 31 strlen 31

Note with and without the patch there is copy_to_user from that area
taking place.

Regardless, as you can see all the symlinks so far have i_size lining
up with strlen.

The only case which does not is the (presumably intentionally
corrupted) fs image from syzbot. Sounds like the link should be
rejected by ext4 instead?

Again I can do that strlen() call, but it seems like papering over the
problem for me.

-- 
Mateusz Guzik <mjguzik gmail.com>