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

Mahanta Jambigi <[email protected]>
Newsgroups org.kernel.vger.linux-modules,org.kernel.vger.linux-kernel,org.kernel.vger.linux-s390,org.kernel.vger.netdev
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.