Re: [PATCH 2/2] mmc: core: Turn mmc_alloc|free_host() into static functions

[Ulf Hansson]

On Thu, Sep 17, 2026 at 9:58 AM Johan Hovold <johan@xxxxxxxxxx> wrote:
>
> On Wed, Sep 16, 2026 at 04:12:10PM +0200, Ulf Hansson wrote:
> > On Wed, Sep 16, 2026 at 12:00 PM Johan Hovold <johan@xxxxxxxxxx> wrote:
>
> > > Devres generally only works when all resources are device managed.
> > > Therefore you should always provide the underlying non-devres manages
> > > interface as well so that you don't force devres on drivers where it
> > > could cause trouble.
> > >
> > > > > Especially with the work cancellations present in mmc_free_host() (which
> > > > > I have pointed out elsewhere should not be there), a driver may need to
> > > > > free the host before tearing down other non-devres managed resources
> > > > > during unbind.
> > > >
> > > > Can you please point me to such an example so I can try to understand better?
> > >
> > > We just discussed the renesas driver which can schedule rescan work
> > > before registering the host controller. [1]
> >
> > That isn't a problem, but it's not the right thing to do as
> > host->rescan_disable is set.
>
> Ok, good, I see now that you also cancel the rescan work when the host
> is stopped so that seems fine even if the second cancel when freeing the
> host is unexpected and bit misleading (as it was added to work around a
> driver doing something it should not have).
>
> > > If such a driver also has non-devres managed resources that are freed
> > > before the work is cancelled you have a use-after-free.
> >
> > If you are referring to the mmc rescan work; mmc_rescan() will just
> > bail out as host->rescan_disable has been set as soon as
> > mmc_remove_host() is called (see mmc_stop_host()). In other words, I
> > don't see how a use-after-free would be possible in this regard.
>
> Looks like you also recently fixed a related issue with sdio interrupt
> work which also wasn't stopped (see commit 6feadbecdae6 ("mmc: core:
> Cancel SDIO IRQ work before freeing host")).
>
> So I was referring to all work which may still be running after
> mmc_remove_host() and potentially call back into the driver being
> unbound.

Okay, I see and understand your concern.

Although, if things work as they *should*, the mmc core must not call
back into the host driver when mmc_remove_host() has returned.

*If* that would be the case, then there is a bug in the mmc core that
needs to be fixed.

>
> A quick look at a driver using sdio_signal_irq() indicates that this may
> still be racy. In dw_mci_remove() the host is removed and clocks
> disabled before freeing the host and cancelling the sdio work. A racing
> interrupt could have scheduled work that may result in MMIO accesses
> with clocks disabled.

In this case, mmc_remove_host() also removes the corresponding SDIO
functional devices that correspond to the SDIO card, see
mmc_sdio_remove().

This leads to the SDIO functional driver's ->remove() callback being
called and from there it's expected that the SDIO irqs gets released
(see sdio_card_irq_put()), which must be done with the mmc host
claimed. If this isn't happening, it's a bug in the SDIO functional
driver.

Yes, there may still be an sdio_irq_work scheduled beyond this, but
when it runs and claims the host in sdio_run_irqs(), it sees that
there is no SDIO irqs enabled anymore and just returns. So, there
should be no calls back into the host beyond mmc_remove_host().

>
> > > I only pointed at greybus as an example of a driver which has non-devres
> > > managed resources. If there is (rescan) work still scheduled after
> > > probe() or remove() returns, there's a potential use-after-free.
> >
> > See above. This should not be an issue, at least it has nothing to do
> > whether we are using managed resources or not.
>
> Sure, if mmc core guarantees that no callbacks are made despite the work
> being left running it should be fine. But that does not seem to be the
> case with sdio interrupts currently.

See above. SDIO irqs should work perfectly fine too.

>
> But regardless of any potential races in mmc core, my devres comment is
> a more general one: subsystems should not force devres on anyone.

I understand your point, but I don't agree, at least for mmc.

Instead my take is; come with a use case where devres doesn't work and
I am perfectly fine to bring back the non-devres helpers.

Kind regards
Uffe