public inbox for linux-modules@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] target: replace -EEXIST with -EBUSY
From: Daniel Gomez @ 2025-12-20  3:37 UTC (permalink / raw)
  To: Martin K. Petersen, Hannes Reinecke, James E.J. Bottomley
  Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	Aaron Tomlin, Lucas De Marchi, linux-scsi, target-devel,
	linux-modules, linux-kernel, Daniel Gomez
In-Reply-To: <20251220-dev-module-init-eexists-linux-scsi-v1-0-5379db749d54@samsung.com>

From: Daniel Gomez <da.gomez@samsung.com>

The -EEXIST error code is reserved by the module loading infrastructure
to indicate that a module is already loaded. When a module's init
function returns -EEXIST, userspace tools like kmod interpret this as
"module already loaded" and treat the operation as successful, returning
0 to the user even though the module initialization actually failed.

This follows the precedent set by commit 54416fd76770 ("netfilter:
conntrack: helper: Replace -EEXIST by -EBUSY") which fixed the same
issue in nf_conntrack_helper_register()

Affected modules:
  * target_core_file target_core_iblock target_core_pscsi
  * target_core_user

Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
---
 drivers/target/target_core_hba.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/target_core_hba.c b/drivers/target/target_core_hba.c
index d508b343ba7b..dcc11671d919 100644
--- a/drivers/target/target_core_hba.c
+++ b/drivers/target/target_core_hba.c
@@ -50,7 +50,7 @@ int transport_backend_register(const struct target_backend_ops *ops)
 			pr_err("backend %s already registered.\n", ops->name);
 			mutex_unlock(&backend_mutex);
 			kfree(tb);
-			return -EEXIST;
+			return -EBUSY;
 		}
 	}
 	target_setup_backend_cits(tb);

-- 
2.52.0


^ permalink raw reply related

* [PATCH 0/2] scsi: target+fcoe: replace -EEXIST with -EBUSY in module_init() paths
From: Daniel Gomez @ 2025-12-20  3:37 UTC (permalink / raw)
  To: Martin K. Petersen, Hannes Reinecke, James E.J. Bottomley
  Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	Aaron Tomlin, Lucas De Marchi, linux-scsi, target-devel,
	linux-modules, linux-kernel, Daniel Gomez

The error code -EEXIST is reserved by the kernel module loader to
indicate that a module with the same name is already loaded. When a
module's init function returns -EEXIST, kmod interprets this as "module
already loaded" and reports success instead of failure [1].

The kernel module loader will include a safety net that provides -EEXIST
to -EBUSY with a warning [2], and a documentation patch has been sent to
prevent future occurrences [3].

These affected code paths were identified using a static analysis tool
[4] that traces -EEXIST returns to module_init(). The tool was developed
with AI assistance and all findings were manually validated.

Link: https://lore.kernel.org/all/aKEVQhJpRdiZSliu@orbyte.nwl.cc/ [1]
Link: https://lore.kernel.org/all/20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com/ [2]
Link: https://lore.kernel.org/all/20251218-dev-module-init-eexists-modules-docs-v1-0-361569aa782a@samsung.com/ [3]
Link: https://gitlab.com/-/snippets/4913469 [4]

Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
---
Daniel Gomez (2):
      target: replace -EEXIST with -EBUSY
      scsi: fcoe: replace -EEXIST with -EBUSY

 drivers/scsi/fcoe/fcoe_transport.c | 2 +-
 drivers/target/target_core_hba.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251218-dev-module-init-eexists-linux-scsi-4e91a16f7bdd

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


^ permalink raw reply

* Re: [PATCH 3/3] module: Add compile-time check for embedded NUL characters
From: Luck, Tony @ 2025-12-19 16:21 UTC (permalink / raw)
  To: Dan Carpenter, Al Viro
  Cc: Daniel Gomez, Sami Tolvanen, Chris Li, Eric Biggers, Kees Cook,
	Luis Chamberlain, Rusty Russell, Petr Pavlu,
	linux-modules@vger.kernel.org, Malcolm Priestley,
	Mauro Carvalho Chehab, Hans Verkuil, Uwe Kleine-König,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	linux-hardening@vger.kernel.org
In-Reply-To: <aUVI9smf2t7PvDF6@stanley.mountain>

On Fri, Dec 19, 2025 at 03:45:42PM +0300, Dan Carpenter wrote:
> On Fri, Dec 12, 2025 at 02:30:48AM +0900, Daniel Gomez wrote:
> > Maybe the flag fix just needs to be applied to the evaluation? Other op
> > structs do the same. But Dan's patch did not implement evaluate. E.g.:
> > 
> > static struct symbol_op constant_p_op = {
> > 	.evaluate = evaluate_to_int_const_expr,
> > 	.expand = expand_constant_p
> > };
> > 
> 
> I was waiting for you to send this as a patch.  I can do it if you
> need me to.

Dan,

Al Viro thought this was wrong. His alternative patch is here:

https://git.kernel.org/pub/scm/linux/kernel/git/viro/sparse.git/commit/?id=2634e39bf02697a18fece057208150362c985992

-Tony

^ permalink raw reply

* Re: [PATCH v2 3/3] module: Add compile-time check for embedded NUL characters
From: Matthieu Baerts @ 2025-12-19 14:59 UTC (permalink / raw)
  To: Dan Carpenter, Daniel Gomez
  Cc: Kees Cook, Rusty Russell, Petr Pavlu, Sami Tolvanen,
	linux-modules, Hans Verkuil, Malcolm Priestley,
	Mauro Carvalho Chehab, Hans Verkuil, Uwe Kleine-König,
	linux-kernel, linux-media, linux-hardening, Luis Chamberlain,
	Chris Li, linux-sparse
In-Reply-To: <aUVIlvOSvobrdrKV@stanley.mountain>

Hi Dan, Daniel

On 19/12/2025 13:44, Dan Carpenter wrote:
> On Fri, Dec 19, 2025 at 01:29:21PM +0100, Matthieu Baerts wrote:
>> net/mptcp/crypto_test.c:72:1: error: bad integer constant expression
>> net/mptcp/crypto_test.c:72:1: error: static assertion failed: "MODULE_INFO(license, ...) contains embedded NUL byte"
>> net/mptcp/crypto_test.c:73:1: error: bad integer constant expression
>> net/mptcp/crypto_test.c:73:1: error: static assertion failed: "MODULE_INFO(description, ...) contains embedded NUL byte"
> 
> There was a fix for that posted.  Let me ping them to see if anyone is
> planning to send an actual patch.
> 
> https://lore.kernel.org/all/20251211175101.GA3405942@google.com/

Thank you both for your reply! I didn't think about looking at the v1.

I confirm that Sami's patch silences the errors on my side. Thanks!

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply

* Re: [PATCH] netfilter: replace -EEXIST with -EBUSY
From: Daniel Gomez @ 2025-12-19 13:39 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Phil Sutter,
	Nikolay Aleksandrov, Ido Schimmel, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Luis Chamberlain,
	Petr Pavlu, Sami Tolvanen, Aaron Tomlin, Lucas De Marchi,
	netfilter-devel, coreteam, bridge, netdev, linux-modules,
	linux-kernel, Daniel Gomez
In-Reply-To: <aUUDRGqMQ_Ss3bDJ@strlen.de>

On 19/12/2025 08.48, Florian Westphal wrote:
> Daniel Gomez <da.gomez@kernel.org> wrote:
>> From: Daniel Gomez <da.gomez@samsung.com>
>>
>> The -EEXIST error code is reserved by the module loading infrastructure
>> to indicate that a module is already loaded. When a module's init
>> function returns -EEXIST, userspace tools like kmod interpret this as
>> "module already loaded" and treat the operation as successful, returning
>> 0 to the user even though the module initialization actually failed.
>>
>> This follows the precedent set by commit 54416fd76770 ("netfilter:
>> conntrack: helper: Replace -EEXIST by -EBUSY") which fixed the same
>> issue in nf_conntrack_helper_register().
>>
>> Affected modules:
>>   * ebtable_broute ebtable_filter ebtable_nat arptable_filter
>>   * ip6table_filter ip6table_mangle ip6table_nat ip6table_raw
>>   * ip6table_security iptable_filter iptable_mangle iptable_nat
>>   * iptable_raw iptable_security
> 
> But this is very different from what 54416fd76770 fixes.
> 
> Before 54416fd76770. userspace can make a configuration entry that
> prevents and unrelated module from getting loaded but at the same time
> it doesn't provide any error to userspace.
> 
> All these -EEXIST should not be possible unless the module is
> already loaded.

I see.

> I'll apply this patch but its not related to 54416fd76770 afaics.

Thanks.

Then, what about removing that paragraph that mentions the commit and add
something like:

Replace -EEXIST with -EBUSY to ensure correct error reporting in the module
initialization path.

^ permalink raw reply

* Re: [PATCH 3/3] module: Add compile-time check for embedded NUL characters
From: Dan Carpenter @ 2025-12-19 12:45 UTC (permalink / raw)
  To: Daniel Gomez
  Cc: Sami Tolvanen, Luck, Tony, Chris Li, Eric Biggers, Kees Cook,
	Luis Chamberlain, Rusty Russell, Petr Pavlu,
	linux-modules@vger.kernel.org, Malcolm Priestley,
	Mauro Carvalho Chehab, Hans Verkuil, Uwe Kleine-König,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	linux-hardening@vger.kernel.org
In-Reply-To: <083ebd92-4b3f-47f8-bf0f-395a604b5f05@kernel.org>

On Fri, Dec 12, 2025 at 02:30:48AM +0900, Daniel Gomez wrote:
> Maybe the flag fix just needs to be applied to the evaluation? Other op
> structs do the same. But Dan's patch did not implement evaluate. E.g.:
> 
> static struct symbol_op constant_p_op = {
> 	.evaluate = evaluate_to_int_const_expr,
> 	.expand = expand_constant_p
> };
> 

I was waiting for you to send this as a patch.  I can do it if you
need me to.

regards,
dan carpenter


^ permalink raw reply

* Re: [PATCH v2 3/3] module: Add compile-time check for embedded NUL characters
From: Dan Carpenter @ 2025-12-19 12:44 UTC (permalink / raw)
  To: Matthieu Baerts
  Cc: Kees Cook, Rusty Russell, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	linux-modules, Hans Verkuil, Malcolm Priestley,
	Mauro Carvalho Chehab, Hans Verkuil, Uwe Kleine-König,
	linux-kernel, linux-media, linux-hardening, Luis Chamberlain,
	Chris Li, linux-sparse
In-Reply-To: <47a2f0c7-c25f-4734-840b-fdefc2f3c4a9@kernel.org>

On Fri, Dec 19, 2025 at 01:29:21PM +0100, Matthieu Baerts wrote:
> net/mptcp/crypto_test.c:72:1: error: bad integer constant expression
> net/mptcp/crypto_test.c:72:1: error: static assertion failed: "MODULE_INFO(license, ...) contains embedded NUL byte"
> net/mptcp/crypto_test.c:73:1: error: bad integer constant expression
> net/mptcp/crypto_test.c:73:1: error: static assertion failed: "MODULE_INFO(description, ...) contains embedded NUL byte"

There was a fix for that posted.  Let me ping them to see if anyone is
planning to send an actual patch.

https://lore.kernel.org/all/20251211175101.GA3405942@google.com/

regards
dan carpenter


^ permalink raw reply

* Re: [PATCH v2 3/3] module: Add compile-time check for embedded NUL characters
From: Daniel Gomez @ 2025-12-19 12:41 UTC (permalink / raw)
  To: Matthieu Baerts, Kees Cook, Dan Carpenter
  Cc: Rusty Russell, Petr Pavlu, Sami Tolvanen, linux-modules,
	Hans Verkuil, Malcolm Priestley, Mauro Carvalho Chehab,
	Hans Verkuil, Uwe Kleine-König, linux-kernel, linux-media,
	linux-hardening, Luis Chamberlain, Chris Li, linux-sparse
In-Reply-To: <47a2f0c7-c25f-4734-840b-fdefc2f3c4a9@kernel.org>

On 19/12/2025 13.29, Matthieu Baerts wrote:
> $ touch net/mptcp/crypto_test.c && make C=1 net/mptcp/crypto_test.o
>   CC [M]  net/mptcp/crypto_test.o
>   CHECK   net/mptcp/crypto_test.c
> net/mptcp/crypto_test.c:72:1: error: bad integer constant expression
> net/mptcp/crypto_test.c:72:1: error: static assertion failed: "MODULE_INFO(license, ...) contains embedded NUL byte"
> net/mptcp/crypto_test.c:73:1: error: bad integer constant expression
> net/mptcp/crypto_test.c:73:1: error: static assertion failed: "MODULE_INFO(description, ...) contains embedded NUL byte"
> 

FYI, we were discussing the fix here:

https://lore.kernel.org/all/20251211175101.GA3405942@google.com/

Sami provided a fix to the thread that you can test. It'd be good to know if it
works for you too. But we still have some questions as we are not familiar with
the sparse code.

Sami, would you mind sending a patch to the sparse list and then we can ask
questions there?

^ permalink raw reply

* Re: [PATCH v2 3/3] module: Add compile-time check for embedded NUL characters
From: Matthieu Baerts @ 2025-12-19 12:29 UTC (permalink / raw)
  To: Kees Cook, Dan Carpenter
  Cc: Rusty Russell, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	linux-modules, Hans Verkuil, Malcolm Priestley,
	Mauro Carvalho Chehab, Hans Verkuil, Uwe Kleine-König,
	linux-kernel, linux-media, linux-hardening, Luis Chamberlain,
	Chris Li, linux-sparse
In-Reply-To: <20251010030610.3032147-3-kees@kernel.org>

Hi Kees, Dan,

Sorry to react on an oldish patch, but I have a question about it, see
below.

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")
> 
> 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"); \

When checking MPTCP code on top of Linus tree, I get this new warning
with all MPTCP KUnit tests (net/mptcp/*_test.c), e.g.

$ touch net/mptcp/crypto_test.c && make C=1 net/mptcp/crypto_test.o
  CC [M]  net/mptcp/crypto_test.o
  CHECK   net/mptcp/crypto_test.c
net/mptcp/crypto_test.c:72:1: error: bad integer constant expression
net/mptcp/crypto_test.c:72:1: error: static assertion failed: "MODULE_INFO(license, ...) contains embedded NUL byte"
net/mptcp/crypto_test.c:73:1: error: bad integer constant expression
net/mptcp/crypto_test.c:73:1: error: static assertion failed: "MODULE_INFO(description, ...) contains embedded NUL byte"


I'm using Sparse last development version with Dan's commit:

$ sparse --version
v0.6.4-73-gfbdde312

=> fbdde312 ("builtin: implement __builtin_strlen() for constants")


And the two lines causing the warnings don't have "\0":

    72  MODULE_LICENSE("GPL");
    73  MODULE_DESCRIPTION("KUnit tests for MPTCP Crypto");


Am I missing something?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply

* Re: [PATCH] netfilter: replace -EEXIST with -EBUSY
From: Florian Westphal @ 2025-12-19  7:48 UTC (permalink / raw)
  To: Daniel Gomez
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Phil Sutter,
	Nikolay Aleksandrov, Ido Schimmel, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Luis Chamberlain,
	Petr Pavlu, Sami Tolvanen, Aaron Tomlin, Lucas De Marchi,
	netfilter-devel, coreteam, bridge, netdev, linux-modules,
	linux-kernel, Daniel Gomez
In-Reply-To: <20251219-dev-module-init-eexists-netfilter-v1-1-efd3f62412dc@samsung.com>

Daniel Gomez <da.gomez@kernel.org> wrote:
> From: Daniel Gomez <da.gomez@samsung.com>
> 
> The -EEXIST error code is reserved by the module loading infrastructure
> to indicate that a module is already loaded. When a module's init
> function returns -EEXIST, userspace tools like kmod interpret this as
> "module already loaded" and treat the operation as successful, returning
> 0 to the user even though the module initialization actually failed.
>
> This follows the precedent set by commit 54416fd76770 ("netfilter:
> conntrack: helper: Replace -EEXIST by -EBUSY") which fixed the same
> issue in nf_conntrack_helper_register().
> 
> Affected modules:
>   * ebtable_broute ebtable_filter ebtable_nat arptable_filter
>   * ip6table_filter ip6table_mangle ip6table_nat ip6table_raw
>   * ip6table_security iptable_filter iptable_mangle iptable_nat
>   * iptable_raw iptable_security

But this is very different from what 54416fd76770 fixes.

Before 54416fd76770. userspace can make a configuration entry that
prevents and unrelated module from getting loaded but at the same time
it doesn't provide any error to userspace.

All these -EEXIST should not be possible unless the module is
already loaded.

> diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
> index 5697e3949a36..a04fc1757528 100644
> --- a/net/bridge/netfilter/ebtables.c
> +++ b/net/bridge/netfilter/ebtables.c
> @@ -1299,7 +1299,7 @@ int ebt_register_template(const struct ebt_table *t, int (*table_init)(struct ne
>  	list_for_each_entry(tmpl, &template_tables, list) {
>  		if (WARN_ON_ONCE(strcmp(t->name, tmpl->name) == 0)) {
>  			mutex_unlock(&ebt_mutex);
> -			return -EEXIST;
> +			return -EBUSY;

As you can see from the WARN_ON, this cannot happen unless someone adds a new ebt kernel
table module that tries to register the same name.

> diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
> index 74cef8bf554c..62cf6a30875e 100644
> --- a/net/netfilter/nf_log.c
> +++ b/net/netfilter/nf_log.c
> @@ -89,7 +89,7 @@ int nf_log_register(u_int8_t pf, struct nf_logger *logger)
>  	if (pf == NFPROTO_UNSPEC) {
>  		for (i = NFPROTO_UNSPEC; i < NFPROTO_NUMPROTO; i++) {
>  			if (rcu_access_pointer(loggers[i][logger->type])) {
> -				ret = -EEXIST;
> +				ret = -EBUSY;
>  				goto unlock;

I don't see how this can happen, unless someone adds a new kernel module
that claims the same type as an existing kernel module.

> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index 90b7630421c4..48105ea3df15 100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -1764,7 +1764,7 @@ EXPORT_SYMBOL_GPL(xt_hook_ops_alloc);
>  int xt_register_template(const struct xt_table *table,
>  			 int (*table_init)(struct net *net))
>  {
> -	int ret = -EEXIST, af = table->af;
> +	int ret = -EBUSY, af = table->af;
>  	struct xt_template *t;

Same, this requires someone adding a new kernel module with clashing
name.

I'll apply this patch but its not related to 54416fd76770 afaics.

^ permalink raw reply

* [PATCH] netfilter: replace -EEXIST with -EBUSY
From: Daniel Gomez @ 2025-12-19  5:13 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	Phil Sutter, Nikolay Aleksandrov, Ido Schimmel, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman
  Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	Aaron Tomlin, Lucas De Marchi, netfilter-devel, coreteam, bridge,
	netdev, linux-modules, linux-kernel, Daniel Gomez

From: Daniel Gomez <da.gomez@samsung.com>

The -EEXIST error code is reserved by the module loading infrastructure
to indicate that a module is already loaded. When a module's init
function returns -EEXIST, userspace tools like kmod interpret this as
"module already loaded" and treat the operation as successful, returning
0 to the user even though the module initialization actually failed.

This follows the precedent set by commit 54416fd76770 ("netfilter:
conntrack: helper: Replace -EEXIST by -EBUSY") which fixed the same
issue in nf_conntrack_helper_register().

Affected modules:
  * ebtable_broute ebtable_filter ebtable_nat arptable_filter
  * ip6table_filter ip6table_mangle ip6table_nat ip6table_raw
  * ip6table_security iptable_filter iptable_mangle iptable_nat
  * iptable_raw iptable_security

Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
---
The error code -EEXIST is reserved by the kernel module loader to
indicate that a module with the same name is already loaded. When a
module's init function returns -EEXIST, kmod interprets this as "module
already loaded" and reports success instead of failure [1].

The kernel module loader will include a safety net that provides -EEXIST
to -EBUSY with a warning [2], and a documentation patch has been sent to
prevent future occurrences [3].

These affected code paths were identified using a static analysis tool
[4] that traces -EEXIST returns to module_init(). The tool was developed
with AI assistance and all findings were manually validated.

Link: https://lore.kernel.org/all/aKEVQhJpRdiZSliu@orbyte.nwl.cc/ [1]
Link: https://lore.kernel.org/all/20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com/ [2]
Link: https://lore.kernel.org/all/20251218-dev-module-init-eexists-modules-docs-v1-0-361569aa782a@samsung.com/ [3]
Link: https://gitlab.com/-/snippets/4913469 [4]
---
 net/bridge/netfilter/ebtables.c | 2 +-
 net/netfilter/nf_log.c          | 4 ++--
 net/netfilter/x_tables.c        | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 5697e3949a36..a04fc1757528 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1299,7 +1299,7 @@ int ebt_register_template(const struct ebt_table *t, int (*table_init)(struct ne
 	list_for_each_entry(tmpl, &template_tables, list) {
 		if (WARN_ON_ONCE(strcmp(t->name, tmpl->name) == 0)) {
 			mutex_unlock(&ebt_mutex);
-			return -EEXIST;
+			return -EBUSY;
 		}
 	}
 
diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
index 74cef8bf554c..62cf6a30875e 100644
--- a/net/netfilter/nf_log.c
+++ b/net/netfilter/nf_log.c
@@ -89,7 +89,7 @@ int nf_log_register(u_int8_t pf, struct nf_logger *logger)
 	if (pf == NFPROTO_UNSPEC) {
 		for (i = NFPROTO_UNSPEC; i < NFPROTO_NUMPROTO; i++) {
 			if (rcu_access_pointer(loggers[i][logger->type])) {
-				ret = -EEXIST;
+				ret = -EBUSY;
 				goto unlock;
 			}
 		}
@@ -97,7 +97,7 @@ int nf_log_register(u_int8_t pf, struct nf_logger *logger)
 			rcu_assign_pointer(loggers[i][logger->type], logger);
 	} else {
 		if (rcu_access_pointer(loggers[pf][logger->type])) {
-			ret = -EEXIST;
+			ret = -EBUSY;
 			goto unlock;
 		}
 		rcu_assign_pointer(loggers[pf][logger->type], logger);
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 90b7630421c4..48105ea3df15 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -1764,7 +1764,7 @@ EXPORT_SYMBOL_GPL(xt_hook_ops_alloc);
 int xt_register_template(const struct xt_table *table,
 			 int (*table_init)(struct net *net))
 {
-	int ret = -EEXIST, af = table->af;
+	int ret = -EBUSY, af = table->af;
 	struct xt_template *t;
 
 	mutex_lock(&xt[af].mutex);

---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251218-dev-module-init-eexists-netfilter-56f4fb352dcb

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


^ permalink raw reply related

* Re: [PATCH v3 4/4] tracing: move tracing declarations from kernel.h to a dedicated header
From: Yury Norov @ 2025-12-18 21:53 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Randy Dunlap, Andrew Morton, Masami Hiramatsu, Mathieu Desnoyers,
	Andy Shevchenko, Christophe Leroy, Ingo Molnar, Jani Nikula,
	Joonas Lahtinen, David Laight, Petr Pavlu, Andi Shyti,
	Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-kernel, intel-gfx,
	dri-devel, linux-modules, linux-trace-kernel, Kees Cook
In-Reply-To: <20251218164103.3c535de3@gandalf.local.home>

On Thu, Dec 18, 2025 at 04:41:03PM -0500, Steven Rostedt wrote:
> On Thu, 18 Dec 2025 16:25:42 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Thu, 18 Dec 2025 15:33:47 -0500
> > Yury Norov <yury.norov@gmail.com> wrote:
> > 
> > > > I don't actually remember why I had __trace_puts() pass in the size. I
> > > > could change it to:    
> > > 
> > > This is the best approach. I'll schedule it for v4. Would you like me to
> > > take it as-is, or you'd send a patch?
> > >    
> > 
> > Let me send an official patch.
> > 
> 
> You can find it here (I Cc'd you too). Feel free to add it to your patch set.
> 
>   https://lore.kernel.org/all/20251218163739.5605f9ea@gandalf.local.home/

Thanks, will do.

^ permalink raw reply

* Re: [PATCH v3 4/4] tracing: move tracing declarations from kernel.h to a dedicated header
From: Steven Rostedt @ 2025-12-18 21:41 UTC (permalink / raw)
  To: Yury Norov
  Cc: Randy Dunlap, Andrew Morton, Masami Hiramatsu, Mathieu Desnoyers,
	Andy Shevchenko, Christophe Leroy, Ingo Molnar, Jani Nikula,
	Joonas Lahtinen, David Laight, Petr Pavlu, Andi Shyti,
	Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-kernel, intel-gfx,
	dri-devel, linux-modules, linux-trace-kernel, Kees Cook
In-Reply-To: <20251218162542.476009db@gandalf.local.home>

On Thu, 18 Dec 2025 16:25:42 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 18 Dec 2025 15:33:47 -0500
> Yury Norov <yury.norov@gmail.com> wrote:
> 
> > > I don't actually remember why I had __trace_puts() pass in the size. I
> > > could change it to:    
> > 
> > This is the best approach. I'll schedule it for v4. Would you like me to
> > take it as-is, or you'd send a patch?
> >    
> 
> Let me send an official patch.
> 

You can find it here (I Cc'd you too). Feel free to add it to your patch set.

  https://lore.kernel.org/all/20251218163739.5605f9ea@gandalf.local.home/

-- Steve

^ permalink raw reply

* Re: [PATCH v3 4/4] tracing: move tracing declarations from kernel.h to a dedicated header
From: Steven Rostedt @ 2025-12-18 21:25 UTC (permalink / raw)
  To: Yury Norov
  Cc: Randy Dunlap, Andrew Morton, Masami Hiramatsu, Mathieu Desnoyers,
	Andy Shevchenko, Christophe Leroy, Ingo Molnar, Jani Nikula,
	Joonas Lahtinen, David Laight, Petr Pavlu, Andi Shyti,
	Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-kernel, intel-gfx,
	dri-devel, linux-modules, linux-trace-kernel, Kees Cook
In-Reply-To: <aURlK1gpCrfLEKN9@yury>

On Thu, 18 Dec 2025 15:33:47 -0500
Yury Norov <yury.norov@gmail.com> wrote:

> > I don't actually remember why I had __trace_puts() pass in the size. I
> > could change it to:  
> 
> This is the best approach. I'll schedule it for v4. Would you like me to
> take it as-is, or you'd send a patch?
>  

Let me send an official patch.

-- Steve

^ permalink raw reply

* [PATCH 2/2] docs: hacking: clarify reserved -EEXIST in module_init()
From: Daniel Gomez @ 2025-12-18 20:59 UTC (permalink / raw)
  To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	Aaron Tomlin, Jonathan Corbet, Lucas De Marchi
  Cc: linux-modules, linux-kernel, linux-doc, Daniel Gomez
In-Reply-To: <20251218-dev-module-init-eexists-modules-docs-v1-0-361569aa782a@samsung.com>

From: Daniel Gomez <da.gomez@samsung.com>

The error code -EEXIST is reserved by the kernel module loader to
indicate that a module with the same name is already loaded. Add
a warning note to clarify what these means for module authors and
maintainers to ensure the module_init() path return error do not
conflict with the reserved one.

Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
---
 Documentation/kernel-hacking/hacking.rst | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/kernel-hacking/hacking.rst b/Documentation/kernel-hacking/hacking.rst
index 0042776a9e17..2c929ab93143 100644
--- a/Documentation/kernel-hacking/hacking.rst
+++ b/Documentation/kernel-hacking/hacking.rst
@@ -459,6 +459,13 @@ to fail (unfortunately, this has no effect if the module is compiled
 into the kernel). This function is called in user context with
 interrupts enabled, so it can sleep.
 
+.. warning::
+
+    The error code ``-EEXIST`` is reserved by the module loader to
+    indicate a module is already loaded. kmod interprets this as success,
+    so ``module_init()`` must never return ``-EEXIST``. Use ``-EBUSY`` or
+    ``-EALREADY`` instead.
+
 :c:func:`module_exit()`
 -----------------------
 

-- 
2.52.0


^ permalink raw reply related

* [PATCH 1/2] module: add -EEXIST documentation
From: Daniel Gomez @ 2025-12-18 20:59 UTC (permalink / raw)
  To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	Aaron Tomlin, Jonathan Corbet, Lucas De Marchi
  Cc: linux-modules, linux-kernel, linux-doc, Daniel Gomez
In-Reply-To: <20251218-dev-module-init-eexists-modules-docs-v1-0-361569aa782a@samsung.com>

From: Daniel Gomez <da.gomez@samsung.com>

The error code -EEXIST is reserved by the kernel module loader to
indicate that a module with the same name is already loaded. Add a
comment to clarify what this means for module authors and maintainers
to ensure the module_init() path return error do not conflict with the
reserved one.

Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
---
 kernel/module/main.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index a8394d81174f..655a780981d3 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3038,6 +3038,14 @@ static noinline int do_init_module(struct module *mod)
 	if (mod->init != NULL)
 		ret = do_one_initcall(mod->init);
 	if (ret < 0) {
+		/*
+		 * -EEXIST is reserved by the module loader to mean "already loaded". kmod
+		 * interprets this as success, hiding real module failures. Override with
+		 * -EBUSY and warn.
+		 *
+		 * Module authors: use -EBUSY or -EALREADY instead of -EEXIST.
+		 * See Documentation/kernel-hacking/hacking.rst
+		 */
 		if (ret == -EEXIST) {
 			pr_warn("%s: init suspiciously returned -EEXIST: Overriding with -EBUSY\n",
 				mod->name);

-- 
2.52.0


^ permalink raw reply related

* [PATCH 0/2] module: add -EEXIST module_init() reservation docs
From: Daniel Gomez @ 2025-12-18 20:59 UTC (permalink / raw)
  To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	Aaron Tomlin, Jonathan Corbet, Lucas De Marchi
  Cc: linux-modules, linux-kernel, linux-doc, Daniel Gomez

This series documents the -EEXIST error code reservation in the module
loader, building on Lucas's patches [1] that add the runtime safety net.

When module_init() returns -EEXIST, kmod interprets this as "module
already loaded" and reports success, hiding real init failures. Lucas's
patches warn and override this to -EBUSY. These patches document this
convention to prevent future cases.

This was originally reported in this thread [2].

Link: https://lore.kernel.org/all/20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com/ [1]
Link: https://lore.kernel.org/all/aKEVQhJpRdiZSliu@orbyte.nwl.cc/#t [2]

Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
---
Daniel Gomez (2):
      module: add -EEXIST documentation
      docs: hacking: clarify reserved -EEXIST in module_init()

 Documentation/kernel-hacking/hacking.rst | 7 +++++++
 kernel/module/main.c                     | 8 ++++++++
 2 files changed, 15 insertions(+)
---
base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
change-id: 20251218-dev-module-init-eexists-modules-docs-1dc7cb7a96bb
prerequisite-change-id: 20251013-module-warn-ret-59f085298055:v1
prerequisite-patch-id: c3e4f5b5d01c2b48b4c94e51a60469cb74691853
prerequisite-patch-id: 2d5a726a75f3b9d9c256b8478fb6115a92f04354

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


^ permalink raw reply

* Re: [PATCH v3 4/4] tracing: move tracing declarations from kernel.h to a dedicated header
From: Yury Norov @ 2025-12-18 20:52 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Steven Rostedt, Andrew Morton, Masami Hiramatsu,
	Mathieu Desnoyers, Andy Shevchenko, Christophe Leroy, Ingo Molnar,
	Jani Nikula, Joonas Lahtinen, David Laight, Petr Pavlu,
	Andi Shyti, Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	linux-kernel, intel-gfx, dri-devel, linux-modules,
	linux-trace-kernel, Kees Cook
In-Reply-To: <95f0c65b-15ff-45db-9845-579f7adf4c86@infradead.org>

On Thu, Dec 18, 2025 at 10:34:07AM -0800, Randy Dunlap wrote:
> 
> 
> On 12/18/25 9:33 AM, Steven Rostedt wrote:
> > On Wed, 17 Dec 2025 22:59:33 -0500
> > Yury Norov <yury.norov@gmail.com> wrote:
> > 
> >> I deem to drop trace_printk.h from kernel.h - it is more aligned with
> >> the idea of unloading the header. The original motivation to keep
> >> trace_printk.h in kernel.h was just because a similar printk.h is living
> >> there. But after all, this is a purely debugging header, so no need for
> >> almost every C file to bear debugging stuff.
> > 
> > It is a big deal for debugging stuff. A lot of developers debug their code
> > with trace_printk(), and do the "shotgun approach", where they cut and
> > paste trace_printk()s all over their code in several files. Having to now add:
> > 
> >   #include <linux/trace_printk.h>
> > 
> > whenever a trace_printk() is added is going to be a big PITA and slow down
> > all debugging efforts.
> 
> Eh? Maybe a PITA, but surely not a big one.
> Slow down "all debugging efforts?"
> Please cut down on the hyperbole.

For me, removing trace_prink.h saves 1.5-2% of compile time:

Before:                         
#1 real	5m12.602s               
#2 real	5m11.333s               

After:
#1 real	5m6.190s
#2 real	5m7.151s

I'm building ubuntu-derived localyesconfig with a couple extra drivers.
Steven, if you're not absolutely against, lets drop the trace_printk.h?

Thanks,
Yury

^ permalink raw reply

* Re: [PATCH v3 4/4] tracing: move tracing declarations from kernel.h to a dedicated header
From: Yury Norov @ 2025-12-18 20:33 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Randy Dunlap, Andrew Morton, Masami Hiramatsu, Mathieu Desnoyers,
	Andy Shevchenko, Christophe Leroy, Ingo Molnar, Jani Nikula,
	Joonas Lahtinen, David Laight, Petr Pavlu, Andi Shyti,
	Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-kernel, intel-gfx,
	dri-devel, linux-modules, linux-trace-kernel, Kees Cook
In-Reply-To: <20251218124326.22334325@gandalf.local.home>

On Thu, Dec 18, 2025 at 12:43:26PM -0500, Steven Rostedt wrote:
> On Thu, 18 Dec 2025 12:33:49 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Wed, 17 Dec 2025 22:59:33 -0500
> > Yury Norov <yury.norov@gmail.com> wrote:
> > 
> > > I deem to drop trace_printk.h from kernel.h - it is more aligned with
> > > the idea of unloading the header. The original motivation to keep
> > > trace_printk.h in kernel.h was just because a similar printk.h is living
> > > there. But after all, this is a purely debugging header, so no need for
> > > almost every C file to bear debugging stuff.  
> > 
> > It is a big deal for debugging stuff. A lot of developers debug their code
> > with trace_printk(), and do the "shotgun approach", where they cut and
> > paste trace_printk()s all over their code in several files. Having to now add:
> > 
> >   #include <linux/trace_printk.h>
> > 
> > whenever a trace_printk() is added is going to be a big PITA and slow down
> > all debugging efforts.
> >
> 
> I don't actually remember why I had __trace_puts() pass in the size. I
> could change it to:

This is the best approach. I'll schedule it for v4. Would you like me to
take it as-is, or you'd send a patch?
 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 5b46924fdff5..d5a939b8c391 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -331,10 +331,10 @@ int __trace_printk(unsigned long ip, const char *fmt, ...);
>  	if (__builtin_constant_p(str))					\
>  		__trace_bputs(_THIS_IP_, trace_printk_fmt);		\
>  	else								\
> -		__trace_puts(_THIS_IP_, str, strlen(str));		\
> +		__trace_puts(_THIS_IP_, str);				\
>  })
>  extern int __trace_bputs(unsigned long ip, const char *str);
> -extern int __trace_puts(unsigned long ip, const char *str, int size);
> +extern int __trace_puts(unsigned long ip, const char *str);
>  
>  extern void trace_dump_stack(int skip);
>  
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index e575956ef9b5..686741edb803 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -1178,11 +1178,10 @@ EXPORT_SYMBOL_GPL(__trace_array_puts);
>   * __trace_puts - write a constant string into the trace buffer.
>   * @ip:	   The address of the caller
>   * @str:   The constant string to write
> - * @size:  The size of the string.
>   */
> -int __trace_puts(unsigned long ip, const char *str, int size)
> +int __trace_puts(unsigned long ip, const char *str)
>  {
> -	return __trace_array_puts(printk_trace, ip, str, size);
> +	return __trace_array_puts(printk_trace, ip, str, strlen(str));
>  }
>  EXPORT_SYMBOL_GPL(__trace_puts);
>  
> @@ -1201,7 +1200,7 @@ int __trace_bputs(unsigned long ip, const char *str)
>  	int size = sizeof(struct bputs_entry);
>  
>  	if (!printk_binsafe(tr))
> -		return __trace_puts(ip, str, strlen(str));
> +		return __trace_puts(ip, str);
>  
>  	if (!(tr->trace_flags & TRACE_ITER(PRINTK)))
>  		return 0;
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index b6d42fe06115..de4e6713b84e 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -2116,7 +2116,7 @@ extern void tracing_log_err(struct trace_array *tr,
>   * about performance). The internal_trace_puts() is for such
>   * a purpose.
>   */
> -#define internal_trace_puts(str) __trace_puts(_THIS_IP_, str, strlen(str))
> +#define internal_trace_puts(str) __trace_puts(_THIS_IP_, str)
>  
>  #undef FTRACE_ENTRY
>  #define FTRACE_ENTRY(call, struct_name, id, tstruct, print)	\
> 
> 
> 
> Which removes the strlen() altogether.
> 
> -- Steve

^ permalink raw reply

* Re: [PATCH v4 0/2] lib/crypto: ML-DSA verification support
From: Eric Biggers @ 2025-12-18 19:08 UTC (permalink / raw)
  To: linux-crypto
  Cc: David Howells, Herbert Xu, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Jason A . Donenfeld, Ard Biesheuvel,
	Stephan Mueller, Lukas Wunner, Ignat Korchagin, keyrings,
	linux-modules, linux-kernel
In-Reply-To: <20251214181712.29132-1-ebiggers@kernel.org>

On Sun, Dec 14, 2025 at 10:17:10AM -0800, Eric Biggers wrote:
> This series can also be retrieved from:
> 
>     git fetch https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git mldsa-v4
> 
> This series adds support for verifying ML-DSA signatures to lib/crypto/.
> Patch 1 is the ML-DSA implementation itself.  See that for full details.
> Patch 2 adds the KUnit test suite.
> 
> The initial use case for this will be kernel module signature
> verification.  For more details, see David Howells' patchset
> https://lore.kernel.org/linux-crypto/20251120104439.2620205-1-dhowells@redhat.com/
> 
> Note: I'm planning to apply this to libcrypto-next for 6.20.

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-next

- Eric

^ permalink raw reply

* Re: [PATCH v3 4/4] tracing: move tracing declarations from kernel.h to a dedicated header
From: Randy Dunlap @ 2025-12-18 18:34 UTC (permalink / raw)
  To: Steven Rostedt, Yury Norov
  Cc: Andrew Morton, Masami Hiramatsu, Mathieu Desnoyers,
	Andy Shevchenko, Christophe Leroy, Ingo Molnar, Jani Nikula,
	Joonas Lahtinen, David Laight, Petr Pavlu, Andi Shyti,
	Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-kernel, intel-gfx,
	dri-devel, linux-modules, linux-trace-kernel, Kees Cook
In-Reply-To: <20251218123349.35339242@gandalf.local.home>



On 12/18/25 9:33 AM, Steven Rostedt wrote:
> On Wed, 17 Dec 2025 22:59:33 -0500
> Yury Norov <yury.norov@gmail.com> wrote:
> 
>> I deem to drop trace_printk.h from kernel.h - it is more aligned with
>> the idea of unloading the header. The original motivation to keep
>> trace_printk.h in kernel.h was just because a similar printk.h is living
>> there. But after all, this is a purely debugging header, so no need for
>> almost every C file to bear debugging stuff.
> 
> It is a big deal for debugging stuff. A lot of developers debug their code
> with trace_printk(), and do the "shotgun approach", where they cut and
> paste trace_printk()s all over their code in several files. Having to now add:
> 
>   #include <linux/trace_printk.h>
> 
> whenever a trace_printk() is added is going to be a big PITA and slow down
> all debugging efforts.

Eh? Maybe a PITA, but surely not a big one.
Slow down "all debugging efforts?"
Please cut down on the hyperbole.

-- 
~Randy


^ permalink raw reply

* Re: [PATCH v3 4/4] tracing: move tracing declarations from kernel.h to a dedicated header
From: Steven Rostedt @ 2025-12-18 17:43 UTC (permalink / raw)
  To: Yury Norov
  Cc: Randy Dunlap, Andrew Morton, Masami Hiramatsu, Mathieu Desnoyers,
	Andy Shevchenko, Christophe Leroy, Ingo Molnar, Jani Nikula,
	Joonas Lahtinen, David Laight, Petr Pavlu, Andi Shyti,
	Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-kernel, intel-gfx,
	dri-devel, linux-modules, linux-trace-kernel, Kees Cook
In-Reply-To: <20251218123349.35339242@gandalf.local.home>

On Thu, 18 Dec 2025 12:33:49 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 17 Dec 2025 22:59:33 -0500
> Yury Norov <yury.norov@gmail.com> wrote:
> 
> > I deem to drop trace_printk.h from kernel.h - it is more aligned with
> > the idea of unloading the header. The original motivation to keep
> > trace_printk.h in kernel.h was just because a similar printk.h is living
> > there. But after all, this is a purely debugging header, so no need for
> > almost every C file to bear debugging stuff.  
> 
> It is a big deal for debugging stuff. A lot of developers debug their code
> with trace_printk(), and do the "shotgun approach", where they cut and
> paste trace_printk()s all over their code in several files. Having to now add:
> 
>   #include <linux/trace_printk.h>
> 
> whenever a trace_printk() is added is going to be a big PITA and slow down
> all debugging efforts.
>

I don't actually remember why I had __trace_puts() pass in the size. I
could change it to:

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 5b46924fdff5..d5a939b8c391 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -331,10 +331,10 @@ int __trace_printk(unsigned long ip, const char *fmt, ...);
 	if (__builtin_constant_p(str))					\
 		__trace_bputs(_THIS_IP_, trace_printk_fmt);		\
 	else								\
-		__trace_puts(_THIS_IP_, str, strlen(str));		\
+		__trace_puts(_THIS_IP_, str);				\
 })
 extern int __trace_bputs(unsigned long ip, const char *str);
-extern int __trace_puts(unsigned long ip, const char *str, int size);
+extern int __trace_puts(unsigned long ip, const char *str);
 
 extern void trace_dump_stack(int skip);
 
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index e575956ef9b5..686741edb803 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1178,11 +1178,10 @@ EXPORT_SYMBOL_GPL(__trace_array_puts);
  * __trace_puts - write a constant string into the trace buffer.
  * @ip:	   The address of the caller
  * @str:   The constant string to write
- * @size:  The size of the string.
  */
-int __trace_puts(unsigned long ip, const char *str, int size)
+int __trace_puts(unsigned long ip, const char *str)
 {
-	return __trace_array_puts(printk_trace, ip, str, size);
+	return __trace_array_puts(printk_trace, ip, str, strlen(str));
 }
 EXPORT_SYMBOL_GPL(__trace_puts);
 
@@ -1201,7 +1200,7 @@ int __trace_bputs(unsigned long ip, const char *str)
 	int size = sizeof(struct bputs_entry);
 
 	if (!printk_binsafe(tr))
-		return __trace_puts(ip, str, strlen(str));
+		return __trace_puts(ip, str);
 
 	if (!(tr->trace_flags & TRACE_ITER(PRINTK)))
 		return 0;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index b6d42fe06115..de4e6713b84e 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -2116,7 +2116,7 @@ extern void tracing_log_err(struct trace_array *tr,
  * about performance). The internal_trace_puts() is for such
  * a purpose.
  */
-#define internal_trace_puts(str) __trace_puts(_THIS_IP_, str, strlen(str))
+#define internal_trace_puts(str) __trace_puts(_THIS_IP_, str)
 
 #undef FTRACE_ENTRY
 #define FTRACE_ENTRY(call, struct_name, id, tstruct, print)	\



Which removes the strlen() altogether.

-- Steve

^ permalink raw reply related

* Re: [PATCH v3 4/4] tracing: move tracing declarations from kernel.h to a dedicated header
From: Steven Rostedt @ 2025-12-18 17:33 UTC (permalink / raw)
  To: Yury Norov
  Cc: Randy Dunlap, Andrew Morton, Masami Hiramatsu, Mathieu Desnoyers,
	Andy Shevchenko, Christophe Leroy, Ingo Molnar, Jani Nikula,
	Joonas Lahtinen, David Laight, Petr Pavlu, Andi Shyti,
	Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-kernel, intel-gfx,
	dri-devel, linux-modules, linux-trace-kernel, Kees Cook
In-Reply-To: <aUN8Hm377C5A0ILX@yury>

On Wed, 17 Dec 2025 22:59:33 -0500
Yury Norov <yury.norov@gmail.com> wrote:

> I deem to drop trace_printk.h from kernel.h - it is more aligned with
> the idea of unloading the header. The original motivation to keep
> trace_printk.h in kernel.h was just because a similar printk.h is living
> there. But after all, this is a purely debugging header, so no need for
> almost every C file to bear debugging stuff.

It is a big deal for debugging stuff. A lot of developers debug their code
with trace_printk(), and do the "shotgun approach", where they cut and
paste trace_printk()s all over their code in several files. Having to now add:

  #include <linux/trace_printk.h>

whenever a trace_printk() is added is going to be a big PITA and slow down
all debugging efforts.

-- Steve

^ permalink raw reply

* Re: [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid
From: Petr Mladek @ 2025-12-18  9:09 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Kees Cook,
	Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
	Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
	linux-kernel, bpf, linux-modules, linux-trace-kernel
In-Reply-To: <20251217130904.33163c243172324a5308efe9@linux-foundation.org>

On Wed 2025-12-17 13:09:04, Andrew Morton wrote:
> On Fri, 28 Nov 2025 14:59:13 +0100 Petr Mladek <pmladek@suse.com> wrote:
> 
> > This patchset is cleaning up kallsyms code related to module buildid.
> > It is fixing an invalid access when printing backtraces, see [v1] for
> > more details:
> > 
> > ...
> >
> > [v1] https://lore.kernel.org/r/20251105142319.1139183-1-pmladek@suse.com
> > [v2] https://lore.kernel.org/r/20251112142003.182062-1-pmladek@suse.com
> > 
> 
> It's best to avoid sending people off to the WWW to understand a
> patchset - better that the git history be self-contained.

I see. I'll do better next time.

> So when
> staging this for mm.git I scooped the relevant material from [1] and
> added it to your cover letter, as below.  Looks OK?

It looks OK to me. Thanks for taking the patchset.

Best Regards,
Petr

^ permalink raw reply

* Re: [PATCH v3 4/4] tracing: move tracing declarations from kernel.h to a dedicated header
From: Randy Dunlap @ 2025-12-18  5:59 UTC (permalink / raw)
  To: Yury Norov
  Cc: Andrew Morton, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, Andy Shevchenko, Christophe Leroy, Ingo Molnar,
	Jani Nikula, Joonas Lahtinen, David Laight, Petr Pavlu,
	Andi Shyti, Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	linux-kernel, intel-gfx, dri-devel, linux-modules,
	linux-trace-kernel, Kees Cook
In-Reply-To: <aUN8Hm377C5A0ILX@yury>



On 12/17/25 7:59 PM, Yury Norov wrote:
> On Tue, Dec 16, 2025 at 09:24:55PM -0800, Randy Dunlap wrote:
>> [adding Kees]
>>
>> On 12/16/25 4:13 PM, Andrew Morton wrote:
>>> On Fri,  5 Dec 2025 12:52:35 -0500 "Yury Norov (NVIDIA)" <yury.norov@gmail.com> wrote:
>>>
>>>> Tracing is a half of the kernel.h in terms of LOCs, although it's
>>>> a self-consistent part. It is intended for quick debugging purposes
>>>> and isn't used by the normal tracing utilities.
>>>>
>>>> Move it to a separate header. If someone needs to just throw a
>>>> trace_printk() in their driver, they will not have to pull all
>>>> the heavy tracing machinery.
>>>>
>>>> This is a pure move, except for removing a few 'extern's.
>>>>
>>
>> Hm, for a pure move, this shouldn't be necessary. Anyway, not using
>> FORTIFY in purgatory.o fixes this build error.
>> Or maybe there's a better answer.
>>
>> ---
>>  arch/x86/purgatory/Makefile |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- a/arch/x86/purgatory/Makefile
>> +++ b/arch/x86/purgatory/Makefile
>> @@ -62,7 +62,7 @@ PURGATORY_CFLAGS_REMOVE		+= $(CC_FLAGS_C
>>  endif
>>  
>>  CFLAGS_REMOVE_purgatory.o	+= $(PURGATORY_CFLAGS_REMOVE)
>> -CFLAGS_purgatory.o		+= $(PURGATORY_CFLAGS)
>> +CFLAGS_purgatory.o		+= $(PURGATORY_CFLAGS) -D__NO_FORTIFY
>>  
>>  CFLAGS_REMOVE_sha256.o		+= $(PURGATORY_CFLAGS_REMOVE)
>>  CFLAGS_sha256.o			+= $(PURGATORY_CFLAGS)
> 
> That happened because the new trace_printk.h includes string.h for
> strlen(), so all kernel.h users now indirectly include it, and it
> causes, seemingly, a circular dependency if FORTIFY is enabled.
> 
> A fix would be dropping trace_printk.h from kernel.h, or switching the
> only user of string.h, trace_puts(), to __builtin_strlen().
> 
> Notice, Andy has concerned about this on the previous round, and also
> suggested __builtin_strlen():
> 
>         https://lkml.org/lkml/2025/12/3/910
> 
> I deem to drop trace_printk.h from kernel.h - it is more aligned with
> the idea of unloading the header. The original motivation to keep
> trace_printk.h in kernel.h was just because a similar printk.h is living
> there. But after all, this is a purely debugging header, so no need for
> almost every C file to bear debugging stuff.
> 
> I can actually do both - switch to an intrinsic and drop the header.
> 
> Guys, please let me know what do you thing.


There are some problems with using __builtin_mem{cpy,set} -- don't
know about __builtin_str{whatever}. See

commit 4ce97317f41d
Author: Nick Desaulniers <nick.desaulniers+lkml@gmail.com>
Date:   Wed Aug 7 15:15:32 2019 -0700
    x86/purgatory: Do not use __builtin_memcpy and __builtin_memset


We should drop the header from kernel.h soon anyway, whether now or
in a few weeks/months. IMHO.

-- 
~Randy


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox