On Thu 14-04-22 10:52:23, Ye Bin wrote:
> We got issue as follows:
> EXT4-fs (loop0): mounted filesystem without journal. Opts: ,errors=continue
> ext4_get_first_dir_block: bh->b_data=0xffff88810bee6000 len=34478
> ext4_get_first_dir_block: *parent_de=0xffff88810beee6ae bh->b_data=0xffff88810bee6000
> ext4_rename_dir_prepare: [1] parent_de=0xffff88810beee6ae
> ==================================================================
> BUG: KASAN: use-after-free in ext4_rename_dir_prepare+0x152/0x220
> Read of size 4 at addr ffff88810beee6ae by task rep/1895
>
> CPU: 13 PID: 1895 Comm: rep Not tainted 5.10.0+ #241
> Call Trace:
> dump_stack+0xbe/0xf9
> print_address_description.constprop.0+0x1e/0x220
> kasan_report.cold+0x37/0x7f
> ext4_rename_dir_prepare+0x152/0x220
> ext4_rename+0xf44/0x1ad0
> ext4_rename2+0x11c/0x170
> vfs_rename+0xa84/0x1440
> do_renameat2+0x683/0x8f0
> __x64_sys_renameat+0x53/0x60
> do_syscall_64+0x33/0x40
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x7f45a6fc41c9
> RSP: 002b:00007ffc5a470218 EFLAGS: 00000246 ORIG_RAX: 0000000000000108
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f45a6fc41c9
> RDX: 0000000000000005 RSI: 0000000020000180 RDI: 0000000000000005
> RBP: 00007ffc5a470240 R08: 00007ffc5a470160 R09: 0000000020000080
> R10: 00000000200001c0 R11: 0000000000000246 R12: 0000000000400bb0
> R13: 00007ffc5a470320 R14: 0000000000000000 R15: 0000000000000000
>
> The buggy address belongs to the page:
> page:00000000440015ce refcount:0 mapcount:0 mapping:0000000000000000 index:0x1 pfn:0x10beee
> flags: 0x200000000000000()
> raw: 0200000000000000 ffffea00043ff4c8 ffffea0004325608 0000000000000000
> raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff88810beee580: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ffff88810beee600: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> >ffff88810beee680: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ^
> ffff88810beee700: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ffff88810beee780: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ==================================================================
> Disabling lock debugging due to kernel taint
> ext4_rename_dir_prepare: [2] parent_de->inode=3537895424
> ext4_rename_dir_prepare: [3] dir=0xffff888124170140
> ext4_rename_dir_prepare: [4] ino=2
> ext4_rename_dir_prepare: ent->dir->i_ino=2 parent=-757071872
>
> Reason is first directory entry which 'rec_len' is 34478, then will get illegal
> parent entry. Now, we do not check directory entry after read directory block
> in 'ext4_get_first_dir_block'.
> To solve this issue, check directory entry in 'ext4_get_first_dir_block'.
>
> Signed-off-by: Ye Bin <yebin10@xxxxxxxxxx>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index e37da8d5cd0c..2f78544b1d47 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -3455,6 +3455,9 @@ static struct buffer_head *ext4_get_first_dir_block(handle_t *handle,
> struct buffer_head *bh;
>
> if (!ext4_has_inline_data(inode)) {
> + struct ext4_dir_entry_2 *de;
> + unsigned int offset;
> +
> /* The first directory block must not be a hole, so
> * treat it as DIRENT_HTREE
> */
> @@ -3463,9 +3466,28 @@ static struct buffer_head *ext4_get_first_dir_block(handle_t *handle,
> *retval = PTR_ERR(bh);
> return NULL;
> }
> - *parent_de = ext4_next_entry(
> - (struct ext4_dir_entry_2 *)bh->b_data,
> - inode->i_sb->s_blocksize);
> +
> + de = (struct ext4_dir_entry_2 *) bh->b_data;
> + if (ext4_check_dir_entry(inode, NULL, de, bh, bh->b_data,
> + bh->b_size, 0) ||
> + le32_to_cpu(de->inode) != inode->i_ino ||
> + strcmp(".", de->name)) {
> + ext4_warning_inode(inode, "directory missing '.'");
> + brelse(bh);
> + return NULL;
> + }
> + offset = ext4_rec_len_from_disk(de->rec_len,
> + inode->i_sb->s_blocksize);
> + de = ext4_next_entry(de, inode->i_sb->s_blocksize);
> + if (ext4_check_dir_entry(inode, NULL, de, bh, bh->b_data,
> + bh->b_size, offset) ||
> + le32_to_cpu(de->inode) == 0 || strcmp("..", de->name)) {
> + ext4_warning_inode(inode, "directory missing '..'");
> + brelse(bh);
> + return NULL;
> + }
> + *parent_de = de;
> +
> return bh;
> }
>
> --
> 2.31.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
