On Fri 04-04-25 08:28:05, Artem Sadovnikov wrote:
> Syzkaller detected a use-after-free issue in ext4_insert_dentry that was
> caused by out-of-bounds access due to incorrect splitting in do_split.
>
> BUG: KASAN: use-after-free in ext4_insert_dentry+0x36a/0x6d0 fs/ext4/namei.c:2109
> Write of size 251 at addr ffff888074572f14 by task syz-executor335/5847
>
> CPU: 0 UID: 0 PID: 5847 Comm: syz-executor335 Not tainted 6.12.0-rc6-syzkaller-00318-ga9cda7c0ffed #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/30/2024
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:94 [inline]
> dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
> print_address_description mm/kasan/report.c:377 [inline]
> print_report+0x169/0x550 mm/kasan/report.c:488
> kasan_report+0x143/0x180 mm/kasan/report.c:601
> kasan_check_range+0x282/0x290 mm/kasan/generic.c:189
> __asan_memcpy+0x40/0x70 mm/kasan/shadow.c:106
> ext4_insert_dentry+0x36a/0x6d0 fs/ext4/namei.c:2109
> add_dirent_to_buf+0x3d9/0x750 fs/ext4/namei.c:2154
> make_indexed_dir+0xf98/0x1600 fs/ext4/namei.c:2351
> ext4_add_entry+0x222a/0x25d0 fs/ext4/namei.c:2455
> ext4_add_nondir+0x8d/0x290 fs/ext4/namei.c:2796
> ext4_symlink+0x920/0xb50 fs/ext4/namei.c:3431
> vfs_symlink+0x137/0x2e0 fs/namei.c:4615
> do_symlinkat+0x222/0x3a0 fs/namei.c:4641
> __do_sys_symlink fs/namei.c:4662 [inline]
> __se_sys_symlink fs/namei.c:4660 [inline]
> __x64_sys_symlink+0x7a/0x90 fs/namei.c:4660
> 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
> </TASK>
>
> The following loop is located right above 'if' statement.
>
> for (i = count-1; i >= 0; i--) {
> /* is more than half of this entry in 2nd half of the block? */
> if (size + map[i].size/2 > blocksize/2)
> break;
> size += map[i].size;
> move++;
> }
>
> 'i' in this case could go down to -1, in which case sum of active entries
> wouldn't exceed half the block size, but previous behaviour would also do
> split in half if sum would exceed at the very last block, which in case of
> having too many long name files in a single block could lead to
> out-of-bounds access and following use-after-free.
Thanks for debugging this! The fix looks good, but I'm still failing to see
the use-after-free / end-of-buffer issue. If we wrongly split to two parts
count/2 each, then dx_move_dirents() and dx_pack_dirents() seem to still
work correctly. Just they will make too small amount of space in bh but
still at least one dir entry gets moved? Following add_dirent_to_buf() is
more likely to fail due to ENOSPC but still I don't see the buffer overrun
issue? Can you please tell me what I'm missing? Thanks!
Anyway, since the fix looks obviously correct feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
>
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 5872331b3d91 ("ext4: fix potential negative array index in do_split()")
> Signed-off-by: Artem Sadovnikov <a.sadovnikov@xxxxxxxxx>
> ---
> fs/ext4/namei.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index cb5cb33b1d91..e9712e64ec8f 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1971,7 +1971,7 @@ static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
> * split it in half by count; each resulting block will have at least
> * half the space free.
> */
> - if (i > 0)
> + if (i >= 0)
> split = count - move;
> else
> split = count/2;
> --
> 2.43.0
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
