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

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

[Kees Cook]

 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(-)

-- 
2.34.1

[PATCH v2 1/3] media: dvb-usb-v2: lmedm04: Fix firmware macro definitions

[Kees Cook]

The firmware filename macros incorrectly included semicolons in their
string literal definitions. Right now, this wasn't causing any real
problem, but coming changes to the MODULE_INFO() macro make this more
sensitive. Specifically, when used with MODULE_FIRMWARE(), this
created syntax errors during macro expansion:

    MODULE_FIRMWARE(LME2510_C_S7395);

expands to:

    MODULE_INFO(firmware, "dvb-usb-lme2510c-s7395.fw";)
                                                     ^
                                          syntax error

Remove the trailing semicolons from all six firmware filename macro
definitions. Semicolons should only appear at the point of use, not in
the macro definition.

Reviewed-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Malcolm Priestley <tvboxspy@gmail.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: <linux-media@vger.kernel.org>
---
 drivers/media/usb/dvb-usb-v2/lmedm04.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c b/drivers/media/usb/dvb-usb-v2/lmedm04.c
index 0c510035805b..05c18b6de5c6 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -70,12 +70,12 @@
 #include "ts2020.h"
 
 
-#define LME2510_C_S7395	"dvb-usb-lme2510c-s7395.fw";
-#define LME2510_C_LG	"dvb-usb-lme2510c-lg.fw";
-#define LME2510_C_S0194	"dvb-usb-lme2510c-s0194.fw";
-#define LME2510_C_RS2000 "dvb-usb-lme2510c-rs2000.fw";
-#define LME2510_LG	"dvb-usb-lme2510-lg.fw";
-#define LME2510_S0194	"dvb-usb-lme2510-s0194.fw";
+#define LME2510_C_S7395	"dvb-usb-lme2510c-s7395.fw"
+#define LME2510_C_LG	"dvb-usb-lme2510c-lg.fw"
+#define LME2510_C_S0194	"dvb-usb-lme2510c-s0194.fw"
+#define LME2510_C_RS2000 "dvb-usb-lme2510c-rs2000.fw"
+#define LME2510_LG	"dvb-usb-lme2510-lg.fw"
+#define LME2510_S0194	"dvb-usb-lme2510-s0194.fw"
 
 /* debug */
 static int dvb_usb_lme2510_debug;
-- 
2.34.1

Re: [PATCH v2 1/3] media: dvb-usb-v2: lmedm04: Fix firmware macro definitions

[Mauro Carvalho Chehab]

Em Thu,  9 Oct 2025 20:06:07 -0700
Kees Cook <kees@kernel.org> escreveu:

> The firmware filename macros incorrectly included semicolons in their
> string literal definitions. Right now, this wasn't causing any real
> problem, but coming changes to the MODULE_INFO() macro make this more
> sensitive. Specifically, when used with MODULE_FIRMWARE(), this
> created syntax errors during macro expansion:
> 
>     MODULE_FIRMWARE(LME2510_C_S7395);
> 
> expands to:
> 
>     MODULE_INFO(firmware, "dvb-usb-lme2510c-s7395.fw";)
>                                                      ^
>                                           syntax error
> 
> Remove the trailing semicolons from all six firmware filename macro
> definitions. Semicolons should only appear at the point of use, not in
> the macro definition.
> 
> Reviewed-by: Hans Verkuil <hverkuil+cisco@kernel.org>
> Signed-off-by: Kees Cook <kees@kernel.org>

LGTM.
Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


> ---
> Cc: Malcolm Priestley <tvboxspy@gmail.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: <linux-media@vger.kernel.org>
> ---
>  drivers/media/usb/dvb-usb-v2/lmedm04.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c b/drivers/media/usb/dvb-usb-v2/lmedm04.c
> index 0c510035805b..05c18b6de5c6 100644
> --- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
> +++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
> @@ -70,12 +70,12 @@
>  #include "ts2020.h"
>  
>  
> -#define LME2510_C_S7395	"dvb-usb-lme2510c-s7395.fw";
> -#define LME2510_C_LG	"dvb-usb-lme2510c-lg.fw";
> -#define LME2510_C_S0194	"dvb-usb-lme2510c-s0194.fw";
> -#define LME2510_C_RS2000 "dvb-usb-lme2510c-rs2000.fw";
> -#define LME2510_LG	"dvb-usb-lme2510-lg.fw";
> -#define LME2510_S0194	"dvb-usb-lme2510-s0194.fw";
> +#define LME2510_C_S7395	"dvb-usb-lme2510c-s7395.fw"
> +#define LME2510_C_LG	"dvb-usb-lme2510c-lg.fw"
> +#define LME2510_C_S0194	"dvb-usb-lme2510c-s0194.fw"
> +#define LME2510_C_RS2000 "dvb-usb-lme2510c-rs2000.fw"
> +#define LME2510_LG	"dvb-usb-lme2510-lg.fw"
> +#define LME2510_S0194	"dvb-usb-lme2510-s0194.fw"
>  
>  /* debug */
>  static int dvb_usb_lme2510_debug;

[PATCH v2 2/3] media: radio: si470x: Fix DRIVER_AUTHOR macro definition

[Kees Cook]

The DRIVER_AUTHOR macro incorrectly included a semicolon in its
string literal definition. Right now, this wasn't causing any real
problem, but coming changes to the MODULE_INFO() macro make this more
sensitive. Specifically, when used with MODULE_AUTHOR(), this created
syntax errors during macro expansion:

    MODULE_AUTHOR(DRIVER_AUTHOR);

expands to:

    MODULE_INFO(author, "Joonyoung Shim <jy0922.shim@samsung.com>";)
                                                                  ^
                                                       syntax error

Remove the trailing semicolon from the DRIVER_AUTHOR definition.
Semicolons should only appear at the point of use, not in the macro
definition.

Reviewed-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Hans Verkuil <hverkuil@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: <linux-media@vger.kernel.org>
---
 drivers/media/radio/si470x/radio-si470x-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c
index cdd2ac198f2c..3932a449a1b1 100644
--- a/drivers/media/radio/si470x/radio-si470x-i2c.c
+++ b/drivers/media/radio/si470x/radio-si470x-i2c.c
@@ -10,7 +10,7 @@
 
 
 /* driver definitions */
-#define DRIVER_AUTHOR "Joonyoung Shim <jy0922.shim@samsung.com>";
+#define DRIVER_AUTHOR "Joonyoung Shim <jy0922.shim@samsung.com>"
 #define DRIVER_CARD "Silicon Labs Si470x FM Radio"
 #define DRIVER_DESC "I2C radio driver for Si470x FM Radio Receivers"
 #define DRIVER_VERSION "1.0.2"
-- 
2.34.1

Re: [PATCH v2 2/3] media: radio: si470x: Fix DRIVER_AUTHOR macro definition

[Mauro Carvalho Chehab]

Em Thu,  9 Oct 2025 20:06:08 -0700
Kees Cook <kees@kernel.org> escreveu:

> The DRIVER_AUTHOR macro incorrectly included a semicolon in its
> string literal definition. Right now, this wasn't causing any real
> problem, but coming changes to the MODULE_INFO() macro make this more
> sensitive. Specifically, when used with MODULE_AUTHOR(), this created
> syntax errors during macro expansion:
> 
>     MODULE_AUTHOR(DRIVER_AUTHOR);
> 
> expands to:
> 
>     MODULE_INFO(author, "Joonyoung Shim <jy0922.shim@samsung.com>";)
>                                                                   ^
>                                                        syntax error
> 
> Remove the trailing semicolon from the DRIVER_AUTHOR definition.
> Semicolons should only appear at the point of use, not in the macro
> definition.
> 
> Reviewed-by: Hans Verkuil <hverkuil+cisco@kernel.org>
> Signed-off-by: Kees Cook <kees@kernel.org>

LGTM.
Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

> ---
> Cc: Hans Verkuil <hverkuil@kernel.org>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: <linux-media@vger.kernel.org>
> ---
>  drivers/media/radio/si470x/radio-si470x-i2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c
> index cdd2ac198f2c..3932a449a1b1 100644
> --- a/drivers/media/radio/si470x/radio-si470x-i2c.c
> +++ b/drivers/media/radio/si470x/radio-si470x-i2c.c
> @@ -10,7 +10,7 @@
>  
>  
>  /* driver definitions */
> -#define DRIVER_AUTHOR "Joonyoung Shim <jy0922.shim@samsung.com>";
> +#define DRIVER_AUTHOR "Joonyoung Shim <jy0922.shim@samsung.com>"
>  #define DRIVER_CARD "Silicon Labs Si470x FM Radio"
>  #define DRIVER_DESC "I2C radio driver for Si470x FM Radio Receivers"
>  #define DRIVER_VERSION "1.0.2"

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

[Kees Cook]

Long ago, the kernel module license checks were bypassed by embedding a
NUL character in the MODULE_LICENSE() string[1]. By using a string like
"GPL\0proprietary text", the kernel would only read "GPL" due to C string
termination at the NUL byte, allowing proprietary modules to avoid kernel
tainting and access GPL-only symbols.

The MODULE_INFO() macro stores these strings in the .modinfo ELF
section, and get_next_modinfo() uses strcmp()-family functions
which stop at the first NUL. This split the embedded string into two
separate .modinfo entries, with only the first part being processed by
license_is_gpl_compatible().

Add a compile-time check using static_assert that compares the full
string length (sizeof - 1) against __builtin_strlen(), which stops at
the first NUL. If they differ, compilation fails with a clear error
message.

While this check can still be circumvented by modifying the ELF binary
post-compilation, it prevents accidental embedded NULs and forces
intentional abuse to require deliberate binary manipulation rather than
simple source-level tricks.

Build tested with test modules containing both valid and invalid license
strings. The check correctly rejects:

    MODULE_LICENSE("GPL\0proprietary")

while accepting normal declarations:

    MODULE_LICENSE("GPL")

Link: https://lwn.net/Articles/82305/ [1]
Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Petr Pavlu <petr.pavlu@suse.com>
Cc: Daniel Gomez <da.gomez@kernel.org>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: <linux-modules@vger.kernel.org>
---
 include/linux/moduleparam.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 6907aedc4f74..915f32f7d888 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -26,6 +26,9 @@
 
 /* Generic info of form tag = "info" */
 #define MODULE_INFO(tag, info)					  \
+	static_assert(						  \
+		sizeof(info) - 1 == __builtin_strlen(info),	  \
+		"MODULE_INFO(" #tag ", ...) contains embedded NUL byte"); \
 	static const char __UNIQUE_ID(modinfo)[]			  \
 		__used __section(".modinfo") __aligned(1)		  \
 		= __MODULE_INFO_PREFIX __stringify(tag) "=" info
-- 
2.34.1

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

[Petr Pavlu]

On 10/10/25 5:06 AM, Kees Cook wrote:
> Long ago, the kernel module license checks were bypassed by embedding a
> NUL character in the MODULE_LICENSE() string[1]. By using a string like
> "GPL\0proprietary text", the kernel would only read "GPL" due to C string
> termination at the NUL byte, allowing proprietary modules to avoid kernel
> tainting and access GPL-only symbols.
> 
> The MODULE_INFO() macro stores these strings in the .modinfo ELF
> section, and get_next_modinfo() uses strcmp()-family functions
> which stop at the first NUL. This split the embedded string into two
> separate .modinfo entries, with only the first part being processed by
> license_is_gpl_compatible().
> 
> Add a compile-time check using static_assert that compares the full
> string length (sizeof - 1) against __builtin_strlen(), which stops at
> the first NUL. If they differ, compilation fails with a clear error
> message.
> 
> While this check can still be circumvented by modifying the ELF binary
> post-compilation, it prevents accidental embedded NULs and forces
> intentional abuse to require deliberate binary manipulation rather than
> simple source-level tricks.
> 
> Build tested with test modules containing both valid and invalid license
> strings. The check correctly rejects:
> 
>     MODULE_LICENSE("GPL\0proprietary")
> 
> while accepting normal declarations:
> 
>     MODULE_LICENSE("GPL")
> 
> Link: https://lwn.net/Articles/82305/ [1]
> Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
> Signed-off-by: Kees Cook <kees@kernel.org>

Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>

-- 
Thanks,
Petr

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

[Aaron Tomlin]

On Thu, Oct 09, 2025 at 08:06:09PM -0700, Kees Cook wrote:
> Long ago, the kernel module license checks were bypassed by embedding a
> NUL character in the MODULE_LICENSE() string[1]. By using a string like
> "GPL\0proprietary text", the kernel would only read "GPL" due to C string
> termination at the NUL byte, allowing proprietary modules to avoid kernel
> tainting and access GPL-only symbols.
> 
> The MODULE_INFO() macro stores these strings in the .modinfo ELF
> section, and get_next_modinfo() uses strcmp()-family functions
> which stop at the first NUL. This split the embedded string into two
> separate .modinfo entries, with only the first part being processed by
> license_is_gpl_compatible().
> 
> Add a compile-time check using static_assert that compares the full
> string length (sizeof - 1) against __builtin_strlen(), which stops at
> the first NUL. If they differ, compilation fails with a clear error
> message.
> 
> While this check can still be circumvented by modifying the ELF binary
> post-compilation, it prevents accidental embedded NULs and forces
> intentional abuse to require deliberate binary manipulation rather than
> simple source-level tricks.
> 
> Build tested with test modules containing both valid and invalid license
> strings. The check correctly rejects:
> 
>     MODULE_LICENSE("GPL\0proprietary")
> 
> while accepting normal declarations:
> 
>     MODULE_LICENSE("GPL")
> 
> Link: https://lwn.net/Articles/82305/ [1]
> Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Petr Pavlu <petr.pavlu@suse.com>
> Cc: Daniel Gomez <da.gomez@kernel.org>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: <linux-modules@vger.kernel.org>
> ---
>  include/linux/moduleparam.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index 6907aedc4f74..915f32f7d888 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -26,6 +26,9 @@
>  
>  /* Generic info of form tag = "info" */
>  #define MODULE_INFO(tag, info)					  \
> +	static_assert(						  \
> +		sizeof(info) - 1 == __builtin_strlen(info),	  \
> +		"MODULE_INFO(" #tag ", ...) contains embedded NUL byte"); \
>  	static const char __UNIQUE_ID(modinfo)[]			  \
>  		__used __section(".modinfo") __aligned(1)		  \
>  		= __MODULE_INFO_PREFIX __stringify(tag) "=" info
> -- 
> 2.34.1
> 
> 

Nice!

Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>

-- 
Aaron Tomlin

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

[Hans Verkuil]

On 10/10/2025 05:06, Kees Cook wrote:
> Long ago, the kernel module license checks were bypassed by embedding a
> NUL character in the MODULE_LICENSE() string[1]. By using a string like
> "GPL\0proprietary text", the kernel would only read "GPL" due to C string
> termination at the NUL byte, allowing proprietary modules to avoid kernel
> tainting and access GPL-only symbols.
> 
> The MODULE_INFO() macro stores these strings in the .modinfo ELF
> section, and get_next_modinfo() uses strcmp()-family functions
> which stop at the first NUL. This split the embedded string into two
> separate .modinfo entries, with only the first part being processed by
> license_is_gpl_compatible().
> 
> Add a compile-time check using static_assert that compares the full
> string length (sizeof - 1) against __builtin_strlen(), which stops at
> the first NUL. If they differ, compilation fails with a clear error
> message.
> 
> While this check can still be circumvented by modifying the ELF binary
> post-compilation, it prevents accidental embedded NULs and forces
> intentional abuse to require deliberate binary manipulation rather than
> simple source-level tricks.
> 
> Build tested with test modules containing both valid and invalid license
> strings. The check correctly rejects:
> 
>     MODULE_LICENSE("GPL\0proprietary")
> 
> while accepting normal declarations:
> 
>     MODULE_LICENSE("GPL")

Who will take this series? I can take the first two media patches and
someone else can take this last patch, or I can take all, or someone
else can take all patches. The media patches already have my 'Reviewed-by'.

Any preferences?

Regards,

	Hans

> 
> Link: https://lwn.net/Articles/82305/ [1]
> Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Petr Pavlu <petr.pavlu@suse.com>
> Cc: Daniel Gomez <da.gomez@kernel.org>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: <linux-modules@vger.kernel.org>
> ---
>  include/linux/moduleparam.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index 6907aedc4f74..915f32f7d888 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -26,6 +26,9 @@
>  
>  /* Generic info of form tag = "info" */
>  #define MODULE_INFO(tag, info)					  \
> +	static_assert(						  \
> +		sizeof(info) - 1 == __builtin_strlen(info),	  \
> +		"MODULE_INFO(" #tag ", ...) contains embedded NUL byte"); \
>  	static const char __UNIQUE_ID(modinfo)[]			  \
>  		__used __section(".modinfo") __aligned(1)		  \
>  		= __MODULE_INFO_PREFIX __stringify(tag) "=" info

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

[Daniel Gomez]

On 03/11/2025 09.54, Hans Verkuil wrote:
> On 10/10/2025 05:06, Kees Cook wrote:
>> Long ago, the kernel module license checks were bypassed by embedding a
>> NUL character in the MODULE_LICENSE() string[1]. By using a string like
>> "GPL\0proprietary text", the kernel would only read "GPL" due to C string
>> termination at the NUL byte, allowing proprietary modules to avoid kernel
>> tainting and access GPL-only symbols.
>>
>> The MODULE_INFO() macro stores these strings in the .modinfo ELF
>> section, and get_next_modinfo() uses strcmp()-family functions
>> which stop at the first NUL. This split the embedded string into two
>> separate .modinfo entries, with only the first part being processed by
>> license_is_gpl_compatible().
>>
>> Add a compile-time check using static_assert that compares the full
>> string length (sizeof - 1) against __builtin_strlen(), which stops at
>> the first NUL. If they differ, compilation fails with a clear error
>> message.
>>
>> While this check can still be circumvented by modifying the ELF binary
>> post-compilation, it prevents accidental embedded NULs and forces
>> intentional abuse to require deliberate binary manipulation rather than
>> simple source-level tricks.
>>
>> Build tested with test modules containing both valid and invalid license
>> strings. The check correctly rejects:
>>
>>     MODULE_LICENSE("GPL\0proprietary")
>>
>> while accepting normal declarations:
>>
>>     MODULE_LICENSE("GPL")
> 
> Who will take this series? I can take the first two media patches and
> someone else can take this last patch, or I can take all, or someone
> else can take all patches. The media patches already have my 'Reviewed-by'.
> 
> Any preferences?

I will take patch 3 in modules' tree.


> 
> Regards,
> 
> 	Hans
> 
>>
>> Link: https://lwn.net/Articles/82305/ [1]
>> Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
>> Signed-off-by: Kees Cook <kees@kernel.org>

Reviewed-by: Daniel Gomez <da.gomez@samsung.com>

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

[Hans Verkuil]

On 03/11/2025 09:58, Daniel Gomez wrote:
> On 03/11/2025 09.54, Hans Verkuil wrote:
>> On 10/10/2025 05:06, Kees Cook wrote:
>>> Long ago, the kernel module license checks were bypassed by embedding a
>>> NUL character in the MODULE_LICENSE() string[1]. By using a string like
>>> "GPL\0proprietary text", the kernel would only read "GPL" due to C string
>>> termination at the NUL byte, allowing proprietary modules to avoid kernel
>>> tainting and access GPL-only symbols.
>>>
>>> The MODULE_INFO() macro stores these strings in the .modinfo ELF
>>> section, and get_next_modinfo() uses strcmp()-family functions
>>> which stop at the first NUL. This split the embedded string into two
>>> separate .modinfo entries, with only the first part being processed by
>>> license_is_gpl_compatible().
>>>
>>> Add a compile-time check using static_assert that compares the full
>>> string length (sizeof - 1) against __builtin_strlen(), which stops at
>>> the first NUL. If they differ, compilation fails with a clear error
>>> message.
>>>
>>> While this check can still be circumvented by modifying the ELF binary
>>> post-compilation, it prevents accidental embedded NULs and forces
>>> intentional abuse to require deliberate binary manipulation rather than
>>> simple source-level tricks.
>>>
>>> Build tested with test modules containing both valid and invalid license
>>> strings. The check correctly rejects:
>>>
>>>     MODULE_LICENSE("GPL\0proprietary")
>>>
>>> while accepting normal declarations:
>>>
>>>     MODULE_LICENSE("GPL")
>>
>> Who will take this series? I can take the first two media patches and
>> someone else can take this last patch, or I can take all, or someone
>> else can take all patches. The media patches already have my 'Reviewed-by'.
>>
>> Any preferences?
> 
> I will take patch 3 in modules' tree.

OK, then I'll take patches 1 and 2 in the media tree.

Regards,

	Hans

> 
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>> Link: https://lwn.net/Articles/82305/ [1]
>>> Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
>>> Signed-off-by: Kees Cook <kees@kernel.org>
> 
> Reviewed-by: Daniel Gomez <da.gomez@samsung.com>

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

[Daniel Gomez]

On Thu, 09 Oct 2025 20:06:06 -0700, 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!
> 
> [...]

Applied patch 3, thanks!

[3/3] module: Add compile-time check for embedded NUL characters
      commit: 913359754ea821c4d6f6a77e0449b29984099663

Best regards,
-- 
Daniel Gomez <da.gomez@samsung.com>

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

[Kees Cook]

On Mon, Nov 03, 2025 at 08:49:43PM +0100, Daniel Gomez wrote:
> 
> On Thu, 09 Oct 2025 20:06:06 -0700, 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!
> > 
> > [...]
> 
> Applied patch 3, thanks!
> 
> [3/3] module: Add compile-time check for embedded NUL characters
>       commit: 913359754ea821c4d6f6a77e0449b29984099663

I'm nervous about this going in alone -- it breaks allmodconfig builds
without the media fixes. My intention was to have the media fixes land
first...

Should I send the media fixes to linus right away?

-Kees

-- 
Kees Cook

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

[Daniel Gomez]

On 04/11/2025 01.13, Kees Cook wrote:
> On Mon, Nov 03, 2025 at 08:49:43PM +0100, Daniel Gomez wrote:
>>
>> On Thu, 09 Oct 2025 20:06:06 -0700, 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!
>>>
>>> [...]
>>
>> Applied patch 3, thanks!
>>
>> [3/3] module: Add compile-time check for embedded NUL characters
>>       commit: 913359754ea821c4d6f6a77e0449b29984099663
> 
> I'm nervous about this going in alone -- it breaks allmodconfig builds
> without the media fixes. My intention was to have the media fixes land
> first...
> 
> Should I send the media fixes to linus right away?
> 
> -Kees
> 

I can take both patches. But I think it'd make sense to drop patch 3 first and
then, apply all 3.

Please, Kees, Hans and Mauro, let me know if this is okay with you.

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

[Hans Verkuil]

On 04/11/2025 07:35, Daniel Gomez wrote:
> 
> 
> On 04/11/2025 01.13, Kees Cook wrote:
>> On Mon, Nov 03, 2025 at 08:49:43PM +0100, Daniel Gomez wrote:
>>>
>>> On Thu, 09 Oct 2025 20:06:06 -0700, 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!
>>>>
>>>> [...]
>>>
>>> Applied patch 3, thanks!
>>>
>>> [3/3] module: Add compile-time check for embedded NUL characters
>>>       commit: 913359754ea821c4d6f6a77e0449b29984099663
>>
>> I'm nervous about this going in alone -- it breaks allmodconfig builds
>> without the media fixes. My intention was to have the media fixes land
>> first...
>>
>> Should I send the media fixes to linus right away?
>>
>> -Kees
>>
> 
> I can take both patches. But I think it'd make sense to drop patch 3 first and
> then, apply all 3.
> 
> Please, Kees, Hans and Mauro, let me know if this is okay with you.

I'm fine. If you take it, then I'll drop the media patches from our tree (I merged the
media patches yesterday).

Let me know if you take them.

Regards,

	Hans

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

[Daniel Gomez]

On 04/11/2025 11.35, Hans Verkuil wrote:
> On 04/11/2025 07:35, Daniel Gomez wrote:
>>
>>
>> On 04/11/2025 01.13, Kees Cook wrote:
>>> On Mon, Nov 03, 2025 at 08:49:43PM +0100, Daniel Gomez wrote:
>>>>
>>>> On Thu, 09 Oct 2025 20:06:06 -0700, 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!
>>>>>
>>>>> [...]
>>>>
>>>> Applied patch 3, thanks!
>>>>
>>>> [3/3] module: Add compile-time check for embedded NUL characters
>>>>       commit: 913359754ea821c4d6f6a77e0449b29984099663
>>>
>>> I'm nervous about this going in alone -- it breaks allmodconfig builds
>>> without the media fixes. My intention was to have the media fixes land
>>> first...
>>>
>>> Should I send the media fixes to linus right away?
>>>
>>> -Kees
>>>
>>
>> I can take both patches. But I think it'd make sense to drop patch 3 first and
>> then, apply all 3.
>>
>> Please, Kees, Hans and Mauro, let me know if this is okay with you.
> 
> I'm fine. If you take it, then I'll drop the media patches from our tree (I merged the
> media patches yesterday).
> 
> Let me know if you take them.

Thanks, Hans. I merged patch 3 yesterday as well, but since patch order matters
in this case, it makes sense to take all of them through the modules tree.

Sorry for the trouble, and thanks Kees, for pointing this out.

> 
> Regards,
> 
> 	Hans

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

[Daniel Gomez]

On 04/11/2025 13.03, Daniel Gomez wrote:
> 
> 
> On 04/11/2025 11.35, Hans Verkuil wrote:
>> On 04/11/2025 07:35, Daniel Gomez wrote:
>>>
>>>
>>> On 04/11/2025 01.13, Kees Cook wrote:
>>>> On Mon, Nov 03, 2025 at 08:49:43PM +0100, Daniel Gomez wrote:
>>>>>
>>>>> On Thu, 09 Oct 2025 20:06:06 -0700, 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!
>>>>>>
>>>>>> [...]
>>>>>
>>>>> Applied patch 3, thanks!
>>>>>
>>>>> [3/3] module: Add compile-time check for embedded NUL characters
>>>>>       commit: 913359754ea821c4d6f6a77e0449b29984099663
>>>>
>>>> I'm nervous about this going in alone -- it breaks allmodconfig builds
>>>> without the media fixes. My intention was to have the media fixes land
>>>> first...
>>>>
>>>> Should I send the media fixes to linus right away?
>>>>
>>>> -Kees
>>>>
>>>
>>> I can take both patches. But I think it'd make sense to drop patch 3 first and
>>> then, apply all 3.
>>>
>>> Please, Kees, Hans and Mauro, let me know if this is okay with you.
>>
>> I'm fine. If you take it, then I'll drop the media patches from our tree (I merged the
>> media patches yesterday).
>>
>> Let me know if you take them.
> 
> Thanks, Hans. I merged patch 3 yesterday as well, but since patch order matters
> in this case, it makes sense to take all of them through the modules tree.
> 
> Sorry for the trouble, and thanks Kees, for pointing this out.

Kees,

FYI, I have dropped patch 3 from modules. My intention is to merge all 3 patches
tomorrow.

I believe Hans has also dropped the patches from the media tree as I do not see
them here: https://git.linuxtv.org/media.git/log/

> 
>>
>> Regards,
>>
>> 	Hans

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

[Hans Verkuil]

On 04/11/2025 21:35, Daniel Gomez wrote:
> 
> 
> On 04/11/2025 13.03, Daniel Gomez wrote:
>>
>>
>> On 04/11/2025 11.35, Hans Verkuil wrote:
>>> On 04/11/2025 07:35, Daniel Gomez wrote:
>>>>
>>>>
>>>> On 04/11/2025 01.13, Kees Cook wrote:
>>>>> On Mon, Nov 03, 2025 at 08:49:43PM +0100, Daniel Gomez wrote:
>>>>>>
>>>>>> On Thu, 09 Oct 2025 20:06:06 -0700, 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!
>>>>>>>
>>>>>>> [...]
>>>>>>
>>>>>> Applied patch 3, thanks!
>>>>>>
>>>>>> [3/3] module: Add compile-time check for embedded NUL characters
>>>>>>       commit: 913359754ea821c4d6f6a77e0449b29984099663
>>>>>
>>>>> I'm nervous about this going in alone -- it breaks allmodconfig builds
>>>>> without the media fixes. My intention was to have the media fixes land
>>>>> first...
>>>>>
>>>>> Should I send the media fixes to linus right away?
>>>>>
>>>>> -Kees
>>>>>
>>>>
>>>> I can take both patches. But I think it'd make sense to drop patch 3 first and
>>>> then, apply all 3.
>>>>
>>>> Please, Kees, Hans and Mauro, let me know if this is okay with you.
>>>
>>> I'm fine. If you take it, then I'll drop the media patches from our tree (I merged the
>>> media patches yesterday).
>>>
>>> Let me know if you take them.
>>
>> Thanks, Hans. I merged patch 3 yesterday as well, but since patch order matters
>> in this case, it makes sense to take all of them through the modules tree.
>>
>> Sorry for the trouble, and thanks Kees, for pointing this out.
> 
> Kees,
> 
> FYI, I have dropped patch 3 from modules. My intention is to merge all 3 patches
> tomorrow.
> 
> I believe Hans has also dropped the patches from the media tree as I do not see
> them here: https://git.linuxtv.org/media.git/log/

Correct, I dropped them. They are all yours :-)

Regards,

	Hans

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

[Daniel Gomez]

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>

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

[Daniel Gomez]

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/

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

[Daniel Gomez]

On Thu, 09 Oct 2025 20:06:06 -0700, 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!
> 
> [...]

Applied, thanks!

[1/3] media: dvb-usb-v2: lmedm04: Fix firmware macro definitions
      (no commit info)
[2/3] media: radio: si470x: Fix DRIVER_AUTHOR macro definition
      (no commit info)
[3/3] module: Add compile-time check for embedded NUL characters
      (no commit info)

Best regards,
-- 
Daniel Gomez <da.gomez@samsung.com>