Re: [PATCH 1/2] module: Override -EEXISTS module return

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

On 10/11/2025 16.31, Lucas De Marchi wrote:
> On Mon, Nov 10, 2025 at 04:17:47PM +0100, Daniel Gomez wrote:
>> On 13/10/2025 18.26, Lucas De Marchi wrote:
>>> The -EEXIST errno is reserved by the module loading functionality. When
>>> userspace calls [f]init_module(), it expects a -EEXIST to mean that the
>>> module is already loaded in the kernel. If module_init() returns it,
>>> that is not true anymore.
>>>
>>> Add a warning and override the return code to workaround modules
>>> currently returning the wrong code. It's expected that they eventually
>>> migrate to a better suited error.
>>
>> I've been following the thread (and apologies for the delay) and reviewing the
>> patches, and I do not believe we should push this workaround. While this "fixes"
>> the bug reported, it also hides the real problem and drivers will continue
>> misusing EEXIST at module initialization.
>>
>> From the bug report thread, I agree with Christophe's suggestion that
>> nf_conntrack_helpers_register() should return EBUSY instead of EEXIST. This
>> would fix the root cause for this particular module and will allow others to
>> change their module behavior, if we also follow up with proper documentation
>> about EEXIST.
> 
> the fix will always be for the modules to stop returning EEXIST, no
> discussion on that. This is the messenger, not the fix. Someone starts
> seeing this warning and rjeports the bug. Then the developers can fix
> it. It's much easier than dealing with the outcome of a module returning
> EEXIST.

I see. Thanks for clarifying.

I'm thinking to merge this at the beginning of the next release cycle. So others
can also have time to process the new warning when testing their modules. I hope
that is okay with you.

FYI, I will follow up this with docs and some drivers fixes I've found.

> Also note that we already have a similar reasoning about adding
> a warning for module return codes in this same function.
> 
> Even if we had the means to check possible return codes from all
> module inits, we'd still have external modules.
> 
> See patch 2: after some time we can simplify the warning and maybe even
> remove it.

That was added almost two decades ago. I hope we don't need to wait that long!
:)

commit e24e2e64c468c8060bb7173abecdf11d00ed5751
Author: Alexey Dobriyan <adobriyan@gmail.com>
Date:   Mon Mar 10 11:43:53 2008 -0700

    modules: warn about suspicious return values from module's ->init() hook

I don't see a way to completely remove any of these, especially with out of 
tree modules... or to capture any possible future/new misses.

> 
> Lucas De Marchi