linux-ext4 - Re: [PATCH] ext4: fix crash on test_mb_mark

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0213b23c-e746-4e05-b151-8b0f5bd3d7d2@huawei.com>
Date: Sat, 26 Jul 2025 09:47:15 +0800
From: Baokun Li <libaokun1@...wei.com>
To: Theodore Ts'o <tytso@....edu>, Jan Kara <jack@...e.cz>, Zhang Yi
	<yi.zhang@...weicloud.com>
CC: <linux-ext4@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <adilger.kernel@...ger.ca>,
	<ojaswin@...ux.ibm.com>, <linux@...ck-us.net>, <yi.zhang@...wei.com>,
	<yukuai3@...wei.com>, <yangerkun@...wei.com>
Subject: Re: [PATCH] ext4: fix crash on test_mb_mark_used kunit tests

On 7/25/2025 9:15 PM, Theodore Ts'o wrote:
> On Fri, Jul 25, 2025 at 01:06:18PM +0200, Jan Kara wrote:
>>> This patch applies to the kernel that has only merged bbe11dd13a3f
>>> ("ext4: fix largest free orders lists corruption on mb_optimize_scan
>>> switch"), but not merged 458bfb991155 ("ext4: convert free groups order
>>> lists to xarrays").
>> Hum, I think it would be best to just squash this into bbe11dd13a3f and
>> then just rebase & squash the other unittest fixup to the final commit when
>> we have to rebase anyway. Because otherwise backports to stable kernel will
>> quickly become rather messy.
> What I ended up doing was to add a squashed combination of these two
> commits and dropped it in before the block allocation scalabiltity
> with the following commit description:
>
>      ext4: initialize superblock fields in the kballoc-test.c kunit tests
>      
>      Various changes in the "ext4: better scalability for ext4 block
>      allocation" patch series have resulted in kunit test failures, most
>      notably in the test_new_blocks_simple and the test_mb_mark_used tests.
>      The root cause of these failures is that various in-memory ext4 data
>      structures were not getting initialized, and while previous versions
>      of the functions exercised by the unit tests didn't use these
>      structure members, this was arguably a test bug.
>      
>      Since one of the patches in the block allocation scalability patches
>      is a fix which is has a cc:stable tag, this commit also has a
>      cc:stable tag.
>      
>      CC: stable@...r.kernel.org
>      Link: https://lore.kernel.org/r/20250714130327.1830534-1-libaokun1@huawei.com
>      Link: https://patch.msgid.link/20250725021550.3177573-1-yi.zhang@huaweicloud.com
>      Link: https://patch.msgid.link/20250725021654.3188798-1-yi.zhang@huaweicloud.com
>      Reported-by: Guenter Roeck <linux@...ck-us.net>
>      Closes: https://lore.kernel.org/linux-ext4/b0635ad0-7ebf-4152-a69b-58e7e87d5085@roeck-us.net/
>      Tested-by: Guenter Roeck <linux@...ck-us.net>
>      Signed-off-by: Zhang Yi <yi.zhang@...wei.com>
>      Signed-off-by: Theodore Ts'o <tytso@....edu>
>
> Then in the commit "ext4: convert free groups order lists to xarrays"
> which removed list_head, I modified it to remove the linked list
> initialization from mballoc-test.c, since that's the commit which
> removed those structures.

This looks good to me. Thank you for helping to adapt this patch!

>
> In the future, we should try to make sure that when we modify data
> structures to add or remove struct elements, that we also make sure
> that kunit test should also be updated.  To that end, I've updated the
> kbuild script[1] in xfstests-bld repo so that "kbuild --test" will run
> the Kunit tests.  Hopefully reducing the friction for running tests
> will encourage more kunit tests to be created and so they will kept
> under regular maintenance.
>
> [1] https://github.com/tytso/xfstests-bld/blob/master/kernel-build/kbuild

Yeah, unit tests are a much more efficient way to catch problems compared
to full system tests. Running them regularly would be a great way to
quickly surface issues.

On top of that, I think it's worth revisiting our current code and cleaning
up some of the logic. Specifically, refactoring initialization functions to
align with the single-responsibility principle would enable reuse between
production and testing flows, and minimize strange edge cases we’ve been
seeing.


Cheers,
Baokun