mmc: davinci: avoid NULL deref of host->data in IRQ handler

[PATCH] mmc: davinci: avoid NULL deref of host->data in IRQ handler

Posted by Stepan Ionichev 3 weeks, 3 days ago

mmc_davinci_irq() returns early only when both host->cmd and
host->data are NULL:

  if (host->cmd == NULL && host->data == NULL) {
          ...
          return IRQ_NONE;
  }

So we may legitimately reach the rest of the handler with
host->data == NULL (and therefore data == NULL). The DATDNE branch
already guards against this with an explicit "if (data != NULL)"
check, but the subsequent TOUTRD ("read data timeout") and
CRCWR/CRCRD ("data CRC error") branches dereference data
unconditionally:

  if (qstatus & MMCST0_TOUTRD) {
          data->error = -ETIMEDOUT;        <-- NULL deref
          ...
          davinci_abort_data(host, data);
  }

  if (qstatus & (MMCST0_CRCWR | MMCST0_CRCRD)) {
          data->error = -EILSEQ;           <-- NULL deref
          ...
  }

If either bit is set in qstatus while host->data is NULL, the kernel
will crash inside the IRQ handler. smatch flags this:

  drivers/mmc/host/davinci_mmc.c:933 mmc_davinci_irq() error: we
    previously assumed 'data' could be null (see line 914)

Gate both branches on a non-NULL data, matching the existing pattern
used by the DATDNE branch.

No functional change for callers where data is non-NULL, which is
the only case in which these branches did meaningful work before
this change.

Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
 drivers/mmc/host/davinci_mmc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index 42b0118a4..42ad87aa4 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -928,7 +928,7 @@ static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
 		}
 	}
 
-	if (qstatus & MMCST0_TOUTRD) {
+	if (data && (qstatus & MMCST0_TOUTRD)) {
 		/* Read data timeout */
 		data->error = -ETIMEDOUT;
 		end_transfer = 1;
@@ -940,7 +940,7 @@ static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
 		davinci_abort_data(host, data);
 	}
 
-	if (qstatus & (MMCST0_CRCWR | MMCST0_CRCRD)) {
+	if (data && (qstatus & (MMCST0_CRCWR | MMCST0_CRCRD))) {
 		/* Data CRC error */
 		data->error = -EILSEQ;
 		end_transfer = 1;
-- 
2.43.0

Re: [PATCH] mmc: davinci: avoid NULL deref of host->data in IRQ handler

Posted by Ulf Hansson 2 weeks, 5 days ago

On Wed, 6 May 2026 at 21:05, Stepan Ionichev <sozdayvek@gmail.com> wrote:
>
> mmc_davinci_irq() returns early only when both host->cmd and
> host->data are NULL:
>
>   if (host->cmd == NULL && host->data == NULL) {
>           ...
>           return IRQ_NONE;
>   }
>
> So we may legitimately reach the rest of the handler with
> host->data == NULL (and therefore data == NULL). The DATDNE branch
> already guards against this with an explicit "if (data != NULL)"
> check, but the subsequent TOUTRD ("read data timeout") and
> CRCWR/CRCRD ("data CRC error") branches dereference data
> unconditionally:
>
>   if (qstatus & MMCST0_TOUTRD) {
>           data->error = -ETIMEDOUT;        <-- NULL deref
>           ...
>           davinci_abort_data(host, data);
>   }
>
>   if (qstatus & (MMCST0_CRCWR | MMCST0_CRCRD)) {
>           data->error = -EILSEQ;           <-- NULL deref
>           ...
>   }
>
> If either bit is set in qstatus while host->data is NULL, the kernel
> will crash inside the IRQ handler. smatch flags this:
>
>   drivers/mmc/host/davinci_mmc.c:933 mmc_davinci_irq() error: we
>     previously assumed 'data' could be null (see line 914)
>
> Gate both branches on a non-NULL data, matching the existing pattern
> used by the DATDNE branch.
>
> No functional change for callers where data is non-NULL, which is
> the only case in which these branches did meaningful work before
> this change.
>
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/davinci_mmc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
> index 42b0118a4..42ad87aa4 100644
> --- a/drivers/mmc/host/davinci_mmc.c
> +++ b/drivers/mmc/host/davinci_mmc.c
> @@ -928,7 +928,7 @@ static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
>                 }
>         }
>
> -       if (qstatus & MMCST0_TOUTRD) {
> +       if (data && (qstatus & MMCST0_TOUTRD)) {
>                 /* Read data timeout */
>                 data->error = -ETIMEDOUT;
>                 end_transfer = 1;
> @@ -940,7 +940,7 @@ static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
>                 davinci_abort_data(host, data);
>         }
>
> -       if (qstatus & (MMCST0_CRCWR | MMCST0_CRCRD)) {
> +       if (data && (qstatus & (MMCST0_CRCWR | MMCST0_CRCRD))) {
>                 /* Data CRC error */
>                 data->error = -EILSEQ;
>                 end_transfer = 1;
> --
> 2.43.0
>

Re: [PATCH] mmc: davinci: avoid NULL deref of host->data in IRQ handler

Posted by Bartosz Golaszewski 3 weeks, 2 days ago

On Wed, 6 May 2026 21:05:37 +0200, Stepan Ionichev <sozdayvek@gmail.com> said:
> mmc_davinci_irq() returns early only when both host->cmd and
> host->data are NULL:
>
>   if (host->cmd == NULL && host->data == NULL) {
>           ...
>           return IRQ_NONE;
>   }
>
> So we may legitimately reach the rest of the handler with
> host->data == NULL (and therefore data == NULL). The DATDNE branch
> already guards against this with an explicit "if (data != NULL)"
> check, but the subsequent TOUTRD ("read data timeout") and
> CRCWR/CRCRD ("data CRC error") branches dereference data
> unconditionally:
>
>   if (qstatus & MMCST0_TOUTRD) {
>           data->error = -ETIMEDOUT;        <-- NULL deref
>           ...
>           davinci_abort_data(host, data);
>   }
>
>   if (qstatus & (MMCST0_CRCWR | MMCST0_CRCRD)) {
>           data->error = -EILSEQ;           <-- NULL deref
>           ...
>   }
>
> If either bit is set in qstatus while host->data is NULL, the kernel
> will crash inside the IRQ handler. smatch flags this:
>
>   drivers/mmc/host/davinci_mmc.c:933 mmc_davinci_irq() error: we
>     previously assumed 'data' could be null (see line 914)
>
> Gate both branches on a non-NULL data, matching the existing pattern
> used by the DATDNE branch.
>
> No functional change for callers where data is non-NULL, which is
> the only case in which these branches did meaningful work before
> this change.
>
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> ---
>  drivers/mmc/host/davinci_mmc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
> index 42b0118a4..42ad87aa4 100644
> --- a/drivers/mmc/host/davinci_mmc.c
> +++ b/drivers/mmc/host/davinci_mmc.c
> @@ -928,7 +928,7 @@ static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
>  		}
>  	}
>
> -	if (qstatus & MMCST0_TOUTRD) {
> +	if (data && (qstatus & MMCST0_TOUTRD)) {
>  		/* Read data timeout */
>  		data->error = -ETIMEDOUT;
>  		end_transfer = 1;
> @@ -940,7 +940,7 @@ static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
>  		davinci_abort_data(host, data);
>  	}
>
> -	if (qstatus & (MMCST0_CRCWR | MMCST0_CRCRD)) {
> +	if (data && (qstatus & (MMCST0_CRCWR | MMCST0_CRCRD))) {
>  		/* Data CRC error */
>  		data->error = -EILSEQ;
>  		end_transfer = 1;
> --
> 2.43.0
>
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>