FAILED: patch "[PATCH] ext4: fix slab-use-after-free in ext4_split_extent_at()" failed to apply to 4.19-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-4.19.y
git checkout FETCH_HEAD
git cherry-pick -x c26ab35702f8cd0cdc78f96aa5856bfb77be798f
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2024100717-untrue-mockup-9fb4@gregkh' --subject-prefix 'PATCH 4.19.y' HEAD^..
Possible dependencies:
c26ab35702f8 ("ext4: fix slab-use-after-free in ext4_split_extent_at()") 082cd4ec240b ("ext4: fix bug on in ext4_es_cache_extent as ext4_split_extent_at failed")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From c26ab35702f8cd0cdc78f96aa5856bfb77be798f Mon Sep 17 00:00:00 2001
From: Baokun Li <[email protected]>
Date: Thu, 22 Aug 2024 10:35:23 +0800
Subject: [PATCH] ext4: fix slab-use-after-free in ext4_split_extent_at()
We hit the following use-after-free:
==================================================================
BUG: KASAN: slab-use-after-free in ext4_split_extent_at+0xba8/0xcc0 Read of size 2 at addr ffff88810548ed08 by task kworker/u20:0/40 CPU: 0 PID: 40 Comm: kworker/u20:0 Not tainted 6.9.0-dirty #724 Call Trace: <TASK> kasan_report+0x93/0xc0 ext4_split_extent_at+0xba8/0xcc0 ext4_split_extent.isra.0+0x18f/0x500 ext4_split_convert_extents+0x275/0x750 ext4_ext_handle_unwritten_extents+0x73e/0x1580 ext4_ext_map_blocks+0xe20/0x2dc0 ext4_map_blocks+0x724/0x1700 ext4_do_writepages+0x12d6/0x2a70
[...]
Allocated by task 40: __kmalloc_noprof+0x1ac/0x480 ext4_find_extent+0xf3b/0x1e70 ext4_ext_map_blocks+0x188/0x2dc0 ext4_map_blocks+0x724/0x1700 ext4_do_writepages+0x12d6/0x2a70
[...]
Freed by task 40: kfree+0xf1/0x2b0 ext4_find_extent+0xa71/0x1e70 ext4_ext_insert_extent+0xa22/0x3260 ext4_split_extent_at+0x3ef/0xcc0 ext4_split_extent.isra.0+0x18f/0x500 ext4_split_convert_extents+0x275/0x750 ext4_ext_handle_unwritten_extents+0x73e/0x1580 ext4_ext_map_blocks+0xe20/0x2dc0 ext4_map_blocks+0x724/0x1700 ext4_do_writepages+0x12d6/0x2a70
[...]
==================================================================
The flow of issue triggering is as follows:
ext4_split_extent_at
path = *ppath ext4_ext_insert_extent(ppath)
ext4_ext_create_new_leaf(ppath)
ext4_find_extent(orig_path)
path = *orig_path read_extent_tree_block
// return -ENOMEM or -EIO
ext4_free_ext_path(path)
kfree(path)
*orig_path = NULL
a. If err is -ENOMEM:
ext4_ext_dirty(path + path->p_depth)
// path use-after-free !!!
b. If err is -EIO and we have EXT_DEBUG defined:
ext4_ext_show_leaf(path)
eh = path[depth].p_hdr
// path also use-after-free !!!
So when trying to zeroout or fix the extent length, call ext4_find_extent() to update the path.
In addition we use *ppath directly as an ext4_ext_show_leaf() input to avoid possible use-after-free when EXT_DEBUG is defined, and to avoid unnecessary path updates.
fs/ext4/extents.c +20 -1
@@ -3251,6 +3251,25 @@ static int ext4_split_extent_at(handle_t *handle,if (err != -ENOSPC && err != -EDQUOT && err != -ENOMEM ) goto out; +/* +* Update path is required because previous ext4_ext_insert_extent() +* may have freed or reallocated the path. Using EXT4_EX_NOFAIL +* guarantees that ext4_find_extent() will not return -ENOMEM, +* otherwise -ENOMEM will cause a retry in do_writepages(), and a +* WARN_ON may be triggered in ext4_da_update_reserve_space() due to +* an incorrect ee_len causing the i_reserved_data_blocks exception. +*/ + path =ext4_find_extent (inode, ee_block, ppath, + flags |EXT4_EX_NOFAIL ); +if (IS_ERR (path)) { +EXT4_ERROR_INODE (inode,"Failed split extent on %u, err %ld" , + split,PTR_ERR (path)); +return PTR_ERR (path); + } + depth =ext_depth (inode); + ex = path[depth].p_ext; + *ppath = path; +if (EXT4_EXT_MAY_ZEROOUT & split_flag) {if (split_flag & (EXT4_EXT_DATA_VALID1 |EXT4_EXT_DATA_VALID2 )) {if (split_flag &EXT4_EXT_DATA_VALID1 ) {
@@ -3303,7 +3322,7 @@ static int ext4_split_extent_at(handle_t *handle,ext4_ext_dirty (handle, inode, path + path->p_depth);return err; out: -ext4_ext_show_leaf (inode, path); +ext4_ext_show_leaf (inode, *ppath);return err; }
Hi greg,
Quoted 14 lines · On 2024/10/7 20:28, [email protected] wrote:
The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-4.19.y
git checkoutFETCH_HEAD
git cherry-pick -xc26ab35702f8cd0cdc78f96aa5856bfb77be798f
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2024100717-untrue-mockup-9fb4@gregkh' --subject-prefix 'PATCH 4.19.y' HEAD^..
Quoted 8 lines
Possible dependencies:
c26ab35702f8("ext4: fix slab-use-after-free inext4_split_extent_at()")082cd4ec240b("ext4: fix bug on inext4_es_cache_extentasext4_split_extent_atfailed")thanks,
greg k-h
The dependency for this patch is:3f5424790d43 ("ext4: fix inode tree inconsistency caused by ENOMEM")
After applying this commit, there is no conflict when applying the following two commits on the linux-4.19.y and linux-5.4.y branches:
c26ab35702f8 ("ext4: fix slab-use-after-free in ext4_split_extent_at()") 5b4b2dcace35 ("ext4: update orig_path in ext4_find_extent()")
Regards,
Baokun
Quoted 125 lines
------------------ original commit in Linus's tree ------------------
From
c26ab35702f8cd0cdc78f96aa5856bfb77be798fMon Sep 17 00:00:00 2001
From: Baokun Li <[email protected]>
Date: Thu, 22 Aug 2024 10:35:23 +0800
Subject: [PATCH] ext4: fix slab-use-after-free inext4_split_extent_at()We hit the following use-after-free:
==================================================================
BUG: KASAN: slab-use-after-free in ext4_split_extent_at+0xba8/0xcc0 Read of size 2 at addr ffff88810548ed08 by task kworker/u20:0/40 CPU: 0 PID: 40 Comm: kworker/u20:0 Not tainted 6.9.0-dirty #724 Call Trace: <TASK> kasan_report+0x93/0xc0 ext4_split_extent_at+0xba8/0xcc0 ext4_split_extent.isra.0+0x18f/0x500 ext4_split_convert_extents+0x275/0x750 ext4_ext_handle_unwritten_extents+0x73e/0x1580 ext4_ext_map_blocks+0xe20/0x2dc0 ext4_map_blocks+0x724/0x1700 ext4_do_writepages+0x12d6/0x2a70[...]
Allocated by task 40: __kmalloc_noprof+0x1ac/0x480 ext4_find_extent+0xf3b/0x1e70 ext4_ext_map_blocks+0x188/0x2dc0 ext4_map_blocks+0x724/0x1700 ext4_do_writepages+0x12d6/0x2a70[...]
Freed by task 40: kfree+0xf1/0x2b0 ext4_find_extent+0xa71/0x1e70 ext4_ext_insert_extent+0xa22/0x3260 ext4_split_extent_at+0x3ef/0xcc0 ext4_split_extent.isra.0+0x18f/0x500 ext4_split_convert_extents+0x275/0x750 ext4_ext_handle_unwritten_extents+0x73e/0x1580 ext4_ext_map_blocks+0xe20/0x2dc0 ext4_map_blocks+0x724/0x1700 ext4_do_writepages+0x12d6/0x2a70[...]
==================================================================The flow of issue triggering is as follows:
ext4_split_extent_at
path = *ppath
ext4_ext_insert_extent(ppath)
ext4_ext_create_new_leaf(ppath)
ext4_find_extent(orig_path)
path =*orig_path
read_extent_tree_block
// return-ENOMEMor-EIO
ext4_free_ext_path(path)
kfree(path)
*orig_path= NULL
a. If err is-ENOMEM:ext4_ext_dirty(path + path->p_depth) // path use-after-free !!! b. If err is -EIO and we have EXT_DEBUG defined: ext4_ext_show_leaf(path) eh = path[depth].p_hdr // path also use-after-free !!!So when trying to zeroout or fix the extent length, call
ext4_find_extent()to update the path.In addition we use *ppath directly as an
ext4_ext_show_leaf()input to avoid possible use-after-free whenEXT_DEBUGis defined, and to avoid unnecessary path updates.Fixes dfe5080939ea ("ext4: drop EXT4_EX_NOFREE_ON_ERR from rest of extents handling code")Signed-off-by Baokun Li <[email protected]>Reviewed-by Jan Kara <[email protected]>Reviewed-by Ojaswin Mujoo <[email protected]>Tested-by Ojaswin Mujoo <[email protected]>Signed-off-by Theodore Ts'o <[email protected]>fs/ext4/extents.c +20
-1@@ -3251,6 +3251,25 @@ static int ext4_split_extent_at(handle_t *handle,if (err != -ENOSPC && err != -EDQUOT && err != -ENOMEM ) goto out; +/* +* Update path is required because previous ext4_ext_insert_extent() +* may have freed or reallocated the path. Using EXT4_EX_NOFAIL +* guarantees that ext4_find_extent() will not return -ENOMEM, +* otherwise -ENOMEM will cause a retry in do_writepages(), and a +* WARN_ON may be triggered in ext4_da_update_reserve_space() due to +* an incorrect ee_len causing the i_reserved_data_blocks exception. +*/ + path =ext4_find_extent (inode, ee_block, ppath, + flags |EXT4_EX_NOFAIL ); +if (IS_ERR (path)) { +EXT4_ERROR_INODE (inode,"Failed split extent on %u, err %ld" , + split,PTR_ERR (path)); +return PTR_ERR (path); + } + depth =ext_depth (inode); + ex = path[depth].p_ext; + *ppath = path; +if (EXT4_EXT_MAY_ZEROOUT & split_flag) {if (split_flag & (EXT4_EXT_DATA_VALID1 |EXT4_EXT_DATA_VALID2 )) {if (split_flag &EXT4_EXT_DATA_VALID1 ) {@@ -3303,7 +3322,7 @@ static int ext4_split_extent_at(handle_t *handle,ext4_ext_dirty (handle, inode, path + path->p_depth);return err; out: -ext4_ext_show_leaf (inode, path); +ext4_ext_show_leaf (inode, *ppath);return err; }