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

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

 



(2012/08/22 15:49), Guennadi Liakhovetski wrote:
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>

I verified on kzm9g.
This works with
[PATCH] mmc: sh-mmcif: properly handle MMC_WRITE_MULTIPLE_BLOCK completion IRQ

Tested-by: Tetsuyuki Kobayashi <koba@xxxxxxxxxxx>

---

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;


--
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