Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters

[Daniel Gomez <da.gomez@kernel.org>]

On 05/11/2025 14.03, Daniel Gomez wrote:
> On 10/10/2025 05.06, Kees Cook wrote:
>>  v2:
>>  - use static_assert instead of _Static_assert
>>  - add Hans's Reviewed-by's
>>  v1: https://lore.kernel.org/lkml/20251008033844.work.801-kees@kernel.org/
>>
>> Hi!
>>
>> A long time ago we had an issue with embedded NUL bytes in MODULE_INFO
>> strings[1]. While this stands out pretty strongly when you look at the
>> code, and we can't do anything about a binary module that just plain lies,
>> we never actually implemented the trivial compile-time check needed to
>> detect it.
>>
>> Add this check (and fix 2 instances of needless trailing semicolons that
>> this change exposed).
>>
>> Note that these patches were produced as part of another LLM exercise.
>> This time I wanted to try "what happens if I ask an LLM to go read
>> a specific LWN article and write a patch based on a discussion?" It
>> pretty effortlessly chose and implemented a suggested solution, tested
>> the change, and fixed new build warnings in the process.
>>
>> Since this was a relatively short session, here's an overview of the
>> prompts involved as I guided it through a clean change and tried to see
>> how it would reason about static_assert vs _Static_assert. (It wanted
>> to use what was most common, not what was the current style -- we may
>> want to update the comment above the static_assert macro to suggest
>> using _Static_assert directly these days...)
>>
>>   I want to fix a weakness in the module info strings. Read about it
>>   here: https://lwn.net/Articles/82305/
>>
>>   Since it's only "info" that we need to check, can you reduce the checks
>>   to just that instead of all the other stuff?
>>
>>   I think the change to the comment is redundent, and that should be
>>   in a commit log instead. Let's just keep the change to the static assert.
>>
>>   Is "static_assert" the idiomatic way to use a static assert in this
>>   code base? I've seen _Static_assert used sometimes.
>>
>>   What's the difference between the two?
>>
>>   Does Linux use C11 by default now?
>>
>>   Then let's not use the wrapper any more.
>>
>>   Do an "allmodconfig all -s" build to verify this works for all modules
>>   in the kernel.
>>
>>
>> Thanks!
>>
>> -Kees
>>
>> [1] https://lwn.net/Articles/82305/
>>
>> Kees Cook (3):
>>   media: dvb-usb-v2: lmedm04: Fix firmware macro definitions
>>   media: radio: si470x: Fix DRIVER_AUTHOR macro definition
>>   module: Add compile-time check for embedded NUL characters
>>
>>  include/linux/moduleparam.h                   |  3 +++
>>  drivers/media/radio/si470x/radio-si470x-i2c.c |  2 +-
>>  drivers/media/usb/dvb-usb-v2/lmedm04.c        | 12 ++++++------
>>  3 files changed, 10 insertions(+), 7 deletions(-)
>>
> 
> Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
> 
> I have also tested a build of v6.18-rc3 + patches using allmodconfig:
> 
> Tested-by: Daniel Gomez <da.gomez@samsung.com>
> 

I forgot to mention it required the following patch for the build to succeed:

dmaengine: mmp_pdma: fix DMA mask handling

https://lore.kernel.org/all/176061935426.510550.684278188506408313.b4-ty@kernel.org/