From patchwork Wed Aug 22 06:49:47 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guennadi Liakhovetski X-Patchwork-Id: 1359691 Return-Path: X-Original-To: patchwork-linux-mmc@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 63517DF280 for ; Wed, 22 Aug 2012 06:50:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752713Ab2HVGuJ (ORCPT ); Wed, 22 Aug 2012 02:50:09 -0400 Received: from moutng.kundenserver.de ([212.227.17.8]:53116 "EHLO moutng.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752191Ab2HVGuH (ORCPT ); Wed, 22 Aug 2012 02:50:07 -0400 Received: from axis700.grange (dslb-146-060-120-067.pools.arcor-ip.net [146.60.120.67]) by mrelayeu.kundenserver.de (node=mrbap3) with ESMTP (Nemesis) id 0LnSda-1TaQ2X3wv0-00hak4; Wed, 22 Aug 2012 08:49:48 +0200 Received: by axis700.grange (Postfix, from userid 1000) id 4A3A5189B0D; Wed, 22 Aug 2012 08:49:47 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by axis700.grange (Postfix) with ESMTP id 47957189AF7; Wed, 22 Aug 2012 08:49:47 +0200 (CEST) Date: Wed, 22 Aug 2012 08:49:47 +0200 (CEST) From: Guennadi Liakhovetski X-X-Sender: lyakh@axis700.grange To: Tetsuyuki Kobayashi cc: yusuke.goda.sx@renesas.com, Kuninori Morimoto , Paul Mundt , Magnus , linux-sh@vger.kernel.org, Kuninori Morimoto , linux-mmc@vger.kernel.org Subject: [PATCH] mmc: sh-mmcif: avoid Oops on spurious interrupts In-Reply-To: <5031D9FF.8060801@kmckk.co.jp> Message-ID: References: <878vdxd3mq.wl%kuninori.morimoto.gx@renesas.com> <20120803050039.GA1614@linux-sh.org> <20120809042844.GF1614@linux-sh.org> <87hasc3bv5.wl%kuninori.morimoto.gx@renesas.com> <874nobqntv.wl%kuninori.morimoto.gx@renesas.com> <20120810123804.GK1614@linux-sh.org> <502DDC97.5080501@kmckk.co.jp> <87wr0us6tg.wl%kuninori.morimoto.gx@renesas.com> <20120820031352.GC25767@linux-sh.org> <87obm6ry98.wl%kuninori.morimoto.gx@renesas.com> <20120820043853.GD25767@linux-sh.org> <87mx1qrx1x.wl%kuninori.morimoto.gx@renesas.com> <5031D9FF.8060801@kmckk.co.jp> MIME-Version: 1.0 X-Provags-ID: V02:K0:Muvq+oDqXaKcFdAHJapYVub7QLYObks0kMDa/j98yMm wtWkbL1VFwR3ACruG2KLnBV6r0p/6M+LkpDurqBsAQiuw1I4Cs CsMmHkrvNVma+TkTa3KIvEm0OvV72CBeaXo4R9a3571h8CKR4T 7zFismnIc2JvnJwMaNxem2JSN9ZVH0tj/gV4LYGT5CrDBmC+ka ooPUnjdUNWxHWMaSrdCgYJXn96wwdNv+6VOvs+LE+9F9wIXuwu AMhjUHw0MZNexDN72tMWVxLlsJyLeDap3Q1sYneT3PwsS68uiQ AqaZiDPJYx2Kiulrp0w9jpimRDxGMy3Pt6Wqb5IVsV02pahDae 7U16pTXGu++t0gqqgCPiszElu8Jj8/oRPMABTmDKptfyi3oRrV A+ZO1n7bMmSDQ== Sender: linux-mmc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org 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 Signed-off-by: Guennadi Liakhovetski Tested-by: Tetsuyuki Kobayashi --- 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;