[PATH 6.6 0/3] fix potential ext4 null pointer

[PATH 6.6 0/3] fix potential ext4 null pointer

[Yang Erkun]

Our Hulk robot discovered that there were missing 6b854d552711
("ext4: get rid of ppath in get_ext_path()") when backporting the
linux-6.6.y mainline patch set[1], which could potentially trigger
some error branches in ext4 to cause a panic. I also verified this
point during testing using xfstests-bld[2]. Along with this, we
have also backported other related patches from the corresponding
patch set [1].

[1]. https://lore.kernel.org/all/20240822023545.1994557-1-libaokun@huaweicloud.com/
[2]. https://github.com/tytso/xfstests-bld

generic/051 81s ...  [09:20:24][  509.012535] run fstests generic/051 at 2026-04-21 09:20:24
[  509.314026] EXT4-fs (vdb): mounted filesystem d225342d-c437-4a7d-893b-5d02903a5ea4 r/w with ordered data mode. Quota mode: none.
[  509.397019] EXT4-fs (vdc): mounted filesystem 72b9e6ee-4b56-45ee-b71e-ca491d2fd7e9 r/w with ordered data mode. Quota mode: none.
[  509.399614] EXT4-fs (vdc): shut down requested (1)
[  509.400082] Aborting journal on device vdc-8.
[  509.402378] EXT4-fs (vdc): unmounting filesystem 72b9e6ee-4b56-45ee-b71e-ca491d2fd7e9.
[  509.443140] EXT4-fs (vdc): mounted filesystem 03f15f3c-5938-41ea-bbf8-321de40d01ff r/w with ordered data mode. Quota mode: none.
[  539.831842] EXT4-fs (vdc): unmounting filesystem 03f15f3c-5938-41ea-bbf8-321de40d01ff.
[  539.868710] EXT4-fs (vdc): mounted filesystem 03f15f3c-5938-41ea-bbf8-321de40d01ff r/w with ordered data mode. Quota mode: none.
[  552.967466] BUG: unable to handle page fault for address: ffffffffffffffec
[  552.968455] #PF: supervisor read access in kernel mode
[  552.969157] #PF: error_code(0x0000) - not-present page
[  552.969859] PGD 282c067 P4D 282d067 PUD 282f067 PMD 0
[  552.970575] Oops: 0000 [#1] PREEMPT SMP NOPTI
[  552.971179] CPU: 0 PID: 292843 Comm: fsstress Not tainted 6.6.135-xfstests #2
[  552.972143] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.1-2.fc37 04/01/2014
[  552.973284] RIP: 0010:ext4_ext_map_blocks+0x191/0xab0
[  552.973986] Code: 4c 89 e6 48 89 ef 48 8d 54 24 60 e8 89 6c ff ff 85 c0 89 44 24 28 0f 84 59 02 00 00 48 8b 44 24 30 48 85 c0 0f 84 09 06 00 00 <44> 0f b7 78 08 45 31 f6 48 89 1c 24 49 89 c4 44 89 f3 49 89 c6 49
[  552.976362] RSP: 0018:ffa0000006ab3c78 EFLAGS: 00010286
[  552.976862] RAX: ffffffffffffffe4 RBX: ffa0000006ab3de0 RCX: 0000000000000000
[  552.977525] RDX: ffffffff82244590 RSI: ffffffff825d3cfc RDI: ff1100002e5d5068
[  552.978189] RBP: ff110000064f6628 R08: ffffffff825d3ddd R09: ff1100006b74a618
[  552.978850] R10: 00000000d8f693c7 R11: ff11000077f49ff0 R12: ff110000062040c0
[  552.979511] R13: 0000000000000043 R14: 0000000000025b80 R15: ff110000069f1000
[  552.980159] FS:  00007f7685ec8740(0000) GS:ff1100007dc00000(0000) knlGS:0000000000000000
[  552.980906] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  552.981445] CR2: ffffffffffffffec CR3: 0000000006a44005 CR4: 0000000000771ef0
[  552.982091] PKRU: 55555554
[  552.982351] Call Trace:
[  552.982606]  <TASK>
[  552.982818]  ext4_map_blocks+0x23e/0x6b0
[  552.983191]  ext4_alloc_file_blocks.isra.0+0x12b/0x370
[  552.983671]  ext4_fallocate+0x150/0x310
[  552.984034]  vfs_fallocate+0x13e/0x380
[  552.984391]  ioctl_preallocate+0xa4/0xd0
[  552.984769]  __x64_sys_ioctl+0x71/0xd0
[  552.985126]  do_syscall_64+0x38/0x80
[  552.985479]  entry_SYSCALL_64_after_hwframe+0x78/0xe2
[  552.985956] RIP: 0033:0x7f7685fc8c5b
[  552.986291] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28 00 00
[  552.987925] RSP: 002b:00007fff16838290 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[  552.988571] RAX: ffffffffffffffda RBX: 000000000000991b RCX: 00007f7685fc8c5b
[  552.989186] RDX: 00007fff16838310 RSI: 000000004030582a RDI: 0000000000000003
[  552.989812] RBP: 0000000000000003 R08: 0000000000000002 R09: 00007fff168382fc
[  552.990428] R10: 0000000000001000 R11: 0000000000000246 R12: 0000000000000000
[  552.991043] R13: 00000000001a6829 R14: 8f5c28f5c28f5c29 R15: 000055c9bd970650
[  552.991655]  </TASK>
[  552.991857] CR2: ffffffffffffffec
[  552.992154] ---[ end trace 0000000000000000 ]---
[  552.992557] RIP: 0010:ext4_ext_map_blocks+0x191/0xab0

Baokun Li (3):
  ext4: get rid of ppath in get_ext_path()
  ext4: get rid of ppath in ext4_force_split_extent_at()
  ext4: get rid of ppath in convert_initialized_extent()

 fs/ext4/extents.c     | 111 +++++++++++++++++++++++-------------------
 fs/ext4/move_extent.c |  34 ++++++-------
 2 files changed, 77 insertions(+), 68 deletions(-)

-- 
2.39.2

[PATH 6.6 1/3] ext4: get rid of ppath in get_ext_path()

[Yang Erkun]

From: Baokun Li <libaokun1@huawei.com>

[ Upstream commit 6b854d552711aa33f59eda334e6d94a00d8825bb ]

The use of path and ppath is now very confusing, so to make the code more
readable, pass path between functions uniformly, and get rid of ppath.

After getting rid of ppath in get_ext_path(), its caller may pass an error
pointer to ext4_free_ext_path(), so it needs to teach ext4_free_ext_path()
and ext4_ext_drop_refs() to skip the error pointer. No functional changes.

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-13-libaokun@huaweicloud.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
[we need IS_ERR_OR_NULL to prevent oops]
Reported-by: Hulk Robot <hulkrobot@huawei.com>
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
 fs/ext4/extents.c     |  5 +++--
 fs/ext4/move_extent.c | 34 +++++++++++++++++-----------------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 7626cf2b07f1..300bf2289bc1 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -114,7 +114,7 @@ static void ext4_ext_drop_refs(struct ext4_ext_path *path)
 {
 	int depth, i;
 
-	if (!path)
+	if (IS_ERR_OR_NULL(path))
 		return;
 	depth = path->p_depth;
 	for (i = 0; i <= depth; i++, path++) {
@@ -125,6 +125,8 @@ static void ext4_ext_drop_refs(struct ext4_ext_path *path)
 
 void ext4_free_ext_path(struct ext4_ext_path *path)
 {
+	if (IS_ERR_OR_NULL(path))
+		return;
 	ext4_ext_drop_refs(path);
 	kfree(path);
 }
@@ -4227,7 +4229,6 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 	path = ext4_find_extent(inode, map->m_lblk, NULL, 0);
 	if (IS_ERR(path)) {
 		err = PTR_ERR(path);
-		path = NULL;
 		goto out;
 	}
 
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index d5636a2a718a..96a84de32169 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -17,27 +17,23 @@
  * get_ext_path() - Find an extent path for designated logical block number.
  * @inode:	inode to be searched
  * @lblock:	logical block number to find an extent path
- * @ppath:	pointer to an extent path pointer (for output)
+ * @path:	pointer to an extent path
  *
- * ext4_find_extent wrapper. Return 0 on success, or a negative error value
- * on failure.
+ * ext4_find_extent wrapper. Return an extent path pointer on success,
+ * or an error pointer on failure.
  */
-static inline int
+static inline struct ext4_ext_path *
 get_ext_path(struct inode *inode, ext4_lblk_t lblock,
-		struct ext4_ext_path **ppath)
+	     struct ext4_ext_path *path)
 {
-	struct ext4_ext_path *path = *ppath;
-
-	*ppath = NULL;
 	path = ext4_find_extent(inode, lblock, path, EXT4_EX_NOCACHE);
 	if (IS_ERR(path))
-		return PTR_ERR(path);
+		return path;
 	if (path[ext_depth(inode)].p_ext == NULL) {
 		ext4_free_ext_path(path);
-		return -ENODATA;
+		return ERR_PTR(-ENODATA);
 	}
-	*ppath = path;
-	return 0;
+	return path;
 }
 
 /**
@@ -95,9 +91,11 @@ mext_check_coverage(struct inode *inode, ext4_lblk_t from, ext4_lblk_t count,
 	int ret = 0;
 	ext4_lblk_t last = from + count;
 	while (from < last) {
-		*err = get_ext_path(inode, from, &path);
-		if (*err)
-			goto out;
+		path = get_ext_path(inode, from, path);
+		if (IS_ERR(path)) {
+			*err = PTR_ERR(path);
+			return ret;
+		}
 		ext = path[ext_depth(inode)].p_ext;
 		if (unwritten != ext4_ext_is_unwritten(ext))
 			goto out;
@@ -634,9 +632,11 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk,
 		int offset_in_page;
 		int unwritten, cur_len;
 
-		ret = get_ext_path(orig_inode, o_start, &path);
-		if (ret)
+		path = get_ext_path(orig_inode, o_start, path);
+		if (IS_ERR(path)) {
+			ret = PTR_ERR(path);
 			goto out;
+		}
 		ex = path[path->p_depth].p_ext;
 		cur_blk = le32_to_cpu(ex->ee_block);
 		cur_len = ext4_ext_get_actual_len(ex);
-- 
2.39.2

[PATH 6.6 2/3] ext4: get rid of ppath in ext4_force_split_extent_at()

[Yang Erkun]

From: Baokun Li <libaokun1@huawei.com>

[ Upstream commit f07be1c367369636d7d338d7994473d6eae283c5 ]

The use of path and ppath is now very confusing, so to make the code more
readable, pass path between functions uniformly, and get rid of ppath.

To get rid of the ppath in ext4_force_split_extent_at(), the following is
done here:

 * Free the extents path when an error is encountered.

No functional changes.

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-17-libaokun@huaweicloud.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
 fs/ext4/extents.c | 69 ++++++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 31 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 300bf2289bc1..0406dac7fbf1 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -329,27 +329,20 @@ static inline int ext4_ext_space_root_idx(struct inode *inode, int check)
 	return size;
 }
 
-static inline int
+static inline struct ext4_ext_path *
 ext4_force_split_extent_at(handle_t *handle, struct inode *inode,
-			   struct ext4_ext_path **ppath, ext4_lblk_t lblk,
+			   struct ext4_ext_path *path, ext4_lblk_t lblk,
 			   int nofail)
 {
-	struct ext4_ext_path *path = *ppath;
 	int unwritten = ext4_ext_is_unwritten(path[path->p_depth].p_ext);
 	int flags = EXT4_EX_NOCACHE | EXT4_GET_BLOCKS_PRE_IO;
 
 	if (nofail)
 		flags |= EXT4_GET_BLOCKS_METADATA_NOFAIL | EXT4_EX_NOFAIL;
 
-	path = ext4_split_extent_at(handle, inode, path, lblk, unwritten ?
+	return ext4_split_extent_at(handle, inode, path, lblk, unwritten ?
 			EXT4_EXT_MARK_UNWRIT1|EXT4_EXT_MARK_UNWRIT2 : 0,
 			flags);
-	if (IS_ERR(path)) {
-		*ppath = NULL;
-		return PTR_ERR(path);
-	}
-	*ppath = path;
-	return 0;
 }
 
 static int
@@ -2890,11 +2883,12 @@ int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
 			 * fail removing space due to ENOSPC so try to use
 			 * reserved block if that happens.
 			 */
-			err = ext4_force_split_extent_at(handle, inode, &path,
-							 end + 1, 1);
-			if (err < 0)
+			path = ext4_force_split_extent_at(handle, inode, path,
+							  end + 1, 1);
+			if (IS_ERR(path)) {
+				err = PTR_ERR(path);
 				goto out;
-
+			}
 		} else if (sbi->s_cluster_ratio > 1 && end >= ex_end &&
 			   partial.state == initial) {
 			/*
@@ -5772,17 +5766,21 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
 		/* Prepare left boundary */
 		if (e1_blk < lblk1) {
 			split = 1;
-			*erp = ext4_force_split_extent_at(handle, inode1,
-						&path1, lblk1, 0);
-			if (unlikely(*erp))
+			path1 = ext4_force_split_extent_at(handle, inode1,
+							   path1, lblk1, 0);
+			if (IS_ERR(path1)) {
+				*erp = PTR_ERR(path1);
 				goto finish;
+			}
 		}
 		if (e2_blk < lblk2) {
 			split = 1;
-			*erp = ext4_force_split_extent_at(handle, inode2,
-						&path2,  lblk2, 0);
-			if (unlikely(*erp))
+			path2 = ext4_force_split_extent_at(handle, inode2,
+							   path2, lblk2, 0);
+			if (IS_ERR(path2)) {
+				*erp = PTR_ERR(path2);
 				goto finish;
+			}
 		}
 		/* ext4_split_extent_at() may result in leaf extent split,
 		 * path must to be revalidated. */
@@ -5798,17 +5796,21 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
 
 		if (len != e1_len) {
 			split = 1;
-			*erp = ext4_force_split_extent_at(handle, inode1,
-						&path1, lblk1 + len, 0);
-			if (unlikely(*erp))
+			path1 = ext4_force_split_extent_at(handle, inode1,
+							path1, lblk1 + len, 0);
+			if (IS_ERR(path1)) {
+				*erp = PTR_ERR(path1);
 				goto finish;
+			}
 		}
 		if (len != e2_len) {
 			split = 1;
-			*erp = ext4_force_split_extent_at(handle, inode2,
-						&path2, lblk2 + len, 0);
-			if (*erp)
+			path2 = ext4_force_split_extent_at(handle, inode2,
+							path2, lblk2 + len, 0);
+			if (IS_ERR(path2)) {
+				*erp = PTR_ERR(path2);
 				goto finish;
+			}
 		}
 		/* ext4_split_extent_at() may result in leaf extent split,
 		 * path must to be revalidated. */
@@ -5974,24 +5976,29 @@ int ext4_ext_replay_update_ex(struct inode *inode, ext4_lblk_t start,
 		ext4_ext_get_actual_len(ex) != len) {
 		/* We need to split this extent to match our extent first */
 		down_write(&EXT4_I(inode)->i_data_sem);
-		ret = ext4_force_split_extent_at(NULL, inode, &path, start, 1);
+		path = ext4_force_split_extent_at(NULL, inode, path, start, 1);
 		up_write(&EXT4_I(inode)->i_data_sem);
-		if (ret)
+		if (IS_ERR(path)) {
+			ret = PTR_ERR(path);
 			goto out;
+		}
 
 		path = ext4_find_extent(inode, start, path, 0);
 		if (IS_ERR(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, &path,
-							 start + len, 1);
+			path = ext4_force_split_extent_at(NULL, inode, path,
+							  start + len, 1);
 			up_write(&EXT4_I(inode)->i_data_sem);
-			if (ret)
+			if (IS_ERR(path)) {
+				ret = PTR_ERR(path);
 				goto out;
+			}
 
 			path = ext4_find_extent(inode, start, path, 0);
 			if (IS_ERR(path))
-- 
2.39.2

[PATH 6.6 3/3] ext4: get rid of ppath in convert_initialized_extent()

[Yang Erkun]

From: Baokun Li <libaokun1@huawei.com>

[ Upstream commit 4191eefef978d734fa8249bede3f9b02a85aa3c0 ]

The use of path and ppath is now very confusing, so to make the code more
readable, pass path between functions uniformly, and get rid of ppath.

To get rid of the ppath in convert_initialized_extent(), the following is
done here:

 * Free the extents path when an error is encountered.

No functional changes.

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-23-libaokun@huaweicloud.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
 fs/ext4/extents.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 0406dac7fbf1..1c55f498ce4f 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3830,13 +3830,12 @@ ext4_convert_unwritten_extents_endio(handle_t *handle, struct inode *inode,
 	return ERR_PTR(err);
 }
 
-static int
+static struct ext4_ext_path *
 convert_initialized_extent(handle_t *handle, struct inode *inode,
 			   struct ext4_map_blocks *map,
-			   struct ext4_ext_path **ppath,
+			   struct ext4_ext_path *path,
 			   unsigned int *allocated)
 {
-	struct ext4_ext_path *path = *ppath;
 	struct ext4_extent *ex;
 	ext4_lblk_t ee_block;
 	unsigned int ee_len;
@@ -3861,29 +3860,25 @@ convert_initialized_extent(handle_t *handle, struct inode *inode,
 	if (ee_block != map->m_lblk || ee_len > map->m_len) {
 		path = ext4_split_convert_extents(handle, inode, map, path,
 				EXT4_GET_BLOCKS_CONVERT_UNWRITTEN, NULL);
-		if (IS_ERR(path)) {
-			*ppath = NULL;
-			return PTR_ERR(path);
-		}
+		if (IS_ERR(path))
+			return path;
 
 		path = ext4_find_extent(inode, map->m_lblk, path, 0);
-		if (IS_ERR(path)) {
-			*ppath = NULL;
-			return PTR_ERR(path);
-		}
-		*ppath = path;
+		if (IS_ERR(path))
+			return path;
 		depth = ext_depth(inode);
 		ex = path[depth].p_ext;
 		if (!ex) {
 			EXT4_ERROR_INODE(inode, "unexpected hole at %lu",
 					 (unsigned long) map->m_lblk);
-			return -EFSCORRUPTED;
+			err = -EFSCORRUPTED;
+			goto errout;
 		}
 	}
 
 	err = ext4_ext_get_access(handle, inode, path + depth);
 	if (err)
-		return err;
+		goto errout;
 	/* first mark the extent as unwritten */
 	ext4_ext_mark_unwritten(ex);
 
@@ -3895,7 +3890,7 @@ convert_initialized_extent(handle_t *handle, struct inode *inode,
 	/* Mark modified extent as dirty */
 	err = ext4_ext_dirty(handle, inode, path + path->p_depth);
 	if (err)
-		return err;
+		goto errout;
 	ext4_ext_show_leaf(inode, path);
 
 	ext4_update_inode_fsync_trans(handle, inode, 1);
@@ -3904,7 +3899,11 @@ convert_initialized_extent(handle_t *handle, struct inode *inode,
 	if (*allocated > map->m_len)
 		*allocated = map->m_len;
 	map->m_len = *allocated;
-	return 0;
+	return path;
+
+errout:
+	ext4_free_ext_path(path);
+	return ERR_PTR(err);
 }
 
 static struct ext4_ext_path *
@@ -4271,8 +4270,10 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 			 */
 			if ((!ext4_ext_is_unwritten(ex)) &&
 			    (flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN)) {
-				err = convert_initialized_extent(handle,
-					inode, map, &path, &allocated);
+				path = convert_initialized_extent(handle,
+					inode, map, path, &allocated);
+				if (IS_ERR(path))
+					err = PTR_ERR(path);
 				goto out;
 			} else if (!ext4_ext_is_unwritten(ex)) {
 				map->m_flags |= EXT4_MAP_MAPPED;
-- 
2.39.2