All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Mahanta Jambigi <mjambigi@linux.ibm.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>,
	Petr Pavlu <petr.pavlu@suse.com>,
	Daniel Gomez <da.gomez@kernel.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	Aaron Tomlin <atomlin@atomlin.com>,
	linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, linux-s390@vger.kernel.org,
	"D. Wythe" <alibuda@linux.alibaba.com>,
	Dust Li <dust.li@linux.alibaba.com>,
	Sidraya Jayagond <sidraya@linux.ibm.com>,
	Tony Lu <tonylu@linux.alibaba.com>,
	Wen Gu <guwen@linux.alibaba.com>,
	Alexandra Winter <wintera@linux.ibm.com>,
	Halil Pasic <pasic@linux.ibm.com>,
	Hidayath Khan <hidayath@linux.ibm.com>
Subject: Re: [RFC] module: init-failure path can free a module with live try_module_get() users
Date: Mon, 21 Sep 2026 16:33:47 +0100	[thread overview]
Message-ID: <20260921163347.2630a6f4@pumpkin> (raw)
In-Reply-To: <2cd4adea-9b70-4951-be8b-40c1bcc7ecd6@linux.ibm.com>

On Mon, 21 Sep 2026 20:04:10 +0530
Mahanta Jambigi <mjambigi@linux.ibm.com> wrote:

> On 18/09/26 6:52 pm, David Laight wrote:
> > 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.  
> 
> Thanks David — you're right that the race is broader. This patch
> addresses only the UAF on the __init failure path: once we set
> MODULE_STATE_GOING and call synchronize_rcu(), new callers see GOING and
> fail; we then drain existing refs before free_module().
> 
> The concurrent-init race you describe — two callers entering
> MODULE_STATE_COMING simultaneously during a successful init — is not
> addressed here and would require changes to try_module_get() itself, as
> you suggest. That is the long-standing FIXME in module_is_live() and is
> a separate, larger change.

I suspect it is also much more common.
Module load doesn't normally fail, but if you can persuade the system to
unload an unused module I'd expect a non-root user can hit the concurrent
init race.

David

      reply	other threads:[~2026-09-21 15:33 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-24  6:13 [RFC] module: init-failure path can free a module with live try_module_get() users Mahanta Jambigi
2026-08-28 12:03 ` Petr Pavlu
2026-08-31 10:00   ` Mahanta Jambigi
2026-09-08 12:10     ` Petr Pavlu
2026-09-18 12:36       ` Mahanta Jambigi
2026-09-18 13:22 ` David Laight
2026-09-21 14:34   ` Mahanta Jambigi
2026-09-21 15:33     ` David Laight [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260921163347.2630a6f4@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=alibuda@linux.alibaba.com \
    --cc=atomlin@atomlin.com \
    --cc=da.gomez@kernel.org \
    --cc=dust.li@linux.alibaba.com \
    --cc=guwen@linux.alibaba.com \
    --cc=hidayath@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=mjambigi@linux.ibm.com \
    --cc=netdev@vger.kernel.org \
    --cc=pasic@linux.ibm.com \
    --cc=petr.pavlu@suse.com \
    --cc=samitolvanen@google.com \
    --cc=sidraya@linux.ibm.com \
    --cc=tonylu@linux.alibaba.com \
    --cc=wintera@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.