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@xxxxxxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx> > > 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? > However, if the strlen thing is indeed the source of truth, the > inode_set_cached_link call can be trivially patched to use it instead > of i_size. Agreed. Please CC me and I can review it. :) -Kees -- Kees Cook
