On Thu 22-08-24 10:35:26, libaokun@xxxxxxxxxxxxxxx wrote: > From: Baokun Li <libaokun1@xxxxxxxxxx> > > As Ojaswin mentioned in Link, in ext4_ext_insert_extent(), if the path is > reallocated in ext4_ext_create_new_leaf(), we'll use the stale path and > cause UAF. Below is a sample trace with dummy values: > > ext4_ext_insert_extent > path = *ppath = 2000 > ext4_ext_create_new_leaf(ppath) > ext4_find_extent(ppath) > path = *ppath = 2000 > if (depth > path[0].p_maxdepth) > kfree(path = 2000); > *ppath = path = NULL; > path = kcalloc() = 3000 > *ppath = 3000; > return path; > /* here path is still 2000, UAF! */ > eh = path[depth].p_hdr > > ================================================================== > BUG: KASAN: slab-use-after-free in ext4_ext_insert_extent+0x26d4/0x3330 > Read of size 8 at addr ffff8881027bf7d0 by task kworker/u36:1/179 > CPU: 3 UID: 0 PID: 179 Comm: kworker/u6:1 Not tainted 6.11.0-rc2-dirty #866 > Call Trace: > <TASK> > ext4_ext_insert_extent+0x26d4/0x3330 > ext4_ext_map_blocks+0xe22/0x2d40 > ext4_map_blocks+0x71e/0x1700 > ext4_do_writepages+0x1290/0x2800 > [...] > > Allocated by task 179: > ext4_find_extent+0x81c/0x1f70 > ext4_ext_map_blocks+0x146/0x2d40 > ext4_map_blocks+0x71e/0x1700 > ext4_do_writepages+0x1290/0x2800 > ext4_writepages+0x26d/0x4e0 > do_writepages+0x175/0x700 > [...] > > Freed by task 179: > kfree+0xcb/0x240 > ext4_find_extent+0x7c0/0x1f70 > ext4_ext_insert_extent+0xa26/0x3330 > ext4_ext_map_blocks+0xe22/0x2d40 > ext4_map_blocks+0x71e/0x1700 > ext4_do_writepages+0x1290/0x2800 > ext4_writepages+0x26d/0x4e0 > do_writepages+0x175/0x700 > [...] > ================================================================== > > So use *ppath to update the path to avoid the above problem. > > Reported-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx> > Closes: https://lore.kernel.org/r/ZqyL6rmtwl6N4MWR@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx > Fixes: 10809df84a4d ("ext4: teach ext4_ext_find_extent() to realloc path if necessary") > Cc: stable@xxxxxxxxxx > Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@xxxxxxx> Honza > --- > fs/ext4/extents.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 5879aef159d8..91c6586afcca 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -2116,6 +2116,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > ppath, newext); > if (err) > goto cleanup; > + path = *ppath; > depth = ext_depth(inode); > eh = path[depth].p_hdr; > > -- > 2.39.2 > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR