mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [RFC PATCH] mmc: dw_mmc: disable bh_work and stop timers on remove
@ 2026-11-01  6:37 BeaCox
  2026-11-01 10:43 ` Shawn Lin
  2026-11-02  2:29 ` [RFC PATCH v2 0/2] mmc: dw_mmc: fix teardown races " BeaCox
  0 siblings, 2 replies; 9+ messages in thread
From: BeaCox @ 2026-11-01  6:37 UTC (permalink / raw)
  To: Jaehoon Chung, Shawn Lin, Ulf Hansson; +Cc: linux-mmc, linux-kernel, stable

dw_mci_remove() does not drain bh_work before the driver core releases
the devm-managed host. The interrupt handler, watchdog, fault-injection
timer and DMA completion path can queue that work, whose callback
accesses the host and MMC request state.

After removing the MMC host and masking controller interrupts, drain
the interrupt handler and disable bh_work synchronously. Disabling the
work also rejects later queue attempts, unlike cancellation alone.
Stop both timers after the work callback has finished so that it cannot
rearm a timer after teardown has cancelled it.

The analysis, patch and validation were assisted by OpenAI Codex.

Fixes: 921c87ba3893 ("mmc: Convert from tasklet to BH workqueue")
Cc: stable@vger.kernel.org
Signed-off-by: BeaCox <0xbeac@gmail.com>
---
Testing: x86_64 object build with gcc 13.3 and W=1, with
CONFIG_FAULT_INJECTION enabled and disabled. A userspace model of the
extracted remove function checks pending work and modeled late IRQ/timer
queue attempts. It is an ordering check, not a kernel reproducer.

No matching hardware, kernel crash trace or demonstrated unprivileged
trigger is available. Please review the MMC shutdown assumptions and
external DMA callback lifetime. This is an RFC for a teardown fix; no
privilege-escalation or CVE eligibility claim is made.

 drivers/mmc/host/dw_mmc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index ff0b25d287..cc5e163ee7 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3406,6 +3406,12 @@ void dw_mci_remove(struct dw_mci *host)
 	mci_writel(host, RINTSTS, 0xFFFFFFFF);
 	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
 
+	synchronize_irq(host->irq);
+	/* Stop the work before cancelling timers that it can rearm. */
+	disable_work_sync(&host->bh_work);
+	dw_mci_stop_fault_timer(host);
+	hrtimer_cancel(&host->wd_timer);
+
 	/* disable clock to CIU */
 	mci_writel(host, CLKENA, 0);
 	mci_writel(host, CLKSRC, 0);

base-commit: cbbddb6b08cc9900a87cccaa9b22bbe06aa4f50a
-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH] mmc: dw_mmc: disable bh_work and stop timers on remove
  2026-11-01  6:37 [RFC PATCH] mmc: dw_mmc: disable bh_work and stop timers on remove BeaCox
@ 2026-11-01 10:43 ` Shawn Lin
  2026-11-02  2:26   ` BeaCox
  2026-11-02  2:29 ` [RFC PATCH v2 0/2] mmc: dw_mmc: fix teardown races " BeaCox
  1 sibling, 1 reply; 9+ messages in thread
From: Shawn Lin @ 2026-11-01 10:43 UTC (permalink / raw)
  To: BeaCox, Jaehoon Chung, Ulf Hansson
  Cc: shawn.lin, linux-mmc, linux-kernel, stable



在 2026/11/01 星期日 14:37, BeaCox 写道:
> dw_mci_remove() does not drain bh_work before the driver core releases
> the devm-managed host. The interrupt handler, watchdog, fault-injection
> timer and DMA completion path can queue that work, whose callback
> accesses the host and MMC request state.
> 
> After removing the MMC host and masking controller interrupts, drain
> the interrupt handler and disable bh_work synchronously. Disabling the
> work also rejects later queue attempts, unlike cancellation alone.
> Stop both timers after the work callback has finished so that it cannot
> rearm a timer after teardown has cancelled it.
> 
> The analysis, patch and validation were assisted by OpenAI Codex.
> 
> Fixes: 921c87ba3893 ("mmc: Convert from tasklet to BH workqueue")

The patch itself looks correct to me.

But I checked 921c87ba3893, the pre-conversion dw_mci_remove() never
called tasklet_kill() either, so the deferred-callback race predates the
tasklet-to-BH-workqueue conversion. 921c87ba3893 only replaced
tasklet_schedule() with queue_work() and removed nothing, so it did not
introduce this. Moreover, wd_timer is introduced recently, so the patch
it not backportable.

> Cc: stable@vger.kernel.org
> Signed-off-by: BeaCox <0xbeac@gmail.com>
> ---
> Testing: x86_64 object build with gcc 13.3 and W=1, with
> CONFIG_FAULT_INJECTION enabled and disabled. A userspace model of the
> extracted remove function checks pending work and modeled late IRQ/timer
> queue attempts. It is an ordering check, not a kernel reproducer.
> 
> No matching hardware, kernel crash trace or demonstrated unprivileged
> trigger is available. Please review the MMC shutdown assumptions and
> external DMA callback lifetime. This is an RFC for a teardown fix; no
> privilege-escalation or CVE eligibility claim is made.
> 
>   drivers/mmc/host/dw_mmc.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index ff0b25d287..cc5e163ee7 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -3406,6 +3406,12 @@ void dw_mci_remove(struct dw_mci *host)
>   	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>   	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
>   
> +	synchronize_irq(host->irq);
> +	/* Stop the work before cancelling timers that it can rearm. */
> +	disable_work_sync(&host->bh_work);
> +	dw_mci_stop_fault_timer(host);
> +	hrtimer_cancel(&host->wd_timer);
> +
>   	/* disable clock to CIU */
>   	mci_writel(host, CLKENA, 0);
>   	mci_writel(host, CLKSRC, 0);
> 
> base-commit: cbbddb6b08cc9900a87cccaa9b22bbe06aa4f50a


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH] mmc: dw_mmc: disable bh_work and stop timers on remove
  2026-11-01 10:43 ` Shawn Lin
@ 2026-11-02  2:26   ` BeaCox
  2026-11-02  2:32     ` Shawn Lin
  0 siblings, 1 reply; 9+ messages in thread
From: BeaCox @ 2026-11-02  2:26 UTC (permalink / raw)
  To: Shawn Lin; +Cc: Jaehoon Chung, Ulf Hansson, linux-mmc, linux-kernel, BeaCox

Hi Shawn,

Thanks for the review and for checking the history.  You are right on
both points.

dw_mci_remove() never called tasklet_kill(), so the race goes back to
the original driver (f95f3850f7a9); 921c87ba3893 only replaced
tasklet_schedule() with queue_work().  I have replaced the Fixes tag
accordingly.

I have also split the fix in two, since the two parts have different
lifetimes:

  1/2 drains bh_work.  This is the long-standing race and the patch
      applies to 6.11 and later, where the bh_work form exists; it is
      marked Cc: stable # 6.11+.
  2/2 cancels the watchdog timer.  That code is new in 7.4
      (ce0043b9a302), so there is nothing to backport it to.

The code is otherwise unchanged.  v2 follows shortly.

If you would rather keep it as a single patch without the stable tag,
just let me know and I will squash them.

Thanks,
BeaCox

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH v2 0/2] mmc: dw_mmc: fix teardown races on remove
  2026-11-01  6:37 [RFC PATCH] mmc: dw_mmc: disable bh_work and stop timers on remove BeaCox
  2026-11-01 10:43 ` Shawn Lin
@ 2026-11-02  2:29 ` BeaCox
  2026-11-02  2:29   ` [RFC PATCH v2 1/2] mmc: dw_mmc: drain bh_work on host removal BeaCox
  2026-11-02  2:29   ` [RFC PATCH v2 2/2] mmc: dw_mmc: cancel the watchdog timer on remove BeaCox
  1 sibling, 2 replies; 9+ messages in thread
From: BeaCox @ 2026-11-02  2:29 UTC (permalink / raw)
  To: Jaehoon Chung, Shawn Lin, Ulf Hansson; +Cc: linux-mmc, linux-kernel, BeaCox

This splits the dw_mci_remove() teardown fix in two, because the two
objects have different lifetimes.

1/2 drains bh_work.  The race predates the BH workqueue conversion: the
tasklet that it replaced was never killed in dw_mci_remove() either,
going back to the original driver, and commit 921c87ba3893 ("mmc:
Convert from tasklet to BH workqueue") only replaced tasklet_schedule()
with queue_work().  The patch therefore applies to 6.11 and later,
where the bh_work form exists.

2/2 cancels the central watchdog timer, which is new in 7.4
(ce0043b9a302), after the work has been disabled so that a work item
that is still running cannot rearm it.

v1 incorrectly used 921c87ba3893 as the Fixes tag; thanks to Shawn Lin
for pointing out that it did not introduce the race.  The code is
otherwise unchanged.

Testing is still limited to W=1 object builds and a userspace lifecycle
model; no hardware was available.

BeaCox (2):
  mmc: dw_mmc: drain bh_work on host removal
  mmc: dw_mmc: cancel the watchdog timer on remove

 drivers/mmc/host/dw_mmc.c | 6 ++++++
 1 file changed, 6 insertions(+)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH v2 1/2] mmc: dw_mmc: drain bh_work on host removal
  2026-11-02  2:29 ` [RFC PATCH v2 0/2] mmc: dw_mmc: fix teardown races " BeaCox
@ 2026-11-02  2:29   ` BeaCox
  2026-11-02  2:34     ` Shawn Lin
  2026-11-02  2:29   ` [RFC PATCH v2 2/2] mmc: dw_mmc: cancel the watchdog timer on remove BeaCox
  1 sibling, 1 reply; 9+ messages in thread
From: BeaCox @ 2026-11-02  2:29 UTC (permalink / raw)
  To: Jaehoon Chung, Shawn Lin, Ulf Hansson
  Cc: linux-mmc, linux-kernel, BeaCox, stable

dw_mci_remove() does not drain bh_work before the driver core releases
the devm-managed host.  The interrupt handler, the fault-injection timer
and the DMA completion path can queue that work, and its callback
accesses the host and MMC request state.  A work item that is still
pending or running after remove returns dereferences freed memory.

This is not a regression from the tasklet-to-BH-workqueue conversion.
The tasklet that bh_work replaced was never killed in dw_mci_remove()
either, going back to the original driver.  Commit 921c87ba3893
("mmc: Convert from tasklet to BH workqueue") only replaced
tasklet_schedule() with queue_work().

After removing the MMC host and masking controller interrupts, drain
the interrupt handler and disable the work synchronously.  Disabling
the work also rejects later queue attempts, unlike cancellation alone.
Stop the fault-injection timer after the work callback has finished so
that it cannot rearm the timer after teardown has cancelled it.

Fixes: f95f3850f7a9 ("mmc: dw_mmc: Add Synopsys DesignWare mmc host driver")
Cc: stable@vger.kernel.org # 6.11+
Signed-off-by: BeaCox <0xbeac@gmail.com>
---
 drivers/mmc/host/dw_mmc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index ff0b25d..5b47643 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3406,6 +3406,11 @@ void dw_mci_remove(struct dw_mci *host)
 	mci_writel(host, RINTSTS, 0xFFFFFFFF);
 	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
 
+	synchronize_irq(host->irq);
+	/* Stop the work before the timer that it can rearm. */
+	disable_work_sync(&host->bh_work);
+	dw_mci_stop_fault_timer(host);
+
 	/* disable clock to CIU */
 	mci_writel(host, CLKENA, 0);
 	mci_writel(host, CLKSRC, 0);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH v2 2/2] mmc: dw_mmc: cancel the watchdog timer on remove
  2026-11-02  2:29 ` [RFC PATCH v2 0/2] mmc: dw_mmc: fix teardown races " BeaCox
  2026-11-02  2:29   ` [RFC PATCH v2 1/2] mmc: dw_mmc: drain bh_work on host removal BeaCox
@ 2026-11-02  2:29   ` BeaCox
  2026-11-02  2:33     ` Shawn Lin
  1 sibling, 1 reply; 9+ messages in thread
From: BeaCox @ 2026-11-02  2:29 UTC (permalink / raw)
  To: Jaehoon Chung, Shawn Lin, Ulf Hansson; +Cc: linux-mmc, linux-kernel, BeaCox

The central watchdog added for command and data timeout handling is
armed while a request is in flight and is only stopped when the
completion is delivered.  If the host is removed with a request still
in flight, the timer can expire after the controller has been disabled
and its callback reads the controller registers and the host state.

Cancel the timer after bh_work has been disabled, so that a work item
that is still running cannot rearm it after the cancellation.

Fixes: ce0043b9a302 ("mmc: dw_mmc: add central watchdog and convert CTO onto it")
Signed-off-by: BeaCox <0xbeac@gmail.com>
---
 drivers/mmc/host/dw_mmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 5b47643..dc3beab 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3410,6 +3410,7 @@ void dw_mci_remove(struct dw_mci *host)
 	/* Stop the work before the timer that it can rearm. */
 	disable_work_sync(&host->bh_work);
 	dw_mci_stop_fault_timer(host);
+	hrtimer_cancel(&host->wd_timer);
 
 	/* disable clock to CIU */
 	mci_writel(host, CLKENA, 0);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH] mmc: dw_mmc: disable bh_work and stop timers on remove
  2026-11-02  2:26   ` BeaCox
@ 2026-11-02  2:32     ` Shawn Lin
  0 siblings, 0 replies; 9+ messages in thread
From: Shawn Lin @ 2026-11-02  2:32 UTC (permalink / raw)
  To: BeaCox; +Cc: shawn.lin, Jaehoon Chung, Ulf Hansson, linux-mmc, linux-kernel

在 2026/11/02 星期一 10:26, BeaCox 写道:
> Hi Shawn,
> 
> Thanks for the review and for checking the history.  You are right on
> both points.
> 
> dw_mci_remove() never called tasklet_kill(), so the race goes back to
> the original driver (f95f3850f7a9); 921c87ba3893 only replaced
> tasklet_schedule() with queue_work().  I have replaced the Fixes tag
> accordingly.
> 
> I have also split the fix in two, since the two parts have different
> lifetimes:
> 
>    1/2 drains bh_work.  This is the long-standing race and the patch
>        applies to 6.11 and later, where the bh_work form exists; it is
>        marked Cc: stable # 6.11+.
>    2/2 cancels the watchdog timer.  That code is new in 7.4
>        (ce0043b9a302), so there is nothing to backport it to.
> 
> The code is otherwise unchanged.  v2 follows shortly.
> 
> If you would rather keep it as a single patch without the stable tag,
> just let me know and I will squash them.
> 

I'm fine with split the fix in two patches.

> Thanks,
> BeaCox
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH v2 2/2] mmc: dw_mmc: cancel the watchdog timer on remove
  2026-11-02  2:29   ` [RFC PATCH v2 2/2] mmc: dw_mmc: cancel the watchdog timer on remove BeaCox
@ 2026-11-02  2:33     ` Shawn Lin
  0 siblings, 0 replies; 9+ messages in thread
From: Shawn Lin @ 2026-11-02  2:33 UTC (permalink / raw)
  To: BeaCox; +Cc: shawn.lin, linux-mmc, linux-kernel, Jaehoon Chung, Ulf Hansson

在 2026/11/02 星期一 10:29, BeaCox 写道:
> The central watchdog added for command and data timeout handling is
> armed while a request is in flight and is only stopped when the
> completion is delivered.  If the host is removed with a request still
> in flight, the timer can expire after the controller has been disabled
> and its callback reads the controller registers and the host state.
> 
> Cancel the timer after bh_work has been disabled, so that a work item
> that is still running cannot rearm it after the cancellation.
>

Thanks for this fix,

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>


> Fixes: ce0043b9a302 ("mmc: dw_mmc: add central watchdog and convert CTO onto it")
> Signed-off-by: BeaCox <0xbeac@gmail.com>
> ---
>   drivers/mmc/host/dw_mmc.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 5b47643..dc3beab 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -3410,6 +3410,7 @@ void dw_mci_remove(struct dw_mci *host)
>   	/* Stop the work before the timer that it can rearm. */
>   	disable_work_sync(&host->bh_work);
>   	dw_mci_stop_fault_timer(host);
> +	hrtimer_cancel(&host->wd_timer);
>   
>   	/* disable clock to CIU */
>   	mci_writel(host, CLKENA, 0);


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH v2 1/2] mmc: dw_mmc: drain bh_work on host removal
  2026-11-02  2:29   ` [RFC PATCH v2 1/2] mmc: dw_mmc: drain bh_work on host removal BeaCox
@ 2026-11-02  2:34     ` Shawn Lin
  0 siblings, 0 replies; 9+ messages in thread
From: Shawn Lin @ 2026-11-02  2:34 UTC (permalink / raw)
  To: BeaCox, Jaehoon Chung, Ulf Hansson
  Cc: shawn.lin, linux-mmc, linux-kernel, stable

在 2026/11/02 星期一 10:29, BeaCox 写道:
> dw_mci_remove() does not drain bh_work before the driver core releases
> the devm-managed host.  The interrupt handler, the fault-injection timer
> and the DMA completion path can queue that work, and its callback
> accesses the host and MMC request state.  A work item that is still
> pending or running after remove returns dereferences freed memory.
> 
> This is not a regression from the tasklet-to-BH-workqueue conversion.
> The tasklet that bh_work replaced was never killed in dw_mci_remove()
> either, going back to the original driver.  Commit 921c87ba3893
> ("mmc: Convert from tasklet to BH workqueue") only replaced
> tasklet_schedule() with queue_work().
> 
> After removing the MMC host and masking controller interrupts, drain
> the interrupt handler and disable the work synchronously.  Disabling
> the work also rejects later queue attempts, unlike cancellation alone.
> Stop the fault-injection timer after the work callback has finished so
> that it cannot rearm the timer after teardown has cancelled it.
> 

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Fixes: f95f3850f7a9 ("mmc: dw_mmc: Add Synopsys DesignWare mmc host driver")
> Cc: stable@vger.kernel.org # 6.11+
> Signed-off-by: BeaCox <0xbeac@gmail.com>
> ---
>   drivers/mmc/host/dw_mmc.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index ff0b25d..5b47643 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -3406,6 +3406,11 @@ void dw_mci_remove(struct dw_mci *host)
>   	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>   	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
>   
> +	synchronize_irq(host->irq);
> +	/* Stop the work before the timer that it can rearm. */
> +	disable_work_sync(&host->bh_work);
> +	dw_mci_stop_fault_timer(host);
> +
>   	/* disable clock to CIU */
>   	mci_writel(host, CLKENA, 0);
>   	mci_writel(host, CLKSRC, 0);


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-11-02  2:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-01  6:37 [RFC PATCH] mmc: dw_mmc: disable bh_work and stop timers on remove BeaCox
2026-11-01 10:43 ` Shawn Lin
2026-11-02  2:26   ` BeaCox
2026-11-02  2:32     ` Shawn Lin
2026-11-02  2:29 ` [RFC PATCH v2 0/2] mmc: dw_mmc: fix teardown races " BeaCox
2026-11-02  2:29   ` [RFC PATCH v2 1/2] mmc: dw_mmc: drain bh_work on host removal BeaCox
2026-11-02  2:34     ` Shawn Lin
2026-11-02  2:29   ` [RFC PATCH v2 2/2] mmc: dw_mmc: cancel the watchdog timer on remove BeaCox
2026-11-02  2:33     ` Shawn Lin

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®