[PATCH v3 0/5] Fix some issues about ext4-test

[PATCH v3 0/5] Fix some issues about ext4-test

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

Diff v3 vs v2:
1. Remove three patches from this patchset:
  ext4: fix mballoc-test.c is not compiled when EXT4_KUNIT_TESTS=M
  ext4: introduce EXPORT_SYMBOL_FOR_EXT4_TEST() helper
  ext4: fix extents-test.c is not compiled when EXT4_KUNIT_TESTS=M
2. Fix 'sbi' leak as no "sbi->s_sb->s_op->put_super()" registered.

Diff v2 vs v1:
1. Fix compile warning when disable EXT4_KUNIT_TESTS for patch[1][3];
2. Remove reviewed-by tag for patch[1];

Patch [1]-[2]:
Decoupled mballoc-test and extents-test from ext4. Patch [1] does not
have any changes compared to the previously released version, so the
reviewed-by is added.
Patch [3-7]:
Bugfix for extents-test.c.
Patch [8]:
Bugfix for mballoc-test.c.

Ye Bin (5):
  ext4: call deactivate_super() in extents_kunit_exit()
  ext4: fix miss unlock 'sb->s_umount' in extents_kunit_init()
  ext4: fix the error handling process in extents_kunit_init).
  ext4: fix possible null-ptr-deref in extents_kunit_exit()
  ext4: fix possible null-ptr-deref in mbt_kunit_exit()

 fs/ext4/extents-test.c | 54 +++++++++++++++++++++++++++++++-----------
 fs/ext4/mballoc-test.c |  6 ++++-
 2 files changed, 45 insertions(+), 15 deletions(-)

-- 
2.34.1

[PATCH v3 1/5] ext4: call deactivate_super() in extents_kunit_exit()

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

Call deactivate_super() is called in extents_kunit_exit() to cleanup
the file system resource.

Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/extents-test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
index 5496b2c8e2cd..e3d23e3cda87 100644
--- a/fs/ext4/extents-test.c
+++ b/fs/ext4/extents-test.c
@@ -146,6 +146,7 @@ static void extents_kunit_exit(struct kunit *test)
 	struct ext4_sb_info *sbi = sb->s_fs_info;
 
 	ext4_es_unregister_shrinker(sbi);
+	deactivate_super(sbi->s_sb);
 	kfree(sbi);
 	kfree(k_ctx.k_ei);
 	kfree(k_ctx.k_data);
-- 
2.34.1

[PATCH v3 2/5] ext4: fix miss unlock 'sb->s_umount' in extents_kunit_init()

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

There's warning as follows when do ext4 kunit test:
WARNING: kunit_try_catch/15923 still has locks held!
7.0.0-rc3-next-20260309-00028-g73f965a1bbb1-dirty #281 Tainted: G            E    N
1 lock held by kunit_try_catch/15923:
 #0: ffff888139f860e0 (&type->s_umount_key#70/1){+.+.}-{4:4}, at: alloc_super.constprop.0+0x172/0xa90
Call Trace:
 <TASK>
 dump_stack_lvl+0x180/0x1b0
 debug_check_no_locks_held+0xc8/0xd0
 do_exit+0x1502/0x2b20
 kthread+0x3a9/0x540
 ret_from_fork+0xa76/0xdf0
 ret_from_fork_asm+0x1a/0x30

As sget() will return 'sb' which holds 's->s_umount' lock. However,
"extents-test" miss unlock this lock.
So unlock 's->s_umount' in the end of extents_kunit_init().

Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/extents-test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
index e3d23e3cda87..3d4663d99eb1 100644
--- a/fs/ext4/extents-test.c
+++ b/fs/ext4/extents-test.c
@@ -310,6 +310,8 @@ static int extents_kunit_init(struct kunit *test)
 	kunit_activate_static_stub(test, ext4_ext_zeroout, ext4_ext_zeroout_stub);
 	kunit_activate_static_stub(test, ext4_issue_zeroout,
 				   ext4_issue_zeroout_stub);
+	up_write(&sb->s_umount);
+
 	return 0;
 }
 
-- 
2.34.1

[PATCH v3 3/5] ext4: fix the error handling process in extents_kunit_init).

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

The error processing in extents_kunit_init() is improper, causing
resource leakage.
Reconstruct the error handling process to prevent potential resource
leaks

Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/extents-test.c | 44 ++++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
index 3d4663d99eb1..543236a31e13 100644
--- a/fs/ext4/extents-test.c
+++ b/fs/ext4/extents-test.c
@@ -225,33 +225,37 @@ static int extents_kunit_init(struct kunit *test)
 		(struct kunit_ext_test_param *)(test->param_value);
 	int err;
 
-	sb = sget(&ext_fs_type, NULL, ext_set, 0, NULL);
-	if (IS_ERR(sb))
-		return PTR_ERR(sb);
-
-	sb->s_blocksize = 4096;
-	sb->s_blocksize_bits = 12;
-
 	sbi = kzalloc_obj(struct ext4_sb_info);
 	if (sbi == NULL)
 		return -ENOMEM;
 
+	sb = sget(&ext_fs_type, NULL, ext_set, 0, NULL);
+	if (IS_ERR(sb)) {
+		kfree(sbi);
+		return PTR_ERR(sb);
+	}
+
 	sbi->s_sb = sb;
 	sb->s_fs_info = sbi;
 
+	sb->s_blocksize = 4096;
+	sb->s_blocksize_bits = 12;
+
 	if (!param || !param->disable_zeroout)
 		sbi->s_extent_max_zeroout_kb = 32;
 
 	/* setup the mock inode */
 	k_ctx.k_ei = kzalloc_obj(struct ext4_inode_info);
-	if (k_ctx.k_ei == NULL)
-		return -ENOMEM;
+	if (k_ctx.k_ei == NULL) {
+		err = -ENOMEM;
+		goto out_deactivate;
+	}
 	ei = k_ctx.k_ei;
 	inode = &ei->vfs_inode;
 
 	err = ext4_es_register_shrinker(sbi);
 	if (err)
-		return err;
+		goto out_deactivate;
 
 	ext4_es_init_tree(&ei->i_es_tree);
 	rwlock_init(&ei->i_es_lock);
@@ -267,8 +271,10 @@ static int extents_kunit_init(struct kunit *test)
 	inode->i_sb = sb;
 
 	k_ctx.k_data = kzalloc(EXT_DATA_LEN * 4096, GFP_KERNEL);
-	if (k_ctx.k_data == NULL)
-		return -ENOMEM;
+	if (k_ctx.k_data == NULL) {
+		err = -ENOMEM;
+		goto out_deactivate;
+	}
 
 	/*
 	 * set the data area to a junk value
@@ -313,6 +319,20 @@ static int extents_kunit_init(struct kunit *test)
 	up_write(&sb->s_umount);
 
 	return 0;
+
+out_deactivate:
+	kfree(k_ctx.k_ei);
+	k_ctx.k_ei = NULL;
+
+	kfree(k_ctx.k_data);
+	k_ctx.k_data = NULL;
+
+	if (sbi->s_es_shrinker)
+		ext4_es_unregister_shrinker(sbi);
+	deactivate_locked_super(sb);
+	kfree(sbi);
+
+	return err;
 }
 
 /*
-- 
2.34.1

[PATCH v3 4/5] ext4: fix possible null-ptr-deref in extents_kunit_exit()

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

There's issue as follows:
KASAN: null-ptr-deref in range [0x00000000000002c0-0x00000000000002c7]
Tainted: [E]=UNSIGNED_MODULE, [N]=TEST
RIP: 0010:extents_kunit_exit+0x2e/0xc0 [ext4_test]
Call Trace:
 <TASK>
 kunit_try_run_case_cleanup+0xbc/0x100 [kunit]
 kunit_generic_run_threadfn_adapter+0x89/0x100 [kunit]
 kthread+0x408/0x540
 ret_from_fork+0xa76/0xdf0
 ret_from_fork_asm+0x1a/0x30

Above issue happens as extents_kunit_init() init testcase failed.
So test if testcase is inited success.

Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/extents-test.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
index 543236a31e13..6ae72986ca9c 100644
--- a/fs/ext4/extents-test.c
+++ b/fs/ext4/extents-test.c
@@ -142,9 +142,12 @@ static struct file_system_type ext_fs_type = {
 
 static void extents_kunit_exit(struct kunit *test)
 {
-	struct super_block *sb = k_ctx.k_ei->vfs_inode.i_sb;
-	struct ext4_sb_info *sbi = sb->s_fs_info;
+	struct ext4_sb_info *sbi;
 
+	if (!k_ctx.k_ei)
+		return;
+
+	sbi = k_ctx.k_ei->vfs_inode.i_sb->s_fs_info;
 	ext4_es_unregister_shrinker(sbi);
 	deactivate_super(sbi->s_sb);
 	kfree(sbi);
-- 
2.34.1

[PATCH v3 5/5] ext4: fix possible null-ptr-deref in mbt_kunit_exit()

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

There's issue as follows:
    # test_new_blocks_simple: failed to initialize: -12
KASAN: null-ptr-deref in range [0x0000000000000638-0x000000000000063f]
Tainted: [E]=UNSIGNED_MODULE, [N]=TEST
RIP: 0010:mbt_kunit_exit+0x5e/0x3e0 [ext4_test]
Call Trace:
 <TASK>
 kunit_try_run_case_cleanup+0xbc/0x100 [kunit]
 kunit_generic_run_threadfn_adapter+0x89/0x100 [kunit]
 kthread+0x408/0x540
 ret_from_fork+0xa76/0xdf0
 ret_from_fork_asm+0x1a/0x30

If mbt_kunit_init() init testcase failed will lead to null-ptr-deref.
So add test if 'sb' is inited success in mbt_kunit_exit().

Fixes: 7c9fa399a369 ("ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc")
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/mballoc-test.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/mballoc-test.c b/fs/ext4/mballoc-test.c
index c75b91ae0cf0..90ed505fa4b1 100644
--- a/fs/ext4/mballoc-test.c
+++ b/fs/ext4/mballoc-test.c
@@ -362,7 +362,6 @@ static int mbt_kunit_init(struct kunit *test)
 		return ret;
 	}
 
-	test->priv = sb;
 	kunit_activate_static_stub(test,
 				   ext4_read_block_bitmap_nowait,
 				   ext4_read_block_bitmap_nowait_stub);
@@ -383,6 +382,8 @@ static int mbt_kunit_init(struct kunit *test)
 		return -ENOMEM;
 	}
 
+	test->priv = sb;
+
 	return 0;
 }
 
@@ -390,6 +391,9 @@ static void mbt_kunit_exit(struct kunit *test)
 {
 	struct super_block *sb = (struct super_block *)test->priv;
 
+	if (!sb)
+		return;
+
 	mbt_mb_release(sb);
 	mbt_ctx_release(sb);
 	mbt_ext4_free_super_block(sb);
-- 
2.34.1

Re: [PATCH v3 0/5] Fix some issues about ext4-test

[Ritesh Harjani]

Ye Bin <yebin@huaweicloud.com> writes:

> From: Ye Bin <yebin10@huawei.com>
>
> Diff v3 vs v2:
> 1. Remove three patches from this patchset:
>   ext4: fix mballoc-test.c is not compiled when EXT4_KUNIT_TESTS=M
>   ext4: introduce EXPORT_SYMBOL_FOR_EXT4_TEST() helper
>   ext4: fix extents-test.c is not compiled when EXT4_KUNIT_TESTS=M
> 2. Fix 'sbi' leak as no "sbi->s_sb->s_op->put_super()" registered.
>

...and that this patch series is based on top of [1]
[1]: https://lore.kernel.org/linux-ext4/5bb9041471dab8ce870c191c19cbe4df57473be8.1772381213.git.ritesh.list@gmail.com/

I think you also missed to take few RBs from previous version. But never
mind, thanks for sending the fixes :)

-ritesh

> Diff v2 vs v1:
> 1. Fix compile warning when disable EXT4_KUNIT_TESTS for patch[1][3];
> 2. Remove reviewed-by tag for patch[1];
>
> Patch [1]-[2]:
> Decoupled mballoc-test and extents-test from ext4. Patch [1] does not
> have any changes compared to the previously released version, so the
> reviewed-by is added.
> Patch [3-7]:
> Bugfix for extents-test.c.
> Patch [8]:
> Bugfix for mballoc-test.c.
>
> Ye Bin (5):
>   ext4: call deactivate_super() in extents_kunit_exit()
>   ext4: fix miss unlock 'sb->s_umount' in extents_kunit_init()
>   ext4: fix the error handling process in extents_kunit_init).
>   ext4: fix possible null-ptr-deref in extents_kunit_exit()
>   ext4: fix possible null-ptr-deref in mbt_kunit_exit()
>
>  fs/ext4/extents-test.c | 54 +++++++++++++++++++++++++++++++-----------
>  fs/ext4/mballoc-test.c |  6 ++++-
>  2 files changed, 45 insertions(+), 15 deletions(-)
>
> -- 
> 2.34.1

Re: [PATCH v3 1/5] ext4: call deactivate_super() in extents_kunit_exit()

[Ritesh Harjani]

Ye Bin <yebin@huaweicloud.com> writes:

> From: Ye Bin <yebin10@huawei.com>
>
> Call deactivate_super() is called in extents_kunit_exit() to cleanup
> the file system resource.

yes, since _init() routine call sget(), we should call
deactivate_super() in _exit() routine.

LGTM.
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>


>
> Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/ext4/extents-test.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index 5496b2c8e2cd..e3d23e3cda87 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c
> @@ -146,6 +146,7 @@ static void extents_kunit_exit(struct kunit *test)
>  	struct ext4_sb_info *sbi = sb->s_fs_info;
>  
>  	ext4_es_unregister_shrinker(sbi);
> +	deactivate_super(sbi->s_sb);
>  	kfree(sbi);
>  	kfree(k_ctx.k_ei);
>  	kfree(k_ctx.k_data);
> -- 
> 2.34.1

Re: [PATCH v3 2/5] ext4: fix miss unlock 'sb->s_umount' in extents_kunit_init()

[Ritesh Harjani]

Ye Bin <yebin@huaweicloud.com> writes:

> From: Ye Bin <yebin10@huawei.com>
>
> There's warning as follows when do ext4 kunit test:
> WARNING: kunit_try_catch/15923 still has locks held!
> 7.0.0-rc3-next-20260309-00028-g73f965a1bbb1-dirty #281 Tainted: G            E    N
> 1 lock held by kunit_try_catch/15923:
>  #0: ffff888139f860e0 (&type->s_umount_key#70/1){+.+.}-{4:4}, at: alloc_super.constprop.0+0x172/0xa90
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x180/0x1b0
>  debug_check_no_locks_held+0xc8/0xd0
>  do_exit+0x1502/0x2b20
>  kthread+0x3a9/0x540
>  ret_from_fork+0xa76/0xdf0
>  ret_from_fork_asm+0x1a/0x30
>
> As sget() will return 'sb' which holds 's->s_umount' lock. However,
> "extents-test" miss unlock this lock.
> So unlock 's->s_umount' in the end of extents_kunit_init().
>

Agreed.

LGTM.
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>


> Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/ext4/extents-test.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index e3d23e3cda87..3d4663d99eb1 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c
> @@ -310,6 +310,8 @@ static int extents_kunit_init(struct kunit *test)
>  	kunit_activate_static_stub(test, ext4_ext_zeroout, ext4_ext_zeroout_stub);
>  	kunit_activate_static_stub(test, ext4_issue_zeroout,
>  				   ext4_issue_zeroout_stub);
> +	up_write(&sb->s_umount);
> +
>  	return 0;
>  }
>  
> -- 
> 2.34.1

Re: [PATCH v3 3/5] ext4: fix the error handling process in extents_kunit_init).

[Ritesh Harjani]

Ye Bin <yebin@huaweicloud.com> writes:

> From: Ye Bin <yebin10@huawei.com>
>
> The error processing in extents_kunit_init() is improper, causing
> resource leakage.
> Reconstruct the error handling process to prevent potential resource
> leaks
>

Minor nit.

> Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/ext4/extents-test.c | 44 ++++++++++++++++++++++++++++++------------
>  1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index 3d4663d99eb1..543236a31e13 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c
> @@ -225,33 +225,37 @@ static int extents_kunit_init(struct kunit *test)
>  		(struct kunit_ext_test_param *)(test->param_value);
>  	int err;
>  
> -	sb = sget(&ext_fs_type, NULL, ext_set, 0, NULL);
> -	if (IS_ERR(sb))
> -		return PTR_ERR(sb);
> -
> -	sb->s_blocksize = 4096;
> -	sb->s_blocksize_bits = 12;
> -
>  	sbi = kzalloc_obj(struct ext4_sb_info);
>  	if (sbi == NULL)
>  		return -ENOMEM;
>  
> +	sb = sget(&ext_fs_type, NULL, ext_set, 0, NULL);
> +	if (IS_ERR(sb)) {
> +		kfree(sbi);
> +		return PTR_ERR(sb);
> +	}
> +
>  	sbi->s_sb = sb;
>  	sb->s_fs_info = sbi;
>  
> +	sb->s_blocksize = 4096;
> +	sb->s_blocksize_bits = 12;
> +
>  	if (!param || !param->disable_zeroout)
>  		sbi->s_extent_max_zeroout_kb = 32;
>  
>  	/* setup the mock inode */
>  	k_ctx.k_ei = kzalloc_obj(struct ext4_inode_info);
> -	if (k_ctx.k_ei == NULL)
> -		return -ENOMEM;
> +	if (k_ctx.k_ei == NULL) {
> +		err = -ENOMEM;
> +		goto out_deactivate;
> +	}
>  	ei = k_ctx.k_ei;
>  	inode = &ei->vfs_inode;
>  
>  	err = ext4_es_register_shrinker(sbi);
>  	if (err)
> -		return err;
> +		goto out_deactivate;
>  
>  	ext4_es_init_tree(&ei->i_es_tree);
>  	rwlock_init(&ei->i_es_lock);
> @@ -267,8 +271,10 @@ static int extents_kunit_init(struct kunit *test)
>  	inode->i_sb = sb;
>  
>  	k_ctx.k_data = kzalloc(EXT_DATA_LEN * 4096, GFP_KERNEL);
> -	if (k_ctx.k_data == NULL)
> -		return -ENOMEM;
> +	if (k_ctx.k_data == NULL) {
> +		err = -ENOMEM;
> +		goto out_deactivate;
> +	}
>  
>  	/*
>  	 * set the data area to a junk value
> @@ -313,6 +319,20 @@ static int extents_kunit_init(struct kunit *test)
>  	up_write(&sb->s_umount);
>  
>  	return 0;
> +
> +out_deactivate:
> +	kfree(k_ctx.k_ei);
> +	k_ctx.k_ei = NULL;
> +
> +	kfree(k_ctx.k_data);
> +	k_ctx.k_data = NULL;
> +
> +	if (sbi->s_es_shrinker)
> +		ext4_es_unregister_shrinker(sbi);

I don't think this extra check is necessary.
ext4_es_unregister_shrinker() already has checks in place.

Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

> +	deactivate_locked_super(sb);
> +	kfree(sbi);
> +
> +	return err;
>  }
>  
>  /*
> -- 
> 2.34.1

Re: [PATCH v3 4/5] ext4: fix possible null-ptr-deref in extents_kunit_exit()

[Ritesh Harjani]

Ye Bin <yebin@huaweicloud.com> writes:

> From: Ye Bin <yebin10@huawei.com>
>
> There's issue as follows:
> KASAN: null-ptr-deref in range [0x00000000000002c0-0x00000000000002c7]
> Tainted: [E]=UNSIGNED_MODULE, [N]=TEST
> RIP: 0010:extents_kunit_exit+0x2e/0xc0 [ext4_test]
> Call Trace:
>  <TASK>
>  kunit_try_run_case_cleanup+0xbc/0x100 [kunit]
>  kunit_generic_run_threadfn_adapter+0x89/0x100 [kunit]
>  kthread+0x408/0x540
>  ret_from_fork+0xa76/0xdf0
>  ret_from_fork_asm+0x1a/0x30
>
> Above issue happens as extents_kunit_init() init testcase failed.
> So test if testcase is inited success.
>

LGTM.
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>


> Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/ext4/extents-test.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index 543236a31e13..6ae72986ca9c 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c
> @@ -142,9 +142,12 @@ static struct file_system_type ext_fs_type = {
>  
>  static void extents_kunit_exit(struct kunit *test)
>  {
> -	struct super_block *sb = k_ctx.k_ei->vfs_inode.i_sb;
> -	struct ext4_sb_info *sbi = sb->s_fs_info;
> +	struct ext4_sb_info *sbi;
>  
> +	if (!k_ctx.k_ei)
> +		return;
> +
> +	sbi = k_ctx.k_ei->vfs_inode.i_sb->s_fs_info;
>  	ext4_es_unregister_shrinker(sbi);
>  	deactivate_super(sbi->s_sb);
>  	kfree(sbi);
> -- 
> 2.34.1

Re: [PATCH v3 5/5] ext4: fix possible null-ptr-deref in mbt_kunit_exit()

[Ritesh Harjani]

Ye Bin <yebin@huaweicloud.com> writes:

> From: Ye Bin <yebin10@huawei.com>
>
> There's issue as follows:
>     # test_new_blocks_simple: failed to initialize: -12
> KASAN: null-ptr-deref in range [0x0000000000000638-0x000000000000063f]
> Tainted: [E]=UNSIGNED_MODULE, [N]=TEST
> RIP: 0010:mbt_kunit_exit+0x5e/0x3e0 [ext4_test]
> Call Trace:
>  <TASK>
>  kunit_try_run_case_cleanup+0xbc/0x100 [kunit]
>  kunit_generic_run_threadfn_adapter+0x89/0x100 [kunit]
>  kthread+0x408/0x540
>  ret_from_fork+0xa76/0xdf0
>  ret_from_fork_asm+0x1a/0x30
>
> If mbt_kunit_init() init testcase failed will lead to null-ptr-deref.
> So add test if 'sb' is inited success in mbt_kunit_exit().
>

LGTM.
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>


> Fixes: 7c9fa399a369 ("ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/ext4/mballoc-test.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/mballoc-test.c b/fs/ext4/mballoc-test.c
> index c75b91ae0cf0..90ed505fa4b1 100644
> --- a/fs/ext4/mballoc-test.c
> +++ b/fs/ext4/mballoc-test.c
> @@ -362,7 +362,6 @@ static int mbt_kunit_init(struct kunit *test)
>  		return ret;
>  	}
>  
> -	test->priv = sb;
>  	kunit_activate_static_stub(test,
>  				   ext4_read_block_bitmap_nowait,
>  				   ext4_read_block_bitmap_nowait_stub);
> @@ -383,6 +382,8 @@ static int mbt_kunit_init(struct kunit *test)
>  		return -ENOMEM;
>  	}
>  
> +	test->priv = sb;
> +
>  	return 0;
>  }
>  
> @@ -390,6 +391,9 @@ static void mbt_kunit_exit(struct kunit *test)
>  {
>  	struct super_block *sb = (struct super_block *)test->priv;
>  
> +	if (!sb)
> +		return;
> +
>  	mbt_mb_release(sb);
>  	mbt_ctx_release(sb);
>  	mbt_ext4_free_super_block(sb);
> -- 
> 2.34.1

Re: [PATCH v3 1/5] ext4: call deactivate_super() in extents_kunit_exit()

[Ojaswin Mujoo]

On Sat, Mar 14, 2026 at 03:48:59PM +0800, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> Call deactivate_super() is called in extents_kunit_exit() to cleanup
> the file system resource.
> 
> Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/ext4/extents-test.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index 5496b2c8e2cd..e3d23e3cda87 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c
> @@ -146,6 +146,7 @@ static void extents_kunit_exit(struct kunit *test)
>  	struct ext4_sb_info *sbi = sb->s_fs_info;
>  
>  	ext4_es_unregister_shrinker(sbi);
> +	deactivate_super(sbi->s_sb);
>  	kfree(sbi);
>  	kfree(k_ctx.k_ei);
>  	kfree(k_ctx.k_data);
> -- 
> 2.34.1

Hi,

We need to keep this patch after Patch 2 because else there is a
deadlock:

extents_kunit_init()
  sget
    down_write(s_umount)
extents_kunit_exit()
  deactivate_super
    down_write(s_umount)

Regards,
ojaswin

Re: [PATCH v3 2/5] ext4: fix miss unlock 'sb->s_umount' in extents_kunit_init()

[Ojaswin Mujoo]

On Sat, Mar 14, 2026 at 03:49:00PM +0800, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> There's warning as follows when do ext4 kunit test:
> WARNING: kunit_try_catch/15923 still has locks held!
> 7.0.0-rc3-next-20260309-00028-g73f965a1bbb1-dirty #281 Tainted: G            E    N
> 1 lock held by kunit_try_catch/15923:
>  #0: ffff888139f860e0 (&type->s_umount_key#70/1){+.+.}-{4:4}, at: alloc_super.constprop.0+0x172/0xa90
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x180/0x1b0
>  debug_check_no_locks_held+0xc8/0xd0
>  do_exit+0x1502/0x2b20
>  kthread+0x3a9/0x540
>  ret_from_fork+0xa76/0xdf0
>  ret_from_fork_asm+0x1a/0x30
> 
> As sget() will return 'sb' which holds 's->s_umount' lock. However,
> "extents-test" miss unlock this lock.
> So unlock 's->s_umount' in the end of extents_kunit_init().
> 
> Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/ext4/extents-test.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index e3d23e3cda87..3d4663d99eb1 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c
> @@ -310,6 +310,8 @@ static int extents_kunit_init(struct kunit *test)
>  	kunit_activate_static_stub(test, ext4_ext_zeroout, ext4_ext_zeroout_stub);
>  	kunit_activate_static_stub(test, ext4_issue_zeroout,
>  				   ext4_issue_zeroout_stub);
> +	up_write(&sb->s_umount);
> +

Looks good, feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Just make sure to put this before patch 1 (more info in review to patch
1)

Regards,
ojaswin

>  	return 0;
>  }
>  
> -- 
> 2.34.1
> 

Re: [PATCH v3 3/5] ext4: fix the error handling process in extents_kunit_init).

[Ojaswin Mujoo]

On Sat, Mar 14, 2026 at 03:49:01PM +0800, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> The error processing in extents_kunit_init() is improper, causing
> resource leakage.
> Reconstruct the error handling process to prevent potential resource
> leaks
> 
> Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
> Signed-off-by: Ye Bin <yebin10@huawei.com>

Looks good,

Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Regards,
ojaswin

> ---
>  fs/ext4/extents-test.c | 44 ++++++++++++++++++++++++++++++------------
>  1 file changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index 3d4663d99eb1..543236a31e13 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c
> @@ -225,33 +225,37 @@ static int extents_kunit_init(struct kunit *test)
>  		(struct kunit_ext_test_param *)(test->param_value);
>  	int err;
>  
> -	sb = sget(&ext_fs_type, NULL, ext_set, 0, NULL);
> -	if (IS_ERR(sb))
> -		return PTR_ERR(sb);
> -
> -	sb->s_blocksize = 4096;
> -	sb->s_blocksize_bits = 12;
> -
>  	sbi = kzalloc_obj(struct ext4_sb_info);
>  	if (sbi == NULL)
>  		return -ENOMEM;
>  
> +	sb = sget(&ext_fs_type, NULL, ext_set, 0, NULL);
> +	if (IS_ERR(sb)) {
> +		kfree(sbi);
> +		return PTR_ERR(sb);
> +	}
> +
>  	sbi->s_sb = sb;
>  	sb->s_fs_info = sbi;
>  
> +	sb->s_blocksize = 4096;
> +	sb->s_blocksize_bits = 12;
> +
>  	if (!param || !param->disable_zeroout)
>  		sbi->s_extent_max_zeroout_kb = 32;
>  
>  	/* setup the mock inode */
>  	k_ctx.k_ei = kzalloc_obj(struct ext4_inode_info);
> -	if (k_ctx.k_ei == NULL)
> -		return -ENOMEM;
> +	if (k_ctx.k_ei == NULL) {
> +		err = -ENOMEM;
> +		goto out_deactivate;
> +	}
>  	ei = k_ctx.k_ei;
>  	inode = &ei->vfs_inode;
>  
>  	err = ext4_es_register_shrinker(sbi);
>  	if (err)
> -		return err;
> +		goto out_deactivate;
>  
>  	ext4_es_init_tree(&ei->i_es_tree);
>  	rwlock_init(&ei->i_es_lock);
> @@ -267,8 +271,10 @@ static int extents_kunit_init(struct kunit *test)
>  	inode->i_sb = sb;
>  
>  	k_ctx.k_data = kzalloc(EXT_DATA_LEN * 4096, GFP_KERNEL);
> -	if (k_ctx.k_data == NULL)
> -		return -ENOMEM;
> +	if (k_ctx.k_data == NULL) {
> +		err = -ENOMEM;
> +		goto out_deactivate;
> +	}
>  
>  	/*
>  	 * set the data area to a junk value
> @@ -313,6 +319,20 @@ static int extents_kunit_init(struct kunit *test)
>  	up_write(&sb->s_umount);
>  
>  	return 0;
> +
> +out_deactivate:
> +	kfree(k_ctx.k_ei);
> +	k_ctx.k_ei = NULL;
> +
> +	kfree(k_ctx.k_data);
> +	k_ctx.k_data = NULL;
> +
> +	if (sbi->s_es_shrinker)
> +		ext4_es_unregister_shrinker(sbi);
> +	deactivate_locked_super(sb);
> +	kfree(sbi);
> +
> +	return err;
>  }
>  
>  /*
> -- 
> 2.34.1
> 

Re: [PATCH v3 4/5] ext4: fix possible null-ptr-deref in extents_kunit_exit()

[Ojaswin Mujoo]

On Sat, Mar 14, 2026 at 03:49:02PM +0800, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> There's issue as follows:
> KASAN: null-ptr-deref in range [0x00000000000002c0-0x00000000000002c7]
> Tainted: [E]=UNSIGNED_MODULE, [N]=TEST
> RIP: 0010:extents_kunit_exit+0x2e/0xc0 [ext4_test]
> Call Trace:
>  <TASK>
>  kunit_try_run_case_cleanup+0xbc/0x100 [kunit]
>  kunit_generic_run_threadfn_adapter+0x89/0x100 [kunit]
>  kthread+0x408/0x540
>  ret_from_fork+0xa76/0xdf0
>  ret_from_fork_asm+0x1a/0x30
> 
> Above issue happens as extents_kunit_init() init testcase failed.
> So test if testcase is inited success.
> 
> Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/ext4/extents-test.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index 543236a31e13..6ae72986ca9c 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c
> @@ -142,9 +142,12 @@ static struct file_system_type ext_fs_type = {
>  
>  static void extents_kunit_exit(struct kunit *test)
>  {
> -	struct super_block *sb = k_ctx.k_ei->vfs_inode.i_sb;
> -	struct ext4_sb_info *sbi = sb->s_fs_info;
> +	struct ext4_sb_info *sbi;
>  
> +	if (!k_ctx.k_ei)
> +		return;
> +
> +	sbi = k_ctx.k_ei->vfs_inode.i_sb->s_fs_info;
>  	ext4_es_unregister_shrinker(sbi);
>  	deactivate_super(sbi->s_sb);
>  	kfree(sbi);

Looks good, feel free to add:

Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Regards,
ojaswin

> -- 
> 2.34.1
> 

Re: [PATCH v3 5/5] ext4: fix possible null-ptr-deref in mbt_kunit_exit()

[Ojaswin Mujoo]

On Sat, Mar 14, 2026 at 03:49:03PM +0800, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> There's issue as follows:
>     # test_new_blocks_simple: failed to initialize: -12
> KASAN: null-ptr-deref in range [0x0000000000000638-0x000000000000063f]
> Tainted: [E]=UNSIGNED_MODULE, [N]=TEST
> RIP: 0010:mbt_kunit_exit+0x5e/0x3e0 [ext4_test]
> Call Trace:
>  <TASK>
>  kunit_try_run_case_cleanup+0xbc/0x100 [kunit]
>  kunit_generic_run_threadfn_adapter+0x89/0x100 [kunit]
>  kthread+0x408/0x540
>  ret_from_fork+0xa76/0xdf0
>  ret_from_fork_asm+0x1a/0x30
> 
> If mbt_kunit_init() init testcase failed will lead to null-ptr-deref.
> So add test if 'sb' is inited success in mbt_kunit_exit().
> 
> Fixes: 7c9fa399a369 ("ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc")
> Signed-off-by: Ye Bin <yebin10@huawei.com>

Hi Ye, looks good.

Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Regards,
ojaswin

> ---
>  fs/ext4/mballoc-test.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/mballoc-test.c b/fs/ext4/mballoc-test.c
> index c75b91ae0cf0..90ed505fa4b1 100644
> --- a/fs/ext4/mballoc-test.c
> +++ b/fs/ext4/mballoc-test.c
> @@ -362,7 +362,6 @@ static int mbt_kunit_init(struct kunit *test)
>  		return ret;
>  	}
>  
> -	test->priv = sb;
>  	kunit_activate_static_stub(test,
>  				   ext4_read_block_bitmap_nowait,
>  				   ext4_read_block_bitmap_nowait_stub);
> @@ -383,6 +382,8 @@ static int mbt_kunit_init(struct kunit *test)
>  		return -ENOMEM;
>  	}
>  
> +	test->priv = sb;
> +
>  	return 0;
>  }
>  
> @@ -390,6 +391,9 @@ static void mbt_kunit_exit(struct kunit *test)
>  {
>  	struct super_block *sb = (struct super_block *)test->priv;
>  
> +	if (!sb)
> +		return;
> +
>  	mbt_mb_release(sb);
>  	mbt_ctx_release(sb);
>  	mbt_ext4_free_super_block(sb);
> -- 
> 2.34.1
> 

Re: [PATCH v3 3/5] ext4: fix the error handling process in extents_kunit_init).

[yebin (H)]

On 2026/3/14 20:29, Ritesh Harjani (IBM) wrote:
> Ye Bin <yebin@huaweicloud.com> writes:
>
>> From: Ye Bin <yebin10@huawei.com>
>>
>> The error processing in extents_kunit_init() is improper, causing
>> resource leakage.
>> Reconstruct the error handling process to prevent potential resource
>> leaks
>>
>
> Minor nit.
>
>> Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>> ---
>>   fs/ext4/extents-test.c | 44 ++++++++++++++++++++++++++++++------------
>>   1 file changed, 32 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
>> index 3d4663d99eb1..543236a31e13 100644
>> --- a/fs/ext4/extents-test.c
>> +++ b/fs/ext4/extents-test.c
>> @@ -225,33 +225,37 @@ static int extents_kunit_init(struct kunit *test)
>>   		(struct kunit_ext_test_param *)(test->param_value);
>>   	int err;
>>
>> -	sb = sget(&ext_fs_type, NULL, ext_set, 0, NULL);
>> -	if (IS_ERR(sb))
>> -		return PTR_ERR(sb);
>> -
>> -	sb->s_blocksize = 4096;
>> -	sb->s_blocksize_bits = 12;
>> -
>>   	sbi = kzalloc_obj(struct ext4_sb_info);
>>   	if (sbi == NULL)
>>   		return -ENOMEM;
>>
>> +	sb = sget(&ext_fs_type, NULL, ext_set, 0, NULL);
>> +	if (IS_ERR(sb)) {
>> +		kfree(sbi);
>> +		return PTR_ERR(sb);
>> +	}
>> +
>>   	sbi->s_sb = sb;
>>   	sb->s_fs_info = sbi;
>>
>> +	sb->s_blocksize = 4096;
>> +	sb->s_blocksize_bits = 12;
>> +
>>   	if (!param || !param->disable_zeroout)
>>   		sbi->s_extent_max_zeroout_kb = 32;
>>
>>   	/* setup the mock inode */
>>   	k_ctx.k_ei = kzalloc_obj(struct ext4_inode_info);
>> -	if (k_ctx.k_ei == NULL)
>> -		return -ENOMEM;
>> +	if (k_ctx.k_ei == NULL) {
>> +		err = -ENOMEM;
>> +		goto out_deactivate;
>> +	}
>>   	ei = k_ctx.k_ei;
>>   	inode = &ei->vfs_inode;
>>
>>   	err = ext4_es_register_shrinker(sbi);
>>   	if (err)
>> -		return err;
>> +		goto out_deactivate;
>>
>>   	ext4_es_init_tree(&ei->i_es_tree);
>>   	rwlock_init(&ei->i_es_lock);
>> @@ -267,8 +271,10 @@ static int extents_kunit_init(struct kunit *test)
>>   	inode->i_sb = sb;
>>
>>   	k_ctx.k_data = kzalloc(EXT_DATA_LEN * 4096, GFP_KERNEL);
>> -	if (k_ctx.k_data == NULL)
>> -		return -ENOMEM;
>> +	if (k_ctx.k_data == NULL) {
>> +		err = -ENOMEM;
>> +		goto out_deactivate;
>> +	}
>>
>>   	/*
>>   	 * set the data area to a junk value
>> @@ -313,6 +319,20 @@ static int extents_kunit_init(struct kunit *test)
>>   	up_write(&sb->s_umount);
>>
>>   	return 0;
>> +
>> +out_deactivate:
>> +	kfree(k_ctx.k_ei);
>> +	k_ctx.k_ei = NULL;
>> +
>> +	kfree(k_ctx.k_data);
>> +	k_ctx.k_data = NULL;
>> +
>> +	if (sbi->s_es_shrinker)
>> +		ext4_es_unregister_shrinker(sbi);
>
> I don't think this extra check is necessary.
> ext4_es_unregister_shrinker() already has checks in place.
>
I was mainly thinking about what abnormal behavior might be caused if 
percpu_counter_destroy() is called before percpu_counter_init() is called.
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>
>> +	deactivate_locked_super(sb);
>> +	kfree(sbi);
>> +
>> +	return err;
>>   }
>>
>>   /*
>> --
>> 2.34.1
>
> .
>

Re: [PATCH v3 1/5] ext4: call deactivate_super() in extents_kunit_exit()

[yebin (H)]

On 2026/3/15 13:39, Ojaswin Mujoo wrote:
> On Sat, Mar 14, 2026 at 03:48:59PM +0800, Ye Bin wrote:
>> From: Ye Bin <yebin10@huawei.com>
>>
>> Call deactivate_super() is called in extents_kunit_exit() to cleanup
>> the file system resource.
>>
>> Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>> ---
>>   fs/ext4/extents-test.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
>> index 5496b2c8e2cd..e3d23e3cda87 100644
>> --- a/fs/ext4/extents-test.c
>> +++ b/fs/ext4/extents-test.c
>> @@ -146,6 +146,7 @@ static void extents_kunit_exit(struct kunit *test)
>>   	struct ext4_sb_info *sbi = sb->s_fs_info;
>>
>>   	ext4_es_unregister_shrinker(sbi);
>> +	deactivate_super(sbi->s_sb);
>>   	kfree(sbi);
>>   	kfree(k_ctx.k_ei);
>>   	kfree(k_ctx.k_data);
>> --
>> 2.34.1
>
> Hi,
>
> We need to keep this patch after Patch 2 because else there is a
> deadlock:
>
> extents_kunit_init()
>    sget
>      down_write(s_umount)
> extents_kunit_exit()
>    deactivate_super
>      down_write(s_umount)
>
> Regards,
> ojaswin
>
Thank you for your suggestion. I will change the order of the two
patches and release another version.
>
> .
>