From: Johan Hovold <johan@kernel.org>
To: Ulf Hansson <ulf.hansson@oss.qualcomm.com>
Cc: linux-mmc@vger.kernel.org, Ulf Hansson <ulfh@kernel.org>,
Rui Miguel Silva <rmfrfs@gmail.com>,
Alex Elder <elder@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] mmc: core: Turn mmc_alloc|free_host() into static functions
Date: Thu, 17 Sep 2026 09:58:24 +0200 [thread overview]
Message-ID: <aqudoCAtZTgq_31Y@hovoldconsulting.com> (raw)
In-Reply-To: <CAPx+jO8WS3MqzS3txy1B3FnQ9OzRjDGzxMLLZ+Mys-jqego3_w@mail.gmail.com>
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@kernel.org> 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.
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.
> > 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.
But regardless of any potential races in mmc core, my devres comment is
a more general one: subsystems should not force devres on anyone.
Johan
next prev parent reply other threads:[~2026-09-17 7:58 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-15 13:20 [PATCH 0/2] mmc: core: Avoid export of mmc_alloc|free_host() Ulf Hansson
2026-09-15 13:20 ` [PATCH 1/2] staging: greybus: sdio: Convert to devm_mmc_alloc_host() Ulf Hansson
2026-09-15 13:47 ` Rui Miguel Silva
2026-09-16 7:37 ` Greg Kroah-Hartman
2026-09-15 13:20 ` [PATCH 2/2] mmc: core: Turn mmc_alloc|free_host() into static functions Ulf Hansson
2026-09-16 0:33 ` Shawn Lin
2026-09-16 7:43 ` Johan Hovold
2026-09-16 9:36 ` Ulf Hansson
2026-09-16 10:00 ` Johan Hovold
2026-09-16 14:12 ` Ulf Hansson
2026-09-17 7:58 ` Johan Hovold [this message]
2026-09-17 13:09 ` Ulf Hansson
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=aqudoCAtZTgq_31Y@hovoldconsulting.com \
--to=johan@kernel.org \
--cc=elder@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=greybus-dev@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=rmfrfs@gmail.com \
--cc=ulf.hansson@oss.qualcomm.com \
--cc=ulfh@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®