linux-kernel - Re: [PATCH resent] lis3: Fix Oops with NULL platform data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 22 Sep 2010 15:47:07 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	<samu.p.onkalo@...ia.com>
Cc:	<akpm@...ux-foundation.org>, <Eric.Piel@...mplin-utc.net>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH resent] lis3: Fix Oops with NULL platform data

At Wed, 22 Sep 2010 13:56:15 +0200,
<samu.p.onkalo@...ia.com> wrote:
> 
> 
> Hi
> 
> >-----Original Message-----
> >From: ext Takashi Iwai [mailto:tiwai@...e.de]
> >Sent: 22 September, 2010 14:34
> >To: Andrew Morton
> >Cc: Onkalo Samu.P (Nokia-MS/Tampere); Éric Piel; linux-
> >kernel@...r.kernel.org
> >Subject: [PATCH resent] lis3: Fix Oops with NULL platform data
> >
> >The recent addition of threaded irq handler causes a NULL dereference
> >when used with hp_accel driver, which has NULL pdata.
> >
> >Cc: <stable@...nel.org>
> >Signed-off-by: Takashi Iwai <tiwai@...e.de>
> >---
> >
> >This should go to 2.6.36 and 2.6.35-stable tree.
> >
> > drivers/hwmon/lis3lv02d.c |    6 ++++--
> > 1 files changed, 4 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
> >index 5e15967..1c266ec 100644
> >--- a/drivers/hwmon/lis3lv02d.c
> >+++ b/drivers/hwmon/lis3lv02d.c
> >@@ -354,7 +354,8 @@ static irqreturn_t lis302dl_interrupt_thread1_8b(int
> >irq, void *data)
> >
> > 	struct lis3lv02d *lis3 = data;
> >
> >-	if ((lis3->pdata->irq_cfg & LIS3_IRQ1_MASK) == LIS3_IRQ1_CLICK)
> >+	if (lis3->pdata &&
> >+	    (lis3->pdata->irq_cfg & LIS3_IRQ1_MASK) == LIS3_IRQ1_CLICK)
> > 		lis302dl_interrupt_handle_click(lis3);
> > 	else
> > 		lis302dl_interrupt_handle_ff_wu(lis3);
> >@@ -367,7 +368,8 @@ static irqreturn_t lis302dl_interrupt_thread2_8b(int
> >irq, void *data)
> >
> > 	struct lis3lv02d *lis3 = data;
> >
> >-	if ((lis3->pdata->irq_cfg & LIS3_IRQ2_MASK) == LIS3_IRQ2_CLICK)
> >+	if (lis3->pdata &&
> >+	    (lis3->pdata->irq_cfg & LIS3_IRQ2_MASK) == LIS3_IRQ2_CLICK)
> > 		lis302dl_interrupt_handle_click(lis3);
> > 	else
> > 		lis302dl_interrupt_handle_ff_wu(lis3);
> 
> Yes, that will remove the kernel oops definitely, but I'm wondering if
> this thread should be executed at all if there is no pdata available.

We may control the caller of request_threaded_irq(), instead.
The revised patch is below.  (With this patch, the patch for the new
LIS3CD chip must be revised, too.)

> Eric, is it so that in laptops interrupts are used for free fall detection
> and polled input device is for joystick?

Yes, it's so for HP laptops.

> Interrupts can (and are used) also for other purposes but in that case
> some additional configurations are done based on platform data.
> 
> With this patch, there are just some additional input event refreshes in
> top of polled device updates. It should really not matter. So,
> 
> Acked-by: Samu Onkalo <samu.p.onkalo@...ia.com>
> 
> -Samu


thanks,

Takashi

===
>From 0e5bdc7516915e92fe471a77da36ecb75d995c0f Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@...e.de>
Date: Wed, 22 Sep 2010 12:16:04 +0200
Subject: [PATCH] lis3: Fix Oops with NULL platform data

The recent addition of threaded irq handler causes a NULL dereference
when used with hp_accel driver, which has NULL pdata.

Signed-off-by: Takashi Iwai <tiwai@...e.de>
---
this version avoids the unnecessary thread_irq call instead of checking
pdata in the irq handler.

 drivers/hwmon/lis3lv02d.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 5e15967..0a24177 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -301,7 +301,7 @@ static irqreturn_t lis302dl_interrupt(int irq, void *dummy)
 	wake_up_interruptible(&lis3_dev.misc_wait);
 	kill_fasync(&lis3_dev.async_queue, SIGIO, POLL_IN);
 out:
-	if (lis3_dev.whoami == WAI_8B && lis3_dev.idev &&
+	if (lis3_dev.pdata && lis3_dev.whoami == WAI_8B && lis3_dev.idev &&
 	    lis3_dev.idev->input->users)
 		return IRQ_WAKE_THREAD;
 	return IRQ_HANDLED;
@@ -742,7 +742,7 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
 	 * io-apic is not configurable (and generates a warning) but I keep it
 	 * in case of support for other hardware.
 	 */
-	if (dev->whoami == WAI_8B)
+	if (dev->pdata && dev->whoami == WAI_8B)
 		thread_fn = lis302dl_interrupt_thread1_8b;
 	else
 		thread_fn = NULL;
-- 
1.7.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/