linux-kernel - Re: [PATCH] kunit: cs_dsp: Depend on FW_CS

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <6ed7f92e-f621-4680-8f80-dc793e093a39@opensource.cirrus.com>
Date: Sat, 22 Mar 2025 20:28:05 +0000
From: Richard Fitzgerald <rf@...nsource.cirrus.com>
To: Nico Pache <npache@...hat.com>
Cc: broonie@...nel.org, patches@...nsource.cirrus.com,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        kunit-dev@...glegroups.com, simont@...nsource.cirrus.com,
        ckeepax@...nsource.cirrus.com, brendan.higgins@...ux.dev,
        davidgow@...gle.com, rmoar@...gle.com, johannes.berg@...el.com,
        sj@...nel.org
Subject: Re: [PATCH] kunit: cs_dsp: Depend on FW_CS_DSP rather then enabling
 it

On 22/3/25 19:02, Richard Fitzgerald wrote:
> On 22/3/25 10:11, Richard Fitzgerald wrote:
>> On 20/3/25 17:35, Nico Pache wrote:
>>> Sorry links got mangled
>>>
>> Thanks. I'm on vacation right now but I'll take a look through
>> all those when I have time.
>>
>> The unterminated string bugfix is this:
>> https://lore.kernel.org/all/20250211-cs_dsp-kunit-strings-v1-1- 
>> d9bc2035d154@...utronix.de/
>>
>> I got lucky on all the UM, X86 and ARM builds I tested.
>>
> It looks like that bugfix has got lost.
> Mark sent an email on 20 Feb to say it has been merged into his
> sound tree. But that patch isn't in torvalds/master.
> 
> It's possible that the unterminated strings are causing the problems
> you have seen.

I've reproduced this, and it's not a bug in the test. The test is
correctly finding a change in behavior that was introduced into cs_dsp.
This doesn't affect normal firmware files that have real content. But
the tests create an "empty" firmware file that has only the header. This
is useful, so we can have a dummy firmware file that doesn't create
side-effects when it is loaded.

A normal file contains data blocks, which will set ret = 0 when they are
processed. An "empty" file does not have any data block so ret is never
set to 0. The end of the function used to do this:

ret = regmap_async_complete(regmap);

which would make ret == 0 on exit from the function. But a recent commit
removed the async regmap writes and so removed this line. A real
firmware file will already have set ret == 0 when it processed a data
block. The dummy files don't.

So... we have a kunit test, but KUNIT_ALL_TESTS doesn't mean "run all
tests". It means "run test for modules that are already selected."
Modules must be manually selected to get a KUNIT_ALL_TESTS run to
actually run the test code. So it didn't get a kunit test run.

I'll send a patch to fix this.