Re: [PATCH 00/20] ext4: some bugfixes and cleanups for ext4 extents path

[Ojaswin Mujoo]

On Wed, Jul 10, 2024 at 12:06:34PM +0800, libaokun@xxxxxxxxxxxxxxx wrote:
> From: Baokun Li <libaokun1@xxxxxxxxxx>
> 
> Hi all!
> 
> This patch series is a hardening of ext4 extents path related code.
> The following is a brief overview of the patches, see the patches for
> more details.
> 
> Patch 1-2: Refactor ext4_ext_rm_idx() as suggested by Jan, and add
> appropriate error handling branches to ext4_ext_rm_idx() and
> ext4_ext_correct_indexes() to avoid inconsistent extents tree.
>  PS: This comes from the previous work of my colleague zhanchengbin
>  (see link), who is no longer in charge of these and I have taken over.
>  Link: https://lore.kernel.org/r/20230213080514.535568-3-zhanchengbin1@xxxxxxxxxx/
> 
> Patch 3-4: Fix an issue that caused p_bh to be released twice if it wasn't
> set to NULL after path->p_bh was released. And add a helper function after
> the quick fix to prevent this from happening again.
> 
> Patch 5-7: Quick fixes for use-after-free and double-free problems caused
> by mixing path(pointer to an extent path) and ppath(pointer to an extent
> path pointer).
> 
> Patch 8-19: Now the use of path and ppath is so confusing that we can
> trigger use-after-free or double-free by accessing a stale pointer, or
> we can get a memory leak by forgetting to update ppath. And it's very
> difficult to read the code. So to make the code more readable, get rid
> of ppath and pass path between functions uniformly to avoid these risks.
> 
> Patch 20: Reduces the consumption of unnecessary memory operations by
> avoiding repetitive allocation and release paths.
> 
> "kvm-xfstests -c ext4/all -g auto" has been executed with no new failures.
> 
> Comments and questions are, as always, welcome.
> Please let me know what you think.
> 
> Thanks,
> Baokun
> 
> Baokun Li (20):
>   ext4: refactor ext4_ext_rm_idx() to index 'path'
>   ext4: prevent partial update of the extents path
>   ext4: fix double brelse() the buffer of the extents path
>   ext4: add new ext4_ext_path_brelse() helper
>   ext4: fix slab-use-after-free in ext4_split_extent_at()
>   ext4: avoid use-after-free in ext4_ext_show_leaf()
>   ext4: drop ppath from ext4_ext_replay_update_ex() to avoid double-free
>   ext4: get rid of ppath in ext4_find_extent()
>   ext4: get rid of ppath in get_ext_path()
>   ext4: get rid of ppath in ext4_ext_create_new_leaf()
>   ext4: get rid of ppath in ext4_ext_insert_extent()
>   ext4: get rid of ppath in ext4_split_extent_at()
>   ext4: get rid of ppath in ext4_force_split_extent_at()
>   ext4: get rid of ppath in ext4_split_extent()
>   ext4: get rid of ppath in ext4_split_convert_extents()
>   ext4: get rid of ppath in ext4_convert_unwritten_extents_endio()
>   ext4: get rid of ppath in ext4_ext_convert_to_initialized()
>   ext4: get rid of ppath in ext4_ext_handle_unwritten_extents()
>   ext4: get rid of ppath in convert_initialized_extent()
>   ext4: avoid unnecessary extent path frees and allocations
> 
>  fs/ext4/ext4.h        |   9 +-
>  fs/ext4/extents.c     | 746 +++++++++++++++++++++++-------------------
>  fs/ext4/fast_commit.c |  17 +-
>  fs/ext4/migrate.c     |   5 +-
>  fs/ext4/move_extent.c |  36 +-
>  5 files changed, 439 insertions(+), 374 deletions(-)
> 
> -- 
> 2.39.2

Tested kvm-xfstests auto on Powerpc with 4k, 64k and adv and no
new regressions are seen.

Feel free to add:

Tested-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>

Regards,
Ojaswin