[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2025122212-fiction-setback-ede5@gregkh>
Date: Mon, 22 Dec 2025 12:56:38 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Daniel Gomez <da.gomez@...nel.org>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Daniel Scally <djrscally@...il.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Danilo Krummrich <dakr@...nel.org>,
Luis Chamberlain <mcgrof@...nel.org>,
Petr Pavlu <petr.pavlu@...e.com>,
Sami Tolvanen <samitolvanen@...gle.com>,
Aaron Tomlin <atomlin@...mlin.com>,
Lucas De Marchi <demarchi@...nel.org>, linux-acpi@...r.kernel.org,
linux-modules@...r.kernel.org, linux-kernel@...r.kernel.org,
Daniel Gomez <da.gomez@...sung.com>
Subject: Re: [PATCH] software node: replace -EEXIST with -EBUSY
On Mon, Dec 22, 2025 at 09:48:54AM +0100, Daniel Gomez wrote:
> On 22/12/2025 09.19, Greg Kroah-Hartman wrote:
> > On Sat, Dec 20, 2025 at 04:55:00AM +0100, Daniel Gomez wrote:
> >> From: Daniel Gomez <da.gomez@...sung.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:
> >> * meraki_mx100 pcengines_apuv2
> >>
> >> Signed-off-by: Daniel Gomez <da.gomez@...sung.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]
> >> ---
> >> drivers/base/swnode.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> >> index 16a8301c25d6..083593d99a18 100644
> >> --- a/drivers/base/swnode.c
> >> +++ b/drivers/base/swnode.c
> >> @@ -919,7 +919,7 @@ int software_node_register(const struct software_node *node)
> >> struct swnode *parent = software_node_to_swnode(node->parent);
> >>
> >> if (software_node_to_swnode(node))
> >> - return -EEXIST;
> >> + return -EBUSY;
> >
> > While I understand the want for the module loader to be returning
> > -EBUSY, that doesn't really make sense down here in this layer of the
> > kernel.
> >
> > So why doesn't the module loader turn -EEXIST return values into -EBUSY
> > if it wishes to pass that value on to userspace? Otherwise you are
>
> Indeed, we are planning to do that as well with "[PATCH 0/2] module: Tweak
> return and warning":
>
> https://lore.kernel.org/all/20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com/#t
>
> However, we don't consider that as the right fix.
>
> > going to be constantly playing "whack-a-mole" here and have really
> > set things up so that NO api can ever return EEXIST as an error value in
> > the future.
>
> 100%.
>
> For that reason, on top of the series from Lucas, we are documenting this to
> make it clear:
>
> https://lore.kernel.org/linux-modules/20251218-dev-module-init-eexists-modules-docs-v1-0-361569aa782a@samsung.com/T/#m2ed6fbffb3f78b9bff53840f6492a198c389cb50
Wait, no, that's not what I mean at all :)
> And sending patches where we see modules need fixing. I have already sent 6 out
> of 20-ish series (that include a total of 40+ fixes):
>
> https://lore.kernel.org/all/20251220-dev-module-init-eexists-linux-scsi-v1-0-5379db749d54@samsung.com
> https://lore.kernel.org/all/20251219-dev-module-init-eexists-netfilter-v1-1-efd3f62412dc@samsung.com
> https://lore.kernel.org/all/20251220-dev-module-init-eexists-bpf-v1-1-7f186663dbe7@samsung.com
> https://lore.kernel.org/all/20251220-dev-module-init-eexists-keyring-v1-1-a2f23248c300@samsung.com
> https://lore.kernel.org/all/20251220-dev-module-init-eexists-dm-devel-v1-1-90ed00444ea0@samsung.com
Please no, let us keep using -EEXIST in the kernel source, and if your
usage is going to map this to userspace somehow, do the translation
there, in the module code, as your original patch above said.
Otherwise, again, this is never going to work, let the subsystems use
this error code how ever they feel they need to.
thanks,
greg k-h
Powered by blists - more mailing lists