[PATCH] mmc: sh-mmcif: avoid Oops on spurious interrupts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On some systems, e.g., kzm9g, MMCIF interfaces can produce spurious 
interrupts without any active request. To prevent the Oops, that results 
in such cases, don't dereference the mmc request pointer until we make 
sure, that we are indeed processing such a request.

Reported-by: Tetsuyuki Kobayashi <koba@xxxxxxxxxxx>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
---

Hello Kobayashi-san

On Mon, 20 Aug 2012, Tetsuyuki Kobayashi wrote:

...

> After applying this patch on kzm9g board, I got this error regarding eMMC.
> I think this is another problem.
> 
> 
> Unable to handle kernel NULL pointer dereference at virtual address 00000008
> pgd = c0004000
> [00000008] *pgd=00000000
> Internal error: Oops: 17 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 1    Not tainted  (3.6.0-rc2+ #103)
> PC is at sh_mmcif_irqt+0x20/0xb30
> LR is at irq_thread+0x94/0x16c

[snip]

> My quick fix is below.
> 
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index 5d81427..e587fbc 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -1104,7 +1104,15 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
>  {
>         struct sh_mmcif_host *host = dev_id;
>         struct mmc_request *mrq = host->mrq;
> -       struct mmc_data *data = mrq->data;
> +       /*struct mmc_data *data = mrq->data; -- this cause null pointer access*/
> +       struct mmc_data *data;
> +
> +       /* quick fix by koba */
> +       if (mrq == NULL) {
> +               printk("sh_mmcif_irqt: mrq == NULL: host->wait_for=%d\n", host->wait_for);
> +       } else {
> +               data = mrq->data;
> +       }
> 
>         cancel_delayed_work_sync(&host->timeout_work);
> 
> 
> With this patch, there is no null pointer accesses and got this log.
> 
> sh_mmcif_irqt: mrq == NULL: host->wait_for=0
> sh_mmcif_irqt: mrq == NULL: host->wait_for=0
>   ...
> 
> host->wait_for is 0. it is MMCIF_WAIT_FOR_REQUEST.
> There is code such like:
> 
>        host->wait_for = MMCIF_WAIT_FOR_REQUEST;
>        host->mrq = NULL;
> 
> So, at the top of sh_mmcif_irqt, if host->wait_for == MMCIF_WAIT_FOR_REQUEST,
> host->mrq = NULL. 
> It is too earlier to access mrq->data before checking host->mrq. it may
> cause null pointer access.
> 
> Goda-san, could you check this and refine the code of sh_mmcif_irqt?

Thanks for your report and a fix. Could you please double-check, whether 
the below patch also fixes your problem? Since such spurious interrupts 
are possible I would commit a check like this one, but in the longer run 
we want to identify and eliminate them, if possible. But since so far 
these interrupts only happen on 1 board model and also not on all units 
and not upon each boot, this could be a bit tricky.

One more question - is this only needed for 3.7 or also for 3.6 / stable?

Thanks
Guennadi

 drivers/mmc/host/sh_mmcif.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 5d81427..82bf921 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1104,7 +1104,6 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 {
 	struct sh_mmcif_host *host = dev_id;
 	struct mmc_request *mrq = host->mrq;
-	struct mmc_data *data = mrq->data;
 
 	cancel_delayed_work_sync(&host->timeout_work);
 
@@ -1152,13 +1151,14 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 	case MMCIF_WAIT_FOR_READ_END:
 	case MMCIF_WAIT_FOR_WRITE_END:
 		if (host->sd_error)
-			data->error = sh_mmcif_error_manage(host);
+			mrq->data->error = sh_mmcif_error_manage(host);
 		break;
 	default:
 		BUG();
 	}
 
 	if (host->wait_for != MMCIF_WAIT_FOR_STOP) {
+		struct mmc_data *data = mrq->data;
 		if (!mrq->cmd->error && data && !data->error)
 			data->bytes_xfered =
 				data->blocks * data->blksz;
-- 
1.7.2.5

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux