[PATCH] mmc: moxart: fix potential use-after-free on remove path.

[PATCH] mmc: moxart: fix potential use-after-free on remove path.

[Greg Kroah-Hartman]

Just like in commit bd2db32e7c3e ("moxart: fix potential use-after-free
on remove path"), we should wait until after we are finished writing to
the mmc host device before removing it, otherwise it could have been
already freed.

Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_2000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/mmc/host/moxart-mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index 3dd8f232052f..256e16390ef3 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -690,12 +690,12 @@ static void moxart_remove(struct platform_device *pdev)
 		dma_release_channel(host->dma_chan_tx);
 	if (!IS_ERR_OR_NULL(host->dma_chan_rx))
 		dma_release_channel(host->dma_chan_rx);
-	mmc_remove_host(mmc);
 
 	writel(0, host->base + REG_INTERRUPT_MASK);
 	writel(0, host->base + REG_POWER_CONTROL);
 	writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
 	       host->base + REG_CLOCK_CONTROL);
+	mmc_remove_host(mmc);
 }
 
 static const struct of_device_id moxart_mmc_match[] = {
-- 
2.53.0

Re: [PATCH] mmc: moxart: fix potential use-after-free on remove path.

[Ulf Hansson]

On Mon, 23 Feb 2026 at 14:48, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> Just like in commit bd2db32e7c3e ("moxart: fix potential use-after-free
> on remove path"), we should wait until after we are finished writing to
> the mmc host device before removing it, otherwise it could have been
> already freed.

mmc_remove_host() doesn't actually free the host, but it reverses what
mmc_add_host() did during probe.

Since the moxart driver uses devm_mmc_alloc_host() the last reference
to the host will be dropped after ->remove() completes, leading to
mmc_free_host() to be called for it.

However, improvements can still be made in the ->remove() callback. See below.

>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: stable <stable@kernel.org>
> Assisted-by: gkh_clanker_2000

What's this?

> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/mmc/host/moxart-mmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
> index 3dd8f232052f..256e16390ef3 100644
> --- a/drivers/mmc/host/moxart-mmc.c
> +++ b/drivers/mmc/host/moxart-mmc.c
> @@ -690,12 +690,12 @@ static void moxart_remove(struct platform_device *pdev)
>                 dma_release_channel(host->dma_chan_tx);
>         if (!IS_ERR_OR_NULL(host->dma_chan_rx))
>                 dma_release_channel(host->dma_chan_rx);
> -       mmc_remove_host(mmc);
>
>         writel(0, host->base + REG_INTERRUPT_MASK);
>         writel(0, host->base + REG_POWER_CONTROL);
>         writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
>                host->base + REG_CLOCK_CONTROL);
> +       mmc_remove_host(mmc);

Rather than moving this to the bottom of the function, it would be
more correct to move it to the beginning.

This way, we ensure things have been closed down properly before
releasing the dma channels.

>  }
>
>  static const struct of_device_id moxart_mmc_match[] = {
> --
> 2.53.0
>

Kind regards
Uffe

Re: [PATCH] mmc: moxart: fix potential use-after-free on remove path.

[Greg Kroah-Hartman]

On Wed, Mar 04, 2026 at 05:25:25PM +0100, Ulf Hansson wrote:
> On Mon, 23 Feb 2026 at 14:48, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > Just like in commit bd2db32e7c3e ("moxart: fix potential use-after-free
> > on remove path"), we should wait until after we are finished writing to
> > the mmc host device before removing it, otherwise it could have been
> > already freed.
> 
> mmc_remove_host() doesn't actually free the host, but it reverses what
> mmc_add_host() did during probe.
> 
> Since the moxart driver uses devm_mmc_alloc_host() the last reference
> to the host will be dropped after ->remove() completes, leading to
> mmc_free_host() to be called for it.

Then how did commit bd2db32e7c3e ("moxart: fix potential use-after-free
on remove path") do anything?  It really wasn't needed either?  And so
the CVE related to it should be rejected?

> However, improvements can still be made in the ->remove() callback. See below.
> 
> >
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Cc: stable <stable@kernel.org>
> > Assisted-by: gkh_clanker_2000
> 
> What's this?

My assorted hacks of scripts that found this issue.

> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  drivers/mmc/host/moxart-mmc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
> > index 3dd8f232052f..256e16390ef3 100644
> > --- a/drivers/mmc/host/moxart-mmc.c
> > +++ b/drivers/mmc/host/moxart-mmc.c
> > @@ -690,12 +690,12 @@ static void moxart_remove(struct platform_device *pdev)
> >                 dma_release_channel(host->dma_chan_tx);
> >         if (!IS_ERR_OR_NULL(host->dma_chan_rx))
> >                 dma_release_channel(host->dma_chan_rx);
> > -       mmc_remove_host(mmc);
> >
> >         writel(0, host->base + REG_INTERRUPT_MASK);
> >         writel(0, host->base + REG_POWER_CONTROL);
> >         writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
> >                host->base + REG_CLOCK_CONTROL);
> > +       mmc_remove_host(mmc);
> 
> Rather than moving this to the bottom of the function, it would be
> more correct to move it to the beginning.
> 
> This way, we ensure things have been closed down properly before
> releasing the dma channels.

Ok, but I was just trying to follow the same pattern in the above
mentioned commit.  If that pattern was not actually fixing something,
then this change also doesn't do anything, so it's not needed either.

thanks,

greg k-h

Re: [PATCH] mmc: moxart: fix potential use-after-free on remove path.

[Ulf Hansson]

On Sun, 8 Mar 2026 at 19:03, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Mar 04, 2026 at 05:25:25PM +0100, Ulf Hansson wrote:
> > On Mon, 23 Feb 2026 at 14:48, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > Just like in commit bd2db32e7c3e ("moxart: fix potential use-after-free
> > > on remove path"), we should wait until after we are finished writing to
> > > the mmc host device before removing it, otherwise it could have been
> > > already freed.
> >
> > mmc_remove_host() doesn't actually free the host, but it reverses what
> > mmc_add_host() did during probe.
> >
> > Since the moxart driver uses devm_mmc_alloc_host() the last reference
> > to the host will be dropped after ->remove() completes, leading to
> > mmc_free_host() to be called for it.
>
> Then how did commit bd2db32e7c3e ("moxart: fix potential use-after-free
> on remove path") do anything?  It really wasn't needed either?  And so
> the CVE related to it should be rejected?

No, commit bd2db32e7c3e is perfectly okay and solves the intended problem.

Before the moxart driver was converted to use devm_mmc_alloc_host() in
commit 973aa22b9f1a, it used mmc_alloc_host() during probe.

Calling mmc_free_host() is needed in these cases, but it should
typically be the final thing a ->remove() callback does, in order to
clean up correctly and prevent use-after-free bugs.

[...]

Kind regards
Uffe

Re: [PATCH] mmc: moxart: fix potential use-after-free on remove path.

[Greg Kroah-Hartman]

On Mon, Mar 09, 2026 at 12:42:53PM +0100, Ulf Hansson wrote:
> On Sun, 8 Mar 2026 at 19:03, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Mar 04, 2026 at 05:25:25PM +0100, Ulf Hansson wrote:
> > > On Mon, 23 Feb 2026 at 14:48, Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > Just like in commit bd2db32e7c3e ("moxart: fix potential use-after-free
> > > > on remove path"), we should wait until after we are finished writing to
> > > > the mmc host device before removing it, otherwise it could have been
> > > > already freed.
> > >
> > > mmc_remove_host() doesn't actually free the host, but it reverses what
> > > mmc_add_host() did during probe.
> > >
> > > Since the moxart driver uses devm_mmc_alloc_host() the last reference
> > > to the host will be dropped after ->remove() completes, leading to
> > > mmc_free_host() to be called for it.
> >
> > Then how did commit bd2db32e7c3e ("moxart: fix potential use-after-free
> > on remove path") do anything?  It really wasn't needed either?  And so
> > the CVE related to it should be rejected?
> 
> No, commit bd2db32e7c3e is perfectly okay and solves the intended problem.
> 
> Before the moxart driver was converted to use devm_mmc_alloc_host() in
> commit 973aa22b9f1a, it used mmc_alloc_host() during probe.

Ah, that makes more sense, thanks, I was confused.

> Calling mmc_free_host() is needed in these cases, but it should
> typically be the final thing a ->remove() callback does, in order to
> clean up correctly and prevent use-after-free bugs.

I moved the call to be the "final thing" in these patches, so they are
ok?

thanks,

greg k-h

Re: [PATCH] mmc: moxart: fix potential use-after-free on remove path.

[Ulf Hansson]

On Mon, 9 Mar 2026 at 13:24, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Mar 09, 2026 at 12:42:53PM +0100, Ulf Hansson wrote:
> > On Sun, 8 Mar 2026 at 19:03, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Wed, Mar 04, 2026 at 05:25:25PM +0100, Ulf Hansson wrote:
> > > > On Mon, 23 Feb 2026 at 14:48, Greg Kroah-Hartman
> > > > <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > Just like in commit bd2db32e7c3e ("moxart: fix potential use-after-free
> > > > > on remove path"), we should wait until after we are finished writing to
> > > > > the mmc host device before removing it, otherwise it could have been
> > > > > already freed.
> > > >
> > > > mmc_remove_host() doesn't actually free the host, but it reverses what
> > > > mmc_add_host() did during probe.
> > > >
> > > > Since the moxart driver uses devm_mmc_alloc_host() the last reference
> > > > to the host will be dropped after ->remove() completes, leading to
> > > > mmc_free_host() to be called for it.
> > >
> > > Then how did commit bd2db32e7c3e ("moxart: fix potential use-after-free
> > > on remove path") do anything?  It really wasn't needed either?  And so
> > > the CVE related to it should be rejected?
> >
> > No, commit bd2db32e7c3e is perfectly okay and solves the intended problem.
> >
> > Before the moxart driver was converted to use devm_mmc_alloc_host() in
> > commit 973aa22b9f1a, it used mmc_alloc_host() during probe.
>
> Ah, that makes more sense, thanks, I was confused.
>
> > Calling mmc_free_host() is needed in these cases, but it should
> > typically be the final thing a ->remove() callback does, in order to
> > clean up correctly and prevent use-after-free bugs.
>
> I moved the call to be the "final thing" in these patches, so they are
> ok?

No, mmc_free_host() should be in the end (unless the devm* variant is
used in probe), while mmc_remove_host() should be in the beginning.

Kind regards
Uffe

Re: [PATCH] mmc: moxart: fix potential use-after-free on remove path.

[Greg Kroah-Hartman]

On Mon, Mar 09, 2026 at 01:29:24PM +0100, Ulf Hansson wrote:
> On Mon, 9 Mar 2026 at 13:24, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Mar 09, 2026 at 12:42:53PM +0100, Ulf Hansson wrote:
> > > On Sun, 8 Mar 2026 at 19:03, Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Wed, Mar 04, 2026 at 05:25:25PM +0100, Ulf Hansson wrote:
> > > > > On Mon, 23 Feb 2026 at 14:48, Greg Kroah-Hartman
> > > > > <gregkh@linuxfoundation.org> wrote:
> > > > > >
> > > > > > Just like in commit bd2db32e7c3e ("moxart: fix potential use-after-free
> > > > > > on remove path"), we should wait until after we are finished writing to
> > > > > > the mmc host device before removing it, otherwise it could have been
> > > > > > already freed.
> > > > >
> > > > > mmc_remove_host() doesn't actually free the host, but it reverses what
> > > > > mmc_add_host() did during probe.
> > > > >
> > > > > Since the moxart driver uses devm_mmc_alloc_host() the last reference
> > > > > to the host will be dropped after ->remove() completes, leading to
> > > > > mmc_free_host() to be called for it.
> > > >
> > > > Then how did commit bd2db32e7c3e ("moxart: fix potential use-after-free
> > > > on remove path") do anything?  It really wasn't needed either?  And so
> > > > the CVE related to it should be rejected?
> > >
> > > No, commit bd2db32e7c3e is perfectly okay and solves the intended problem.
> > >
> > > Before the moxart driver was converted to use devm_mmc_alloc_host() in
> > > commit 973aa22b9f1a, it used mmc_alloc_host() during probe.
> >
> > Ah, that makes more sense, thanks, I was confused.
> >
> > > Calling mmc_free_host() is needed in these cases, but it should
> > > typically be the final thing a ->remove() callback does, in order to
> > > clean up correctly and prevent use-after-free bugs.
> >
> > I moved the call to be the "final thing" in these patches, so they are
> > ok?
> 
> No, mmc_free_host() should be in the end (unless the devm* variant is
> used in probe), while mmc_remove_host() should be in the beginning.

Ok, I think I understand.  I'll respin these two patches based on that
when I get a chance later this week.

thanks for your patience,

greg k-h