| Message ID | 20220922120434.1294789-1-libaokun1@huawei.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | [v2] ext4: fix use-after-free in ext4_ext_shift_extents | expand |
On Thu 22-09-22 20:04:34, Baokun Li wrote: > If the starting position of our insert range happens to be in the hole > between the two ext4_extent_idx, because the lblk of the ext4_extent in > the previous ext4_extent_idx is always less than the start, which leads > to the "extent" variable access across the boundary, the following UAF is > triggered: > ================================================================== > BUG: KASAN: use-after-free in ext4_ext_shift_extents+0x257/0x790 > Read of size 4 at addr ffff88819807a008 by task fallocate/8010 > CPU: 3 PID: 8010 Comm: fallocate Tainted: G E 5.10.0+ #492 > Call Trace: > dump_stack+0x7d/0xa3 > print_address_description.constprop.0+0x1e/0x220 > kasan_report.cold+0x67/0x7f > ext4_ext_shift_extents+0x257/0x790 > ext4_insert_range+0x5b6/0x700 > ext4_fallocate+0x39e/0x3d0 > vfs_fallocate+0x26f/0x470 > ksys_fallocate+0x3a/0x70 > __x64_sys_fallocate+0x4f/0x60 > do_syscall_64+0x33/0x40 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > ================================================================== > > For right shifts, we can divide them into the following situations： > > 1. When the first ee_block of ext4_extent_idx is greater than or equal to > start, make right shifts directly from the first ee_block. > 1) If it is greater than start, we need to continue searching in the > previous ext4_extent_idx. > 2) If it is equal to start, we can exit the loop (iterator=NULL). > > 2. When the first ee_block of ext4_extent_idx is less than start, then > traverse from the last extent to find the first extent whose ee_block > is less than start. > 1) If extent is still the last extent after traversal, it means that > the last ee_block of ext4_extent_idx is less than start, that is, > start is located in the hole between idx and (idx+1), so we can > exit the loop directly (break) without right shifts. > 2) Otherwise, make right shifts at the corresponding position of the > found extent, and then exit the loop (iterator=NULL). > > Fixes: 331573febb6a ("ext4: Add support FALLOC_FL_INSERT_RANGE for fallocate") > Cc: stable@vger.kernel.org # v4.2+ > Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> > Signed-off-by: Baokun Li <libaokun1@huawei.com> Thanks for the fix! It looks good to me. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > V1->V2: > Initialize "ret" after the "again:" label to avoid return value mismatch. > Refactoring reduces cycles and makes code more readable. > > fs/ext4/extents.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index c148bb97b527..39c9f87de0be 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -5179,6 +5179,7 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle, > * and it is decreased till we reach start. > */ > again: > + ret = 0; > if (SHIFT == SHIFT_LEFT) > iterator = &start; > else > @@ -5222,14 +5223,21 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle, > ext4_ext_get_actual_len(extent); > } else { > extent = EXT_FIRST_EXTENT(path[depth].p_hdr); > - if (le32_to_cpu(extent->ee_block) > 0) > + if (le32_to_cpu(extent->ee_block) > start) > *iterator = le32_to_cpu(extent->ee_block) - 1; > - else > - /* Beginning is reached, end of the loop */ > + else if (le32_to_cpu(extent->ee_block) == start) > iterator = NULL; > - /* Update path extent in case we need to stop */ > - while (le32_to_cpu(extent->ee_block) < start) > + else { > + extent = EXT_LAST_EXTENT(path[depth].p_hdr); > + while (le32_to_cpu(extent->ee_block) >= start) > + extent--; > + > + if (extent == EXT_LAST_EXTENT(path[depth].p_hdr)) > + break; > + > extent++; > + iterator = NULL; > + } > path[depth].p_ext = extent; > } > ret = ext4_ext_shift_path_extents(path, shift, inode, > -- > 2.31.1 >
On Thu, 22 Sep 2022 20:04:34 +0800, Baokun Li wrote: > If the starting position of our insert range happens to be in the hole > between the two ext4_extent_idx, because the lblk of the ext4_extent in > the previous ext4_extent_idx is always less than the start, which leads > to the "extent" variable access across the boundary, the following UAF is > triggered: > ================================================================== > BUG: KASAN: use-after-free in ext4_ext_shift_extents+0x257/0x790 > Read of size 4 at addr ffff88819807a008 by task fallocate/8010 > CPU: 3 PID: 8010 Comm: fallocate Tainted: G E 5.10.0+ #492 > Call Trace: > dump_stack+0x7d/0xa3 > print_address_description.constprop.0+0x1e/0x220 > kasan_report.cold+0x67/0x7f > ext4_ext_shift_extents+0x257/0x790 > ext4_insert_range+0x5b6/0x700 > ext4_fallocate+0x39e/0x3d0 > vfs_fallocate+0x26f/0x470 > ksys_fallocate+0x3a/0x70 > __x64_sys_fallocate+0x4f/0x60 > do_syscall_64+0x33/0x40 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > ================================================================== > > [...] Applied, thanks! [1/1] ext4: fix use-after-free in ext4_ext_shift_extents (no commit info) Best regards,
On 2022/9/30 11:19, Theodore Ts'o wrote: > On Thu, 22 Sep 2022 20:04:34 +0800, Baokun Li wrote: >> If the starting position of our insert range happens to be in the hole >> between the two ext4_extent_idx, because the lblk of the ext4_extent in >> the previous ext4_extent_idx is always less than the start, which leads >> to the "extent" variable access across the boundary, the following UAF is >> triggered: >> ================================================================== >> BUG: KASAN: use-after-free in ext4_ext_shift_extents+0x257/0x790 >> Read of size 4 at addr ffff88819807a008 by task fallocate/8010 >> CPU: 3 PID: 8010 Comm: fallocate Tainted: G E 5.10.0+ #492 >> Call Trace: >> dump_stack+0x7d/0xa3 >> print_address_description.constprop.0+0x1e/0x220 >> kasan_report.cold+0x67/0x7f >> ext4_ext_shift_extents+0x257/0x790 >> ext4_insert_range+0x5b6/0x700 >> ext4_fallocate+0x39e/0x3d0 >> vfs_fallocate+0x26f/0x470 >> ksys_fallocate+0x3a/0x70 >> __x64_sys_fallocate+0x4f/0x60 >> do_syscall_64+0x33/0x40 >> entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> ================================================================== >> >> [...] > Applied, thanks! > > [1/1] ext4: fix use-after-free in ext4_ext_shift_extents > (no commit info) > > Best regards, Hi Theodore, Could you tell me why this patch has been applied, but there is no cmmit info, and the patch cannot be found on any branch?
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index c148bb97b527..39c9f87de0be 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -5179,6 +5179,7 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle, * and it is decreased till we reach start. */ again: + ret = 0; if (SHIFT == SHIFT_LEFT) iterator = &start; else @@ -5222,14 +5223,21 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle, ext4_ext_get_actual_len(extent); } else { extent = EXT_FIRST_EXTENT(path[depth].p_hdr); - if (le32_to_cpu(extent->ee_block) > 0) + if (le32_to_cpu(extent->ee_block) > start) *iterator = le32_to_cpu(extent->ee_block) - 1; - else - /* Beginning is reached, end of the loop */ + else if (le32_to_cpu(extent->ee_block) == start) iterator = NULL; - /* Update path extent in case we need to stop */ - while (le32_to_cpu(extent->ee_block) < start) + else { + extent = EXT_LAST_EXTENT(path[depth].p_hdr); + while (le32_to_cpu(extent->ee_block) >= start) + extent--; + + if (extent == EXT_LAST_EXTENT(path[depth].p_hdr)) + break; + extent++; + iterator = NULL; + } path[depth].p_ext = extent; } ret = ext4_ext_shift_path_extents(path, shift, inode,
If the starting position of our insert range happens to be in the hole between the two ext4_extent_idx, because the lblk of the ext4_extent in the previous ext4_extent_idx is always less than the start, which leads to the "extent" variable access across the boundary, the following UAF is triggered: ================================================================== BUG: KASAN: use-after-free in ext4_ext_shift_extents+0x257/0x790 Read of size 4 at addr ffff88819807a008 by task fallocate/8010 CPU: 3 PID: 8010 Comm: fallocate Tainted: G E 5.10.0+ #492 Call Trace: dump_stack+0x7d/0xa3 print_address_description.constprop.0+0x1e/0x220 kasan_report.cold+0x67/0x7f ext4_ext_shift_extents+0x257/0x790 ext4_insert_range+0x5b6/0x700 ext4_fallocate+0x39e/0x3d0 vfs_fallocate+0x26f/0x470 ksys_fallocate+0x3a/0x70 __x64_sys_fallocate+0x4f/0x60 do_syscall_64+0x33/0x40 entry_SYSCALL_64_after_hwframe+0x44/0xa9 ================================================================== For right shifts, we can divide them into the following situations： 1. When the first ee_block of ext4_extent_idx is greater than or equal to start, make right shifts directly from the first ee_block. 1) If it is greater than start, we need to continue searching in the previous ext4_extent_idx. 2) If it is equal to start, we can exit the loop (iterator=NULL). 2. When the first ee_block of ext4_extent_idx is less than start, then traverse from the last extent to find the first extent whose ee_block is less than start. 1) If extent is still the last extent after traversal, it means that the last ee_block of ext4_extent_idx is less than start, that is, start is located in the hole between idx and (idx+1), so we can exit the loop directly (break) without right shifts. 2) Otherwise, make right shifts at the corresponding position of the found extent, and then exit the loop (iterator=NULL). Fixes: 331573febb6a ("ext4: Add support FALLOC_FL_INSERT_RANGE for fallocate") Cc: stable@vger.kernel.org # v4.2+ Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> Signed-off-by: Baokun Li <libaokun1@huawei.com> --- V1->V2: Initialize "ret" after the "again:" label to avoid return value mismatch. Refactoring reduces cycles and makes code more readable. fs/ext4/extents.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)