ext4: drop ppath from ext4_ext_replay_update_ex() to avoid double-free

When calling ext4_force_split_extent_at() in ext4_ext_replay_update_ex(),
the 'ppath' is updated but it is the 'path' that is freed, thus potentially
triggering a double-free in the following process:

ext4_ext_replay_update_ex
  ppath = path
  ext4_force_split_extent_at(&ppath)
    ext4_split_extent_at
      ext4_ext_insert_extent
        ext4_ext_create_new_leaf
          ext4_ext_grow_indepth
            ext4_find_extent
              if (depth > path[0].p_maxdepth)
                kfree(path)                 ---> path First freed
                *orig_path = path = NULL    ---> null ppath
  kfree(path)                               ---> path double-free !!!

So drop the unnecessary ppath and use path directly to avoid this problem.
And use ext4_find_extent() directly to update path, avoiding unnecessary
memory allocation and freeing. Also, propagate the error returned by
ext4_find_extent() instead of using strange error codes.

Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
Cc: stable@kernel.org
Signed-off-by: Baokun Li <libaokun1@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Tested-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Link: https://patch.msgid.link/20240822023545.1994557-8-libaokun@huaweicloud.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index b431fcfb6ef8ed3b994867783a0202e900eb5afd..b8dd2fdd90d9f1f04e0b31aece5a1fd65d9b544e 100644 (file)
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5885,7 +5885,7 @@ out:
 int ext4_ext_replay_update_ex(struct inode *inode, ext4_lblk_t start,
                              int len, int unwritten, ext4_fsblk_t pblk)
 {
-       struct ext4_ext_path *path = NULL, *ppath;
+       struct ext4_ext_path *path;
        struct ext4_extent *ex;
        int ret;
 
@@ -5901,30 +5901,29 @@ int ext4_ext_replay_update_ex(struct inode *inode, ext4_lblk_t start,
        if (le32_to_cpu(ex->ee_block) != start ||
                ext4_ext_get_actual_len(ex) != len) {
                /* We need to split this extent to match our extent first */
-               ppath = path;
                down_write(&EXT4_I(inode)->i_data_sem);
-               ret = ext4_force_split_extent_at(NULL, inode, &ppath, start, 1);
+               ret = ext4_force_split_extent_at(NULL, inode, &path, start, 1);
                up_write(&EXT4_I(inode)->i_data_sem);
                if (ret)
                        goto out;
-               kfree(path);
-               path = ext4_find_extent(inode, start, NULL, 0);
+
+               path = ext4_find_extent(inode, start, &path, 0);
                if (IS_ERR(path))
-                       return -1;
-               ppath = path;
+                       return PTR_ERR(path);
                ex = path[path->p_depth].p_ext;
                WARN_ON(le32_to_cpu(ex->ee_block) != start);
+
                if (ext4_ext_get_actual_len(ex) != len) {
                        down_write(&EXT4_I(inode)->i_data_sem);
-                       ret = ext4_force_split_extent_at(NULL, inode, &ppath,
+                       ret = ext4_force_split_extent_at(NULL, inode, &path,
                                                         start + len, 1);
                        up_write(&EXT4_I(inode)->i_data_sem);
                        if (ret)
                                goto out;
-                       kfree(path);
-                       path = ext4_find_extent(inode, start, NULL, 0);
+
+                       path = ext4_find_extent(inode, start, &path, 0);
                        if (IS_ERR(path))
-                               return -EINVAL;
+                               return PTR_ERR(path);
                        ex = path[path->p_depth].p_ext;
                }
        }