netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] netfilter: replace -EEXIST with -EBUSY
@ 2025-12-19  5:13 Daniel Gomez
  2025-12-19  7:48 ` Florian Westphal
  0 siblings, 1 reply; 4+ messages in thread
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	[flat|nested] 4+ messages in thread

* Re: [PATCH] netfilter: replace -EEXIST with -EBUSY
  2025-12-19  5:13 [PATCH] netfilter: replace -EEXIST with -EBUSY Daniel Gomez
@ 2025-12-19  7:48 ` Florian Westphal
  2025-12-19 13:39   ` Daniel Gomez
  0 siblings, 1 reply; 4+ messages in thread
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

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	[flat|nested] 4+ messages in thread

* Re: [PATCH] netfilter: replace -EEXIST with -EBUSY
  2025-12-19  7:48 ` Florian Westphal
@ 2025-12-19 13:39   ` Daniel Gomez
  2025-12-20 19:16     ` Florian Westphal
  0 siblings, 1 reply; 4+ messages in thread
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

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	[flat|nested] 4+ messages in thread

* Re: [PATCH] netfilter: replace -EEXIST with -EBUSY
  2025-12-19 13:39   ` Daniel Gomez
@ 2025-12-20 19:16     ` Florian Westphal
  0 siblings, 0 replies; 4+ messages in thread
From: Florian Westphal @ 2025-12-20 19:16 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

Daniel Gomez <da.gomez@kernel.org> wrote:
> something like:
> 
> Replace -EEXIST with -EBUSY to ensure correct error reporting in the module
> initialization path.

Makes sense, I will mangle it locally when I apply this.  Thanks!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-12-20 19:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-19  5:13 [PATCH] netfilter: replace -EEXIST with -EBUSY Daniel Gomez
2025-12-19  7:48 ` Florian Westphal
2025-12-19 13:39   ` Daniel Gomez
2025-12-20 19:16     ` Florian Westphal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).