From: Ulf Hansson ulfh@kernel.org
The only remaining user of mmc_alloc|free_host() is the greybus sdio driver. Let's convert it into using the resource managed variant so we can drop the export of mmc_alloc|free_host().
Ulf Hansson (2): staging: greybus: sdio: Convert to devm_mmc_alloc_host() mmc: core: Turn mmc_alloc|free_host() into static functions
drivers/mmc/core/host.c | 35 +++++++++------------------------- drivers/staging/greybus/sdio.c | 11 +++-------- include/linux/mmc/host.h | 2 -- 3 files changed, 12 insertions(+), 36 deletions(-)
From: Ulf Hansson ulfh@kernel.org
Simplify the code by converting from mmc_alloc_host() to the resource managed devm_mmc_alloc_host().
Signed-off-by: Ulf Hansson ulfh@kernel.org --- drivers/staging/greybus/sdio.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/greybus/sdio.c b/drivers/staging/greybus/sdio.c index 3952f3d225db..642d785fe918 100644 --- a/drivers/staging/greybus/sdio.c +++ b/drivers/staging/greybus/sdio.c @@ -767,17 +767,15 @@ static int gb_sdio_probe(struct gbphy_device *gbphy_dev, struct gb_sdio_host *host; int ret = 0;
- mmc = mmc_alloc_host(sizeof(*host), &gbphy_dev->dev); + mmc = devm_mmc_alloc_host(&gbphy_dev->dev, sizeof(*host)); if (!mmc) return -ENOMEM;
connection = gb_connection_create(gbphy_dev->bundle, le16_to_cpu(gbphy_dev->cport_desc->id), gb_sdio_request_handler); - if (IS_ERR(connection)) { - ret = PTR_ERR(connection); - goto exit_mmc_free; - } + if (IS_ERR(connection)) + return PTR_ERR(connection);
host = mmc_priv(mmc); host->mmc = mmc; @@ -835,8 +833,6 @@ static int gb_sdio_probe(struct gbphy_device *gbphy_dev, gb_connection_disable(connection); exit_connection_destroy: gb_connection_destroy(connection); -exit_mmc_free: - mmc_free_host(mmc);
return ret; } @@ -863,7 +859,6 @@ static void gb_sdio_remove(struct gbphy_device *gbphy_dev) mmc_remove_host(mmc); gb_connection_disable(connection); gb_connection_destroy(connection); - mmc_free_host(mmc); }
static const struct gbphy_device_id gb_sdio_id_table[] = {
Hey Ulf, Thanks for the patch.
On Tue Sep 15, 2026 at 2:20 PM WEST, Ulf Hansson wrote:
From: Ulf Hansson ulfh@kernel.org
Simplify the code by converting from mmc_alloc_host() to the resource managed devm_mmc_alloc_host().
Signed-off-by: Ulf Hansson ulfh@kernel.org
LGTM,
Acked-by: Rui Miguel Silva rui.silva@linaro.org
Cheers, Rui
drivers/staging/greybus/sdio.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/greybus/sdio.c b/drivers/staging/greybus/sdio.c index 3952f3d225db..642d785fe918 100644 --- a/drivers/staging/greybus/sdio.c +++ b/drivers/staging/greybus/sdio.c @@ -767,17 +767,15 @@ static int gb_sdio_probe(struct gbphy_device *gbphy_dev, struct gb_sdio_host *host; int ret = 0;
- mmc = mmc_alloc_host(sizeof(*host), &gbphy_dev->dev);
- mmc = devm_mmc_alloc_host(&gbphy_dev->dev, sizeof(*host)); if (!mmc) return -ENOMEM;
connection = gb_connection_create(gbphy_dev->bundle, le16_to_cpu(gbphy_dev->cport_desc->id), gb_sdio_request_handler);
- if (IS_ERR(connection)) {
ret = PTR_ERR(connection);goto exit_mmc_free;- }
- if (IS_ERR(connection))
return PTR_ERR(connection);host = mmc_priv(mmc); host->mmc = mmc; @@ -835,8 +833,6 @@ static int gb_sdio_probe(struct gbphy_device *gbphy_dev, gb_connection_disable(connection); exit_connection_destroy: gb_connection_destroy(connection); -exit_mmc_free:
- mmc_free_host(mmc);
return ret; } @@ -863,7 +859,6 @@ static void gb_sdio_remove(struct gbphy_device *gbphy_dev) mmc_remove_host(mmc); gb_connection_disable(connection); gb_connection_destroy(connection);
- mmc_free_host(mmc);
} static const struct gbphy_device_id gb_sdio_id_table[] = { -- 2.43.0
On Tue, Sep 15, 2026 at 03:20:45PM +0200, Ulf Hansson wrote:
From: Ulf Hansson ulfh@kernel.org
Simplify the code by converting from mmc_alloc_host() to the resource managed devm_mmc_alloc_host().
Signed-off-by: Ulf Hansson ulfh@kernel.org
drivers/staging/greybus/sdio.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
Acked-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
From: Ulf Hansson ulfh@kernel.org
As there are no longer any users of these functions, let's make them internal to the mmc core. While at it, let's also flip the order of the in-parameters to mmc_alloc_host() to be consistent with devm_alloc_host().
Signed-off-by: Ulf Hansson ulfh@kernel.org --- drivers/mmc/core/host.c | 35 +++++++++-------------------------- include/linux/mmc/host.h | 2 -- 2 files changed, 9 insertions(+), 28 deletions(-)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c index 1bcf0e917b59..8e9e61839198 100644 --- a/drivers/mmc/core/host.c +++ b/drivers/mmc/core/host.c @@ -505,14 +505,7 @@ static int mmc_first_nonreserved_index(void) return max + 1; }
-/** - * mmc_alloc_host - initialise the per-host structure. - * @extra: sizeof private data structure - * @dev: pointer to host device model structure - * - * Initialise the per-host structure. - */ -struct mmc_host *mmc_alloc_host(int extra, struct device *dev) +static struct mmc_host *mmc_alloc_host(struct device *dev, int extra) { int index; struct mmc_host *host; @@ -583,7 +576,13 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev) return host; }
-EXPORT_SYMBOL(mmc_alloc_host); +static void mmc_free_host(struct mmc_host *host) +{ + cancel_delayed_work_sync(&host->detect); + cancel_work_sync(&host->sdio_irq_work); + mmc_pwrseq_free(host); + put_device(&host->class_dev); +}
static void devm_mmc_host_release(struct device *dev, void *res) { @@ -598,7 +597,7 @@ struct mmc_host *devm_mmc_alloc_host(struct device *dev, int extra) if (!dr) return NULL;
- host = mmc_alloc_host(extra, dev); + host = mmc_alloc_host(dev, extra); if (!host) { devres_free(dr); return NULL; @@ -692,19 +691,3 @@ void mmc_remove_host(struct mmc_host *host) }
EXPORT_SYMBOL(mmc_remove_host); - -/** - * mmc_free_host - free the host structure - * @host: mmc host - * - * Free the host once all references to it have been dropped. - */ -void mmc_free_host(struct mmc_host *host) -{ - cancel_delayed_work_sync(&host->detect); - cancel_work_sync(&host->sdio_irq_work); - mmc_pwrseq_free(host); - put_device(&host->class_dev); -} - -EXPORT_SYMBOL(mmc_free_host); diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index ddb32bc2946f..198554e48346 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -585,11 +585,9 @@ struct mmc_host {
struct device_node;
-struct mmc_host *mmc_alloc_host(int extra, struct device *); struct mmc_host *devm_mmc_alloc_host(struct device *dev, int extra); int mmc_add_host(struct mmc_host *); void mmc_remove_host(struct mmc_host *); -void mmc_free_host(struct mmc_host *); void mmc_of_parse_clk_phase(struct device *dev, struct mmc_clk_phase_map *map); int mmc_of_parse(struct mmc_host *host);
在 2026/09/15 星期二 21:20, Ulf Hansson 写道:
From: Ulf Hansson ulfh@kernel.org
As there are no longer any users of these functions, let's make them internal to the mmc core. While at it, let's also flip the order of the in-parameters to mmc_alloc_host() to be consistent with devm_alloc_host().
Nice move when vub300 was gone,
Reviewed-by: Shawn Lin shawn.lin@linux.dev
Signed-off-by: Ulf Hansson ulfh@kernel.org
drivers/mmc/core/host.c | 35 +++++++++-------------------------- include/linux/mmc/host.h | 2 -- 2 files changed, 9 insertions(+), 28 deletions(-)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c index 1bcf0e917b59..8e9e61839198 100644 --- a/drivers/mmc/core/host.c +++ b/drivers/mmc/core/host.c @@ -505,14 +505,7 @@ static int mmc_first_nonreserved_index(void) return max + 1; } -/**
- mmc_alloc_host - initialise the per-host structure.
- @extra: sizeof private data structure
- @dev: pointer to host device model structure
- Initialise the per-host structure.
- */
-struct mmc_host *mmc_alloc_host(int extra, struct device *dev) +static struct mmc_host *mmc_alloc_host(struct device *dev, int extra) { int index; struct mmc_host *host; @@ -583,7 +576,13 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev) return host; } -EXPORT_SYMBOL(mmc_alloc_host); +static void mmc_free_host(struct mmc_host *host) +{
- cancel_delayed_work_sync(&host->detect);
- cancel_work_sync(&host->sdio_irq_work);
- mmc_pwrseq_free(host);
- put_device(&host->class_dev);
+} static void devm_mmc_host_release(struct device *dev, void *res) { @@ -598,7 +597,7 @@ struct mmc_host *devm_mmc_alloc_host(struct device *dev, int extra) if (!dr) return NULL;
- host = mmc_alloc_host(extra, dev);
- host = mmc_alloc_host(dev, extra); if (!host) { devres_free(dr); return NULL;
@@ -692,19 +691,3 @@ void mmc_remove_host(struct mmc_host *host) } EXPORT_SYMBOL(mmc_remove_host);
-/**
- mmc_free_host - free the host structure
- @host: mmc host
- Free the host once all references to it have been dropped.
- */
-void mmc_free_host(struct mmc_host *host) -{
- cancel_delayed_work_sync(&host->detect);
- cancel_work_sync(&host->sdio_irq_work);
- mmc_pwrseq_free(host);
- put_device(&host->class_dev);
-}
-EXPORT_SYMBOL(mmc_free_host); diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index ddb32bc2946f..198554e48346 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -585,11 +585,9 @@ struct mmc_host { struct device_node; -struct mmc_host *mmc_alloc_host(int extra, struct device *); struct mmc_host *devm_mmc_alloc_host(struct device *dev, int extra); int mmc_add_host(struct mmc_host *); void mmc_remove_host(struct mmc_host *); -void mmc_free_host(struct mmc_host *); void mmc_of_parse_clk_phase(struct device *dev, struct mmc_clk_phase_map *map); int mmc_of_parse(struct mmc_host *host);
On Tue, Sep 15, 2026 at 03:20:46PM +0200, Ulf Hansson wrote:
From: Ulf Hansson ulfh@kernel.org
As there are no longer any users of these functions, let's make them internal to the mmc core.
Why would you want to do that? The devres helpers should just be simple wrappers around these and sometimes devres just isn't a good fit.
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.
This may even be needed for greybus which currently destroys the connection before freeing the host.
I suggest you just keep the non-devres interface around (after updating the prototype).
While at it, let's also flip the order of the in-parameters to mmc_alloc_host() to be consistent with devm_alloc_host().
Johan
On Wed, Sep 16, 2026 at 9:43 AM Johan Hovold johan@kernel.org wrote:
On Tue, Sep 15, 2026 at 03:20:46PM +0200, Ulf Hansson wrote:
From: Ulf Hansson ulfh@kernel.org
As there are no longer any users of these functions, let's make them internal to the mmc core.
Why would you want to do that? The devres helpers should just be simple wrappers around these and sometimes devres just isn't a good fit.
At the moment there seems to be no need for them. I would rather keep API/interfaces as simple as possible, so I prefer to remove them at this point.
If we see a need for them, we can always add them back.
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?
This may even be needed for greybus which currently destroys the connection before freeing the host.
I looked closer at gb_sdio_remove() (the greybus sdio driver's ->remove() callback) and I think the problem isn't about freeing the host.
Instead it seems like the call to mmc_remove_host() is done too late. To me it looks like when the mmc core tries to power off the card gracefully, through mmc_remove_host() the driver has already moved into a state where it no longer accepts any requests.
I suggest you just keep the non-devres interface around (after updating the prototype).
While at it, let's also flip the order of the in-parameters to mmc_alloc_host() to be consistent with devm_alloc_host().
Johan
Kind regards Uffe
On Wed, Sep 16, 2026 at 11:36:25AM +0200, Ulf Hansson wrote:
On Wed, Sep 16, 2026 at 9:43 AM Johan Hovold johan@kernel.org wrote:
On Tue, Sep 15, 2026 at 03:20:46PM +0200, Ulf Hansson wrote:
From: Ulf Hansson ulfh@kernel.org
As there are no longer any users of these functions, let's make them internal to the mmc core.
Why would you want to do that? The devres helpers should just be simple wrappers around these and sometimes devres just isn't a good fit.
At the moment there seems to be no need for them. I would rather keep API/interfaces as simple as possible, so I prefer to remove them at this point.
If we see a need for them, we can always add them back.
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]
If such a driver also has non-devres managed resources that are freed before the work is cancelled you have a use-after-free.
This may even be needed for greybus which currently destroys the connection before freeing the host.
I looked closer at gb_sdio_remove() (the greybus sdio driver's ->remove() callback) and I think the problem isn't about freeing the host.
Instead it seems like the call to mmc_remove_host() is done too late. To me it looks like when the mmc core tries to power off the card gracefully, through mmc_remove_host() the driver has already moved into a state where it no longer accepts any requests.
Yes, that looks wrong, but that's a separate issue.
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.
Johan
[1] https://lore.kernel.org/lkml/ap7CaVj82BJZgjf6@hovoldconsulting.com/
On Wed, Sep 16, 2026 at 12:00 PM Johan Hovold johan@kernel.org wrote:
On Wed, Sep 16, 2026 at 11:36:25AM +0200, Ulf Hansson wrote:
On Wed, Sep 16, 2026 at 9:43 AM Johan Hovold johan@kernel.org wrote:
On Tue, Sep 15, 2026 at 03:20:46PM +0200, Ulf Hansson wrote:
From: Ulf Hansson ulfh@kernel.org
As there are no longer any users of these functions, let's make them internal to the mmc core.
Why would you want to do that? The devres helpers should just be simple wrappers around these and sometimes devres just isn't a good fit.
At the moment there seems to be no need for them. I would rather keep API/interfaces as simple as possible, so I prefer to remove them at this point.
If we see a need for them, we can always add them back.
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.
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.
If you are referring to an internal work for the host, yes that needs to be managed correctly. Although, that doesn't matter whether the mmc host has been allocated with the resource managed variant or not.
This may even be needed for greybus which currently destroys the connection before freeing the host.
I looked closer at gb_sdio_remove() (the greybus sdio driver's ->remove() callback) and I think the problem isn't about freeing the host.
Instead it seems like the call to mmc_remove_host() is done too late. To me it looks like when the mmc core tries to power off the card gracefully, through mmc_remove_host() the driver has already moved into a state where it no longer accepts any requests.
Yes, that looks wrong, but that's a separate issue.
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.
For the mmc_rescan() work to access the host internal data (for potential causing a use-after-free), the host must have been "started". See mmc_start|stop_host().
Johan
[1] https://lore.kernel.org/lkml/ap7CaVj82BJZgjf6@hovoldconsulting.com/
Kind regards Uffe
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
On Thu, Sep 17, 2026 at 9:58 AM Johan Hovold johan@kernel.org 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@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.
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
-
Greg Kroah-Hartman -
Johan Hovold -
Rui Miguel Silva -
Shawn Lin -
Ulf Hansson