pxamci_dma_irq calls pxamci_data_done while holding host->lock, but
pxamci_data_done is also called from the IRQ handler without the
lock held and completes MMC requests.

Calling into the MMC core under a spinlock breaks the locking contract
and may lead to re-entrancy issues.

Drop the host lock before completing the request in the DMA error path.

Found by Coverity: CID#1658802

Signed-off-by: Ahmet Sezgin Duran <ahmet@sezginduran.net>
---
 drivers/mmc/host/pxamci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index b5ea058ed467..6cf65aca2449 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -551,7 +551,9 @@ static void pxamci_dma_irq(void *param)
 		pr_err("%s: DMA error on %s channel\n", mmc_hostname(host->mmc),
 			host->data->flags & MMC_DATA_READ ? "rx" : "tx");
 		host->data->error = -EIO;
+		spin_unlock_irqrestore(&host->lock, flags);
 		pxamci_data_done(host, 0);
+		return;
 	}
 
 out_unlock:
-- 
2.52.0

On Sat, 10 Jan 2026 at 15:44, Ahmet Sezgin Duran <ahmet@sezginduran.net> wrote:
>
> pxamci_dma_irq calls pxamci_data_done while holding host->lock, but
> pxamci_data_done is also called from the IRQ handler without the
> lock held and completes MMC requests.
>
> Calling into the MMC core under a spinlock breaks the locking contract
> and may lead to re-entrancy issues.
>
> Drop the host lock before completing the request in the DMA error path.
>
> Found by Coverity: CID#1658802
>
> Signed-off-by: Ahmet Sezgin Duran <ahmet@sezginduran.net>

Applied for next and by adding a stable-tag, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/pxamci.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
> index b5ea058ed467..6cf65aca2449 100644
> --- a/drivers/mmc/host/pxamci.c
> +++ b/drivers/mmc/host/pxamci.c
> @@ -551,7 +551,9 @@ static void pxamci_dma_irq(void *param)
>                 pr_err("%s: DMA error on %s channel\n", mmc_hostname(host->mmc),
>                         host->data->flags & MMC_DATA_READ ? "rx" : "tx");
>                 host->data->error = -EIO;
> +               spin_unlock_irqrestore(&host->lock, flags);
>                 pxamci_data_done(host, 0);
> +               return;
>         }
>
>  out_unlock:
> --
> 2.52.0
>

On Thu, 22 Jan 2026 at 19:03, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Sat, 10 Jan 2026 at 15:44, Ahmet Sezgin Duran <ahmet@sezginduran.net> wrote:
> >
> > pxamci_dma_irq calls pxamci_data_done while holding host->lock, but
> > pxamci_data_done is also called from the IRQ handler without the
> > lock held and completes MMC requests.
> >
> > Calling into the MMC core under a spinlock breaks the locking contract
> > and may lead to re-entrancy issues.
> >
> > Drop the host lock before completing the request in the DMA error path.
> >
> > Found by Coverity: CID#1658802
> >
> > Signed-off-by: Ahmet Sezgin Duran <ahmet@sezginduran.net>
>
> Applied for next and by adding a stable-tag, thanks!

I was too quick here. Is this really a problem that you have tested or
just something that you believe is the right thing to do?

Other drivers are holding spinlocks too when they call the mmc core to
complete requests. In principle it depends on how the host implements
locking, whether it's fine or not. For pxamci, I think it should be
fine to keep the lock held as the core will only kick a completion
variable in this case.

That said, I am going to drop this

[...]

Kind regards
Uffe

On 1/23/26 2:58 PM, Ulf Hansson wrote:

 > I was too quick here. Is this really a problem that you have tested or
 > just something that you believe is the right thing to do?

The latter. I thought this could be an issue and tried to handle it 
defensively.

 > Other drivers are holding spinlocks too when they call the mmc core to
 > complete requests. In principle it depends on how the host implements
 > locking, whether it's fine or not. For pxamci, I think it should be
 > fine to keep the lock held as the core will only kick a completion
 > variable in this case.
 >
 > That said, I am going to drop this

Got it, thanks for the clarification.

Regards,

Ahmet Sezgin Duran