[RFC] module: init-failure path can free a module with live try_module_get() users

[RFC] module: init-failure path can free a module with live try_module_get() users

[Mahanta Jambigi]

Hi Luis, Petr, Daniel, Sami, Aaron,

I'm writing to ask about what looks like a generic module-init failure
lifetime problem in the module loader. I ran into it while working on
the SMC networking module (net/smc/), but after several patch
iterations, it seems the root issue may belong in kernel/module/main.c
rather than in SMC itself. I'd appreciate your guidance on whether this
reading is correct, and if so, what fix direction would be preferred.

THE ISSUE IN do_init_module()
=============================

include/linux/module.h has a long-standing FIXME in module_is_live():

    /* FIXME: It'd be nice to isolate modules during init, too, so they
       aren't used before they (may) fail. But presently too much code
       (IDE & SCSI) require entry into the module during init. */
    static inline bool module_is_live(struct module *mod)
    {
        return mod->state != MODULE_STATE_GOING;
    }

Because MODULE_STATE_COMING is not MODULE_STATE_GOING, try_module_get()
can succeed once a module's __init is executing. If __init makes the
module externally reachable partway through and then later fails, the
failure path in do_init_module() appears to do:

    fail:
        mod->state = MODULE_STATE_GOING;
        synchronize_rcu();
        module_put(mod);
        ...
        free_module(mod);

synchronize_rcu() waits for RCU readers, but not for threads that
already obtained a module reference via try_module_get() and are still
executing module text.

By contrast, the normal unload path in try_stop_module() refuses to
proceed while the refcount is non-zero.

So the asymmetry seems to be that the normal unload path waits for
references to drain, while the init-failure path does not.

A concrete race would look like:

  1. Module __init registers an externally reachable interface.
  2. User space enters through that interface and try_module_get()
     succeeds while the module is still COMING.
  3. A later __init step fails.
  4. do_init_module() frees the module.
  5. The in-flight caller is still executing module text.

SMC AS A CONCRETE EXAMPLE
=========================

In SMC, simply moving registration later does not appear to eliminate
the window, because there are two separate registration points that can
make the module reachable via socket():

  1. sock_register(&smc_sock_family_ops)
     After this, socket(AF_SMC, ...) can succeed and reach
     try_module_get() via __sock_create().

  2. smc_inet_init() -> inet_register_protosw()
     After this, socket(AF_INET, SOCK_STREAM, IPPROTO_SMC) can succeed
     and again reach try_module_get().

Either registration point can succeed before a later init step fails.

This may not be specific to SMC; other protocol modules that become
reachable during init, such as Bluetooth, may have similar exposure and
appear worth auditing as well.

ON THE FIXME'S IDE/SCSI CONCERN
===============================

The FIXME mentions IDE and SCSI as reasons not to isolate modules
during init.

  1. IDE was removed in Linux 5.14, so that half of the concern no
     longer applies.

  2. SCSI still appears to self-reference during init
     (scsi_device_get() -> try_module_get(hostt->module) during
     scsi_scan_host()), so a blanket wait-for-refcount-to-drain
     approach in the failure path may deadlock there.

Also, strong_try_module_get() already rejects MODULE_STATE_COMING with
-EBUSY, so the infrastructure for refusing callers during init already
exists in some form.

QUESTIONS
=========

First, is my reading of this init-failure refcount/lifetime asymmetry
correct?

If so, would one of the following directions be acceptable?

  1. An opt-in mechanism (for example, a module flag) for modules that
     are safe to isolate during init and whose init-failure path should
     wait for external references to drain.

  2. Treating MODULE_STATE_COMING as non-live for normal
     try_module_get() users, with some explicit escape hatch for the
     remaining subsystems that genuinely need self-entry during init.

Any guidance on the preferred direction would be much appreciated.

Best regards,
Mahanta Jambigi

Re: [RFC] module: init-failure path can free a module with live try_module_get() users

[Petr Pavlu]

On 8/24/26 8:13 AM, Mahanta Jambigi wrote:
> Hi Luis, Petr, Daniel, Sami, Aaron,
> 
> I'm writing to ask about what looks like a generic module-init failure
> lifetime problem in the module loader. I ran into it while working on
> the SMC networking module (net/smc/), but after several patch
> iterations, it seems the root issue may belong in kernel/module/main.c
> rather than in SMC itself. I'd appreciate your guidance on whether this
> reading is correct, and if so, what fix direction would be preferred.
> 
> THE ISSUE IN do_init_module()
> =============================
> 
> include/linux/module.h has a long-standing FIXME in module_is_live():
> 
>     /* FIXME: It'd be nice to isolate modules during init, too, so they
>        aren't used before they (may) fail. But presently too much code
>        (IDE & SCSI) require entry into the module during init. */
>     static inline bool module_is_live(struct module *mod)
>     {
>         return mod->state != MODULE_STATE_GOING;
>     }
> 
> Because MODULE_STATE_COMING is not MODULE_STATE_GOING, try_module_get()
> can succeed once a module's __init is executing. If __init makes the
> module externally reachable partway through and then later fails, the
> failure path in do_init_module() appears to do:
> 
>     fail:
>         mod->state = MODULE_STATE_GOING;
>         synchronize_rcu();
>         module_put(mod);
>         ...
>         free_module(mod);
> 
> synchronize_rcu() waits for RCU readers, but not for threads that
> already obtained a module reference via try_module_get() and are still
> executing module text.
> 
> By contrast, the normal unload path in try_stop_module() refuses to
> proceed while the refcount is non-zero.
> 
> So the asymmetry seems to be that the normal unload path waits for
> references to drain, while the init-failure path does not.
> 
> A concrete race would look like:
> 
>   1. Module __init registers an externally reachable interface.
>   2. User space enters through that interface and try_module_get()
>      succeeds while the module is still COMING.
>   3. A later __init step fails.
>   4. do_init_module() frees the module.
>   5. The in-flight caller is still executing module text.
> 
> SMC AS A CONCRETE EXAMPLE
> =========================
> 
> In SMC, simply moving registration later does not appear to eliminate
> the window, because there are two separate registration points that can
> make the module reachable via socket():
> 
>   1. sock_register(&smc_sock_family_ops)
>      After this, socket(AF_SMC, ...) can succeed and reach
>      try_module_get() via __sock_create().
> 
>   2. smc_inet_init() -> inet_register_protosw()
>      After this, socket(AF_INET, SOCK_STREAM, IPPROTO_SMC) can succeed
>      and again reach try_module_get().
> 
> Either registration point can succeed before a later init step fails.
> 
> This may not be specific to SMC; other protocol modules that become
> reachable during init, such as Bluetooth, may have similar exposure and
> appear worth auditing as well.
> 
> ON THE FIXME'S IDE/SCSI CONCERN
> ===============================
> 
> The FIXME mentions IDE and SCSI as reasons not to isolate modules
> during init.
> 
>   1. IDE was removed in Linux 5.14, so that half of the concern no
>      longer applies.
> 
>   2. SCSI still appears to self-reference during init
>      (scsi_device_get() -> try_module_get(hostt->module) during
>      scsi_scan_host()), so a blanket wait-for-refcount-to-drain
>      approach in the failure path may deadlock there.
> 
> Also, strong_try_module_get() already rejects MODULE_STATE_COMING with
> -EBUSY, so the infrastructure for refusing callers during init already
> exists in some form.
> 
> QUESTIONS
> =========
> 
> First, is my reading of this init-failure refcount/lifetime asymmetry
> correct?

Your analysis looks correct to me.

> 
> If so, would one of the following directions be acceptable?
> 
>   1. An opt-in mechanism (for example, a module flag) for modules that
>      are safe to isolate during init and whose init-failure path should
>      wait for external references to drain.

In general, it is preferred if the module loader handles all modules in
the same way.

I would say that the module loader should wait for external references
to drain after an init failure for all modules and that it should be the
responsibility of individual modules to ensure that this wait eventually
completes. Excluding some modules would mean that the module loader
could still free them while they are in use by the kernel.

Before such a wait, the module loader should cancel all idempotent
module loads. This is especially important during boot when several
udevd workers may be trying to insert the same module. In that case,
a failed module init function should block only a single udevd task, so
that the system can still boot properly.

> 
>   2. Treating MODULE_STATE_COMING as non-live for normal
>      try_module_get() users, with some explicit escape hatch for the
>      remaining subsystems that genuinely need self-entry during init.

This might make sense, but the kernel currently has over 500 users of
try_module_get() so changing its semantics in this way would need to be
done very carefully.

I'm also not sure there is anything inherently wrong with calling
try_module_get() on a module while it is still executing its init
function. It must mean the module has reached a state in which it
properly initialized functionality to be registered with a specific
subsystem.

For instance, the __sock_create() function mentioned in your email
contains (simplified):

[...]
	if (rcu_access_pointer(net_families[family]) == NULL)
		request_module("net-pf-%d", family);

	rcu_read_lock();
	pf = rcu_dereference(net_families[family]);
	err = -EAFNOSUPPORT;
	if (!pf)
		goto out_release;

	if (!try_module_get(pf->owner))
		goto out_release;
[...]

Consider two tasks that both call socket(XYZ). The first one sees
net_families[family]==NULL and calls request_module(XYZ). The module
then starts loading and its init function calls sock_register(XYZ). At
this point, the second task can see that net_families[family]!=NULL, so
it skips request_module(XYZ) and calls try_module_get(pf->owner). If
that call newly fails because the module is still in
MODULE_STATE_COMING, it breaks the autoload functionality of
__sock_create().

The scheme would need to be changed to call request_module(XYZ) if
either net_families[family]==NULL or !try_module_get(pf->owner).

> 
> Any guidance on the preferred direction would be much appreciated.

Waiting for external references to drain after an init failure makes
sense to me. I'm less sure about changing the semantics of
try_module_get(), or whether strong_try_module_get() should instead be
exposed.

-- 
Thanks,
Petr

Re: [RFC] module: init-failure path can free a module with live try_module_get() users

[Mahanta Jambigi]

On 28/08/26 5:33 pm, Petr Pavlu wrote:
> On 8/24/26 8:13 AM, Mahanta Jambigi wrote:
>> Hi Luis, Petr, Daniel, Sami, Aaron,
>>
>> I'm writing to ask about what looks like a generic module-init failure
>> lifetime problem in the module loader. I ran into it while working on
>> the SMC networking module (net/smc/), but after several patch
>> iterations, it seems the root issue may belong in kernel/module/main.c
>> rather than in SMC itself. I'd appreciate your guidance on whether this
>> reading is correct, and if so, what fix direction would be preferred.
>>
>> THE ISSUE IN do_init_module()
>> =============================
>>
>> include/linux/module.h has a long-standing FIXME in module_is_live():
>>
>>     /* FIXME: It'd be nice to isolate modules during init, too, so they
>>        aren't used before they (may) fail. But presently too much code
>>        (IDE & SCSI) require entry into the module during init. */
>>     static inline bool module_is_live(struct module *mod)
>>     {
>>         return mod->state != MODULE_STATE_GOING;
>>     }
>>
>> Because MODULE_STATE_COMING is not MODULE_STATE_GOING, try_module_get()
>> can succeed once a module's __init is executing. If __init makes the
>> module externally reachable partway through and then later fails, the
>> failure path in do_init_module() appears to do:
>>
>>     fail:
>>         mod->state = MODULE_STATE_GOING;
>>         synchronize_rcu();
>>         module_put(mod);
>>         ...
>>         free_module(mod);
>>
>> synchronize_rcu() waits for RCU readers, but not for threads that
>> already obtained a module reference via try_module_get() and are still
>> executing module text.
>>
>> By contrast, the normal unload path in try_stop_module() refuses to
>> proceed while the refcount is non-zero.
>>
>> So the asymmetry seems to be that the normal unload path waits for
>> references to drain, while the init-failure path does not.
>>
>> A concrete race would look like:
>>
>>   1. Module __init registers an externally reachable interface.
>>   2. User space enters through that interface and try_module_get()
>>      succeeds while the module is still COMING.
>>   3. A later __init step fails.
>>   4. do_init_module() frees the module.
>>   5. The in-flight caller is still executing module text.
>>
>> SMC AS A CONCRETE EXAMPLE
>> =========================
>>
>> In SMC, simply moving registration later does not appear to eliminate
>> the window, because there are two separate registration points that can
>> make the module reachable via socket():
>>
>>   1. sock_register(&smc_sock_family_ops)
>>      After this, socket(AF_SMC, ...) can succeed and reach
>>      try_module_get() via __sock_create().
>>
>>   2. smc_inet_init() -> inet_register_protosw()
>>      After this, socket(AF_INET, SOCK_STREAM, IPPROTO_SMC) can succeed
>>      and again reach try_module_get().
>>
>> Either registration point can succeed before a later init step fails.
>>
>> This may not be specific to SMC; other protocol modules that become
>> reachable during init, such as Bluetooth, may have similar exposure and
>> appear worth auditing as well.
>>
>> ON THE FIXME'S IDE/SCSI CONCERN
>> ===============================
>>
>> The FIXME mentions IDE and SCSI as reasons not to isolate modules
>> during init.
>>
>>   1. IDE was removed in Linux 5.14, so that half of the concern no
>>      longer applies.
>>
>>   2. SCSI still appears to self-reference during init
>>      (scsi_device_get() -> try_module_get(hostt->module) during
>>      scsi_scan_host()), so a blanket wait-for-refcount-to-drain
>>      approach in the failure path may deadlock there.
>>
>> Also, strong_try_module_get() already rejects MODULE_STATE_COMING with
>> -EBUSY, so the infrastructure for refusing callers during init already
>> exists in some form.
>>
>> QUESTIONS
>> =========
>>
>> First, is my reading of this init-failure refcount/lifetime asymmetry
>> correct?
> 
> Your analysis looks correct to me.
> 
>>
>> If so, would one of the following directions be acceptable?
>>
>>   1. An opt-in mechanism (for example, a module flag) for modules that
>>      are safe to isolate during init and whose init-failure path should
>>      wait for external references to drain.
> 
> In general, it is preferred if the module loader handles all modules in
> the same way.
> 
> I would say that the module loader should wait for external references
> to drain after an init failure for all modules and that it should be the
> responsibility of individual modules to ensure that this wait eventually
> completes. Excluding some modules would mean that the module loader
> could still free them while they are in use by the kernel.
> 
> Before such a wait, the module loader should cancel all idempotent
> module loads. This is especially important during boot when several
> udevd workers may be trying to insert the same module. In that case,
> a failed module init function should block only a single udevd task, so
> that the system can still boot properly.
> 
Thank you for the clear direction. I agree with both points — uniform
handling for all modules, and unblocking concurrent loaders before the
drain wait. Below is the proposed change with the rationale for each
step. Proposed change to the fail: path in do_init_module().

fail_free_freeinit:
	kfree(freeinit);
fail:
	/*
	 * Mark dying so try_module_get() fails for all new callers.
	 * synchronize_rcu() ensures this is visible on all CPUs before
	 * we proceed; no new references can be taken after this point.
	 */
	mod->state = MODULE_STATE_GOING;
	synchronize_rcu();

	/* Drop the loader's own reference taken in module_unload_init(). */
	module_put(mod);

	/*
	 * Unblock concurrent loaders before blocking on the drain below,
	 * so that a failed init delays only this task, not every udevd
	 * worker that raced to load the same module.
	 *
	 * Two dedup paths exist:
	 *
	 * - finit_module path: losers of the inode race sleep in
	 *   idempotent_wait_for_completion(). They are unblocked by
	 *   idempotent_complete() in idempotent_init_module(), which
	 *   runs as do_init_module() returns — before we reach here.
	 *   No action needed.
	 *
	 * - init_module path: callers sleep in module_patient_check_exists()
	 *   on module_wq waiting for finished_loading(), which returns
	 *   true once state == MODULE_STATE_GOING. wake_up_all() kicks
	 *   them loose immediately.
	 */
	*wake_up_all*(&module_wq);

	/*
	 * Drain async workers scheduled during __init (e.g. SCSI async
	 * scan). MODULE_STATE_GOING is visible everywhere, so workers
	 * that have not yet called try_module_get() will fail cleanly.
	 * Workers already holding a reference complete and release it
	 * naturally. Must run before free_module() regardless of
	 * async_probe_requested.
	 */
	*async_synchronize_full*();

	/*
	 * Wait for references taken before MODULE_STATE_GOING became
	 * visible. refcnt is monotonically decreasing from here; the
	 * loop terminates provided the module's error path pairs every
	 * __module_get() with a module_put(). The hung-task detector
	 * catches violations.
	 */
	while (*module_refcount*(mod) != 0)
		msleep(10);

	blocking_notifier_call_chain(&module_notify_list,
				     MODULE_STATE_GOING, mod);
	klp_module_going(mod);
	ftrace_release_mod(mod);
	free_module(mod);

	return ret;

Does this direction look correct to you?

Re: [RFC] module: init-failure path can free a module with live try_module_get() users

[Petr Pavlu]

On 8/31/26 12:00 PM, Mahanta Jambigi wrote:
> On 28/08/26 5:33 pm, Petr Pavlu wrote:
>> On 8/24/26 8:13 AM, Mahanta Jambigi wrote:
>>> Hi Luis, Petr, Daniel, Sami, Aaron,
>>>
>>> I'm writing to ask about what looks like a generic module-init failure
>>> lifetime problem in the module loader. I ran into it while working on
>>> the SMC networking module (net/smc/), but after several patch
>>> iterations, it seems the root issue may belong in kernel/module/main.c
>>> rather than in SMC itself. I'd appreciate your guidance on whether this
>>> reading is correct, and if so, what fix direction would be preferred.
>>>
>>> THE ISSUE IN do_init_module()
>>> =============================
>>>
>>> include/linux/module.h has a long-standing FIXME in module_is_live():
>>>
>>>     /* FIXME: It'd be nice to isolate modules during init, too, so they
>>>        aren't used before they (may) fail. But presently too much code
>>>        (IDE & SCSI) require entry into the module during init. */
>>>     static inline bool module_is_live(struct module *mod)
>>>     {
>>>         return mod->state != MODULE_STATE_GOING;
>>>     }
>>>
>>> Because MODULE_STATE_COMING is not MODULE_STATE_GOING, try_module_get()
>>> can succeed once a module's __init is executing. If __init makes the
>>> module externally reachable partway through and then later fails, the
>>> failure path in do_init_module() appears to do:
>>>
>>>     fail:
>>>         mod->state = MODULE_STATE_GOING;
>>>         synchronize_rcu();
>>>         module_put(mod);
>>>         ...
>>>         free_module(mod);
>>>
>>> synchronize_rcu() waits for RCU readers, but not for threads that
>>> already obtained a module reference via try_module_get() and are still
>>> executing module text.
>>>
>>> By contrast, the normal unload path in try_stop_module() refuses to
>>> proceed while the refcount is non-zero.
>>>
>>> So the asymmetry seems to be that the normal unload path waits for
>>> references to drain, while the init-failure path does not.
>>>
>>> A concrete race would look like:
>>>
>>>   1. Module __init registers an externally reachable interface.
>>>   2. User space enters through that interface and try_module_get()
>>>      succeeds while the module is still COMING.
>>>   3. A later __init step fails.
>>>   4. do_init_module() frees the module.
>>>   5. The in-flight caller is still executing module text.
>>>
>>> SMC AS A CONCRETE EXAMPLE
>>> =========================
>>>
>>> In SMC, simply moving registration later does not appear to eliminate
>>> the window, because there are two separate registration points that can
>>> make the module reachable via socket():
>>>
>>>   1. sock_register(&smc_sock_family_ops)
>>>      After this, socket(AF_SMC, ...) can succeed and reach
>>>      try_module_get() via __sock_create().
>>>
>>>   2. smc_inet_init() -> inet_register_protosw()
>>>      After this, socket(AF_INET, SOCK_STREAM, IPPROTO_SMC) can succeed
>>>      and again reach try_module_get().
>>>
>>> Either registration point can succeed before a later init step fails.
>>>
>>> This may not be specific to SMC; other protocol modules that become
>>> reachable during init, such as Bluetooth, may have similar exposure and
>>> appear worth auditing as well.
>>>
>>> ON THE FIXME'S IDE/SCSI CONCERN
>>> ===============================
>>>
>>> The FIXME mentions IDE and SCSI as reasons not to isolate modules
>>> during init.
>>>
>>>   1. IDE was removed in Linux 5.14, so that half of the concern no
>>>      longer applies.
>>>
>>>   2. SCSI still appears to self-reference during init
>>>      (scsi_device_get() -> try_module_get(hostt->module) during
>>>      scsi_scan_host()), so a blanket wait-for-refcount-to-drain
>>>      approach in the failure path may deadlock there.
>>>
>>> Also, strong_try_module_get() already rejects MODULE_STATE_COMING with
>>> -EBUSY, so the infrastructure for refusing callers during init already
>>> exists in some form.
>>>
>>> QUESTIONS
>>> =========
>>>
>>> First, is my reading of this init-failure refcount/lifetime asymmetry
>>> correct?
>>
>> Your analysis looks correct to me.
>>
>>>
>>> If so, would one of the following directions be acceptable?
>>>
>>>   1. An opt-in mechanism (for example, a module flag) for modules that
>>>      are safe to isolate during init and whose init-failure path should
>>>      wait for external references to drain.
>>
>> In general, it is preferred if the module loader handles all modules in
>> the same way.
>>
>> I would say that the module loader should wait for external references
>> to drain after an init failure for all modules and that it should be the
>> responsibility of individual modules to ensure that this wait eventually
>> completes. Excluding some modules would mean that the module loader
>> could still free them while they are in use by the kernel.
>>
>> Before such a wait, the module loader should cancel all idempotent
>> module loads. This is especially important during boot when several
>> udevd workers may be trying to insert the same module. In that case,
>> a failed module init function should block only a single udevd task, so
>> that the system can still boot properly.
>>
> Thank you for the clear direction. I agree with both points — uniform
> handling for all modules, and unblocking concurrent loaders before the
> drain wait. Below is the proposed change with the rationale for each
> step. Proposed change to the fail: path in do_init_module().
> 
> fail_free_freeinit:
> 	kfree(freeinit);
> fail:
> 	/*
> 	 * Mark dying so try_module_get() fails for all new callers.
> 	 * synchronize_rcu() ensures this is visible on all CPUs before
> 	 * we proceed; no new references can be taken after this point.
> 	 */
> 	mod->state = MODULE_STATE_GOING;
> 	synchronize_rcu();

The comment is somewhat misleading. The code invokes synchronize_rcu()
to wait for any existing RCU readers to finish before proceeding, making
sure that all new readers now observe the GOING state.

> 
> 	/* Drop the loader's own reference taken in module_unload_init(). */
> 	module_put(mod);
> 
> 	/*
> 	 * Unblock concurrent loaders before blocking on the drain below,
> 	 * so that a failed init delays only this task, not every udevd
> 	 * worker that raced to load the same module.
> 	 *
> 	 * Two dedup paths exist:
> 	 *
> 	 * - finit_module path: losers of the inode race sleep in
> 	 *   idempotent_wait_for_completion(). They are unblocked by
> 	 *   idempotent_complete() in idempotent_init_module(), which
> 	 *   runs as do_init_module() returns — before we reach here.
> 	 *   No action needed.

This looks incorrect. Since this code is added in do_init_module(),
idempotent_complete() has not yet been invoked.

> 	 *
> 	 * - init_module path: callers sleep in module_patient_check_exists()
> 	 *   on module_wq waiting for finished_loading(), which returns
> 	 *   true once state == MODULE_STATE_GOING. wake_up_all() kicks
> 	 *   them loose immediately.
> 	 */
> 	*wake_up_all*(&module_wq);
> 
> 	/*
> 	 * Drain async workers scheduled during __init (e.g. SCSI async
> 	 * scan). MODULE_STATE_GOING is visible everywhere, so workers
> 	 * that have not yet called try_module_get() will fail cleanly.
> 	 * Workers already holding a reference complete and release it
> 	 * naturally. Must run before free_module() regardless of
> 	 * async_probe_requested.
> 	 */
> 	*async_synchronize_full*();
> 
> 	/*
> 	 * Wait for references taken before MODULE_STATE_GOING became
> 	 * visible. refcnt is monotonically decreasing from here; the
> 	 * loop terminates provided the module's error path pairs every
> 	 * __module_get() with a module_put(). The hung-task detector
> 	 * catches violations.
> 	 */
> 	while (*module_refcount*(mod) != 0)
> 		msleep(10);

I think that instead of repeatedly polling the module refcount, it would
be better for this code to sleep and for module_put() to wake it when
the refcount drops to 0. This could be implemented using either the
existing module_wq, a separate wait queue or a per-module completion.

-- 
Thanks,
Petr

Re: [RFC] module: init-failure path can free a module with live try_module_get() users

[Mahanta Jambigi]

On 08/09/26 5:40 pm, Petr Pavlu wrote:
> On 8/31/26 12:00 PM, Mahanta Jambigi wrote:
>> On 28/08/26 5:33 pm, Petr Pavlu wrote:
>>> On 8/24/26 8:13 AM, Mahanta Jambigi wrote:
>>>> Hi Luis, Petr, Daniel, Sami, Aaron,
>>>>
>>>> I'm writing to ask about what looks like a generic module-init failure
>>>> lifetime problem in the module loader. I ran into it while working on
>>>> the SMC networking module (net/smc/), but after several patch
>>>> iterations, it seems the root issue may belong in kernel/module/main.c
>>>> rather than in SMC itself. I'd appreciate your guidance on whether this
>>>> reading is correct, and if so, what fix direction would be preferred.
>>>>
>>>> THE ISSUE IN do_init_module()
>>>> =============================
>>>>
>>>> include/linux/module.h has a long-standing FIXME in module_is_live():
>>>>
>>>>     /* FIXME: It'd be nice to isolate modules during init, too, so they
>>>>        aren't used before they (may) fail. But presently too much code
>>>>        (IDE & SCSI) require entry into the module during init. */
>>>>     static inline bool module_is_live(struct module *mod)
>>>>     {
>>>>         return mod->state != MODULE_STATE_GOING;
>>>>     }
>>>>
>>>> Because MODULE_STATE_COMING is not MODULE_STATE_GOING, try_module_get()
>>>> can succeed once a module's __init is executing. If __init makes the
>>>> module externally reachable partway through and then later fails, the
>>>> failure path in do_init_module() appears to do:
>>>>
>>>>     fail:
>>>>         mod->state = MODULE_STATE_GOING;
>>>>         synchronize_rcu();
>>>>         module_put(mod);
>>>>         ...
>>>>         free_module(mod);
>>>>
>>>> synchronize_rcu() waits for RCU readers, but not for threads that
>>>> already obtained a module reference via try_module_get() and are still
>>>> executing module text.
>>>>
>>>> By contrast, the normal unload path in try_stop_module() refuses to
>>>> proceed while the refcount is non-zero.
>>>>
>>>> So the asymmetry seems to be that the normal unload path waits for
>>>> references to drain, while the init-failure path does not.
>>>>
>>>> A concrete race would look like:
>>>>
>>>>   1. Module __init registers an externally reachable interface.
>>>>   2. User space enters through that interface and try_module_get()
>>>>      succeeds while the module is still COMING.
>>>>   3. A later __init step fails.
>>>>   4. do_init_module() frees the module.
>>>>   5. The in-flight caller is still executing module text.
>>>>
>>>> SMC AS A CONCRETE EXAMPLE
>>>> =========================
>>>>
>>>> In SMC, simply moving registration later does not appear to eliminate
>>>> the window, because there are two separate registration points that can
>>>> make the module reachable via socket():
>>>>
>>>>   1. sock_register(&smc_sock_family_ops)
>>>>      After this, socket(AF_SMC, ...) can succeed and reach
>>>>      try_module_get() via __sock_create().
>>>>
>>>>   2. smc_inet_init() -> inet_register_protosw()
>>>>      After this, socket(AF_INET, SOCK_STREAM, IPPROTO_SMC) can succeed
>>>>      and again reach try_module_get().
>>>>
>>>> Either registration point can succeed before a later init step fails.
>>>>
>>>> This may not be specific to SMC; other protocol modules that become
>>>> reachable during init, such as Bluetooth, may have similar exposure and
>>>> appear worth auditing as well.
>>>>
>>>> ON THE FIXME'S IDE/SCSI CONCERN
>>>> ===============================
>>>>
>>>> The FIXME mentions IDE and SCSI as reasons not to isolate modules
>>>> during init.
>>>>
>>>>   1. IDE was removed in Linux 5.14, so that half of the concern no
>>>>      longer applies.
>>>>
>>>>   2. SCSI still appears to self-reference during init
>>>>      (scsi_device_get() -> try_module_get(hostt->module) during
>>>>      scsi_scan_host()), so a blanket wait-for-refcount-to-drain
>>>>      approach in the failure path may deadlock there.
>>>>
>>>> Also, strong_try_module_get() already rejects MODULE_STATE_COMING with
>>>> -EBUSY, so the infrastructure for refusing callers during init already
>>>> exists in some form.
>>>>
>>>> QUESTIONS
>>>> =========
>>>>
>>>> First, is my reading of this init-failure refcount/lifetime asymmetry
>>>> correct?
>>>
>>> Your analysis looks correct to me.
>>>
>>>>
>>>> If so, would one of the following directions be acceptable?
>>>>
>>>>   1. An opt-in mechanism (for example, a module flag) for modules that
>>>>      are safe to isolate during init and whose init-failure path should
>>>>      wait for external references to drain.
>>>
>>> In general, it is preferred if the module loader handles all modules in
>>> the same way.
>>>
>>> I would say that the module loader should wait for external references
>>> to drain after an init failure for all modules and that it should be the
>>> responsibility of individual modules to ensure that this wait eventually
>>> completes. Excluding some modules would mean that the module loader
>>> could still free them while they are in use by the kernel.
>>>
>>> Before such a wait, the module loader should cancel all idempotent
>>> module loads. This is especially important during boot when several
>>> udevd workers may be trying to insert the same module. In that case,
>>> a failed module init function should block only a single udevd task, so
>>> that the system can still boot properly.
>>>
>> Thank you for the clear direction. I agree with both points — uniform
>> handling for all modules, and unblocking concurrent loaders before the
>> drain wait. Below is the proposed change with the rationale for each
>> step. Proposed change to the fail: path in do_init_module().
>>
>> fail_free_freeinit:
>> 	kfree(freeinit);
>> fail:
>> 	/*
>> 	 * Mark dying so try_module_get() fails for all new callers.
>> 	 * synchronize_rcu() ensures this is visible on all CPUs before
>> 	 * we proceed; no new references can be taken after this point.
>> 	 */
>> 	mod->state = MODULE_STATE_GOING;
>> 	synchronize_rcu();
> 
> The comment is somewhat misleading. The code invokes synchronize_rcu()
> to wait for any existing RCU readers to finish before proceeding, making
> sure that all new readers now observe the GOING state.

synchronize_rcu() comment — agreed, updated wording:

"synchronize_rcu() waits for any in-progress RCU read-side critical
sections to complete, ensuring all CPUs observe MODULE_STATE_GOING
before we proceed."

>>
>> 	/* Drop the loader's own reference taken in module_unload_init(). */
>> 	module_put(mod);
>>
>> 	/*
>> 	 * Unblock concurrent loaders before blocking on the drain below,
>> 	 * so that a failed init delays only this task, not every udevd
>> 	 * worker that raced to load the same module.
>> 	 *
>> 	 * Two dedup paths exist:
>> 	 *
>> 	 * - finit_module path: losers of the inode race sleep in
>> 	 *   idempotent_wait_for_completion(). They are unblocked by
>> 	 *   idempotent_complete() in idempotent_init_module(), which
>> 	 *   runs as do_init_module() returns — before we reach here.
>> 	 *   No action needed.
> 
> This looks incorrect. Since this code is added in do_init_module(),
> idempotent_complete() has not yet been invoked.

finit_module / idempotent_complete ordering — you are right.
idempotent_complete() is called by idempotent_init_module() after
do_init_module() returns, not before.

> 
>> 	 *
>> 	 * - init_module path: callers sleep in module_patient_check_exists()
>> 	 *   on module_wq waiting for finished_loading(), which returns
>> 	 *   true once state == MODULE_STATE_GOING. wake_up_all() kicks
>> 	 *   them loose immediately.
>> 	 */
>> 	*wake_up_all*(&module_wq);
>>
>> 	/*
>> 	 * Drain async workers scheduled during __init (e.g. SCSI async
>> 	 * scan). MODULE_STATE_GOING is visible everywhere, so workers
>> 	 * that have not yet called try_module_get() will fail cleanly.
>> 	 * Workers already holding a reference complete and release it
>> 	 * naturally. Must run before free_module() regardless of
>> 	 * async_probe_requested.
>> 	 */
>> 	*async_synchronize_full*();
>>
>> 	/*
>> 	 * Wait for references taken before MODULE_STATE_GOING became
>> 	 * visible. refcnt is monotonically decreasing from here; the
>> 	 * loop terminates provided the module's error path pairs every
>> 	 * __module_get() with a module_put(). The hung-task detector
>> 	 * catches violations.
>> 	 */
>> 	while (*module_refcount*(mod) != 0)
>> 		msleep(10);
> 
> I think that instead of repeatedly polling the module refcount, it would
> be better for this code to sleep and for module_put() to wake it when
> the refcount drops to 0. This could be implemented using either the
> existing module_wq, a separate wait queue or a per-module completion.

Refcount drain — agreed that polling is poor style.  Of the three
approaches you suggested, a per-module completion is the cleanest fit
here.  module_wq is a global queue shared across all module loading
activity; reusing it for the drain would require a condition check on
every wake-up and risk spurious wakes across unrelated modules.  A
separate dedicated wait queue would work but adds more infrastructure
than needed for a single binary event.

A per-module struct completion is the right primitive for "wait for this
one thing to happen exactly once": add refs_gone to struct module inside
CONFIG_MODULE_UNLOAD, initialize it in module_unload_init(), and call
complete() from module_put() when refcnt drops to MODULE_REF_BASE.  The
fail: path then replaces the polling loop with a single
wait_for_completion(&mod->refs_gone).

Rather than continuing the design discussion I will send the patch
directly so it is easier to review concretely.

Re: [RFC] module: init-failure path can free a module with live try_module_get() users

[David Laight]

On Mon, 24 Aug 2026 11:43:46 +0530
Mahanta Jambigi <mjambigi@linux.ibm.com> wrote:

> Hi Luis, Petr, Daniel, Sami, Aaron,
> 
> I'm writing to ask about what looks like a generic module-init failure
> lifetime problem in the module loader. I ran into it while working on
> the SMC networking module (net/smc/), but after several patch
> iterations, it seems the root issue may belong in kernel/module/main.c
> rather than in SMC itself. I'd appreciate your guidance on whether this
> reading is correct, and if so, what fix direction would be preferred.
> 
> THE ISSUE IN do_init_module()
> =============================
> 
> include/linux/module.h has a long-standing FIXME in module_is_live():
> 
>     /* FIXME: It'd be nice to isolate modules during init, too, so they
>        aren't used before they (may) fail. But presently too much code
>        (IDE & SCSI) require entry into the module during init. */
>     static inline bool module_is_live(struct module *mod)
>     {
>         return mod->state != MODULE_STATE_GOING;
>     }
> 
> Because MODULE_STATE_COMING is not MODULE_STATE_GOING, try_module_get()
> can succeed once a module's __init is executing. If __init makes the
> module externally reachable partway through and then later fails, the
> failure path in do_init_module() appears to do:

It is rather worse that that.
If sock_create() auto-loads a module (eg sctp) then nothing stops a second
sock_create() entering the protocol code before the initialisation completes.
That can be hit by two separate applications, I hit it from an out of tree
kernel module and avoided the problem by putting a mutex() around the
sock_create() call.

It might help by letting try_module_get(THIS_MODULE) always succeed
while blocking other requests until initialisation completes.
The code making the call must own a reference (otherwise the code could
just disappear), and that reference stops the module being unloaded.
That would let the initialisation code grab extra references (eg for
a worker thread) without allowing other codes paths enter the
part-initialised driver.

David

> 
>     fail:
>         mod->state = MODULE_STATE_GOING;
>         synchronize_rcu();
>         module_put(mod);
>         ...
>         free_module(mod);
> 
> synchronize_rcu() waits for RCU readers, but not for threads that
> already obtained a module reference via try_module_get() and are still
> executing module text.
> 
> By contrast, the normal unload path in try_stop_module() refuses to
> proceed while the refcount is non-zero.
> 
> So the asymmetry seems to be that the normal unload path waits for
> references to drain, while the init-failure path does not.
> 
> A concrete race would look like:
> 
>   1. Module __init registers an externally reachable interface.
>   2. User space enters through that interface and try_module_get()
>      succeeds while the module is still COMING.
>   3. A later __init step fails.
>   4. do_init_module() frees the module.
>   5. The in-flight caller is still executing module text.
> 
> SMC AS A CONCRETE EXAMPLE
> =========================
> 
> In SMC, simply moving registration later does not appear to eliminate
> the window, because there are two separate registration points that can
> make the module reachable via socket():
> 
>   1. sock_register(&smc_sock_family_ops)
>      After this, socket(AF_SMC, ...) can succeed and reach
>      try_module_get() via __sock_create().
> 
>   2. smc_inet_init() -> inet_register_protosw()
>      After this, socket(AF_INET, SOCK_STREAM, IPPROTO_SMC) can succeed
>      and again reach try_module_get().
> 
> Either registration point can succeed before a later init step fails.
> 
> This may not be specific to SMC; other protocol modules that become
> reachable during init, such as Bluetooth, may have similar exposure and
> appear worth auditing as well.
> 
> ON THE FIXME'S IDE/SCSI CONCERN
> ===============================
> 
> The FIXME mentions IDE and SCSI as reasons not to isolate modules
> during init.
> 
>   1. IDE was removed in Linux 5.14, so that half of the concern no
>      longer applies.
> 
>   2. SCSI still appears to self-reference during init
>      (scsi_device_get() -> try_module_get(hostt->module) during
>      scsi_scan_host()), so a blanket wait-for-refcount-to-drain
>      approach in the failure path may deadlock there.
> 
> Also, strong_try_module_get() already rejects MODULE_STATE_COMING with
> -EBUSY, so the infrastructure for refusing callers during init already
> exists in some form.
> 
> QUESTIONS
> =========
> 
> First, is my reading of this init-failure refcount/lifetime asymmetry
> correct?
> 
> If so, would one of the following directions be acceptable?
> 
>   1. An opt-in mechanism (for example, a module flag) for modules that
>      are safe to isolate during init and whose init-failure path should
>      wait for external references to drain.
> 
>   2. Treating MODULE_STATE_COMING as non-live for normal
>      try_module_get() users, with some explicit escape hatch for the
>      remaining subsystems that genuinely need self-entry during init.
> 
> Any guidance on the preferred direction would be much appreciated.
> 
> Best regards,
> Mahanta Jambigi
>