Re: [PATCH] lis3lv02d: avoid divide by zero due to unchecked registerread
From: Éric Piel
Date: Mon May 16 2011 - 07:41:47 EST
Op 16-05-11 00:46, Christian Lamparter schreef:
After an "unexpected" reboot, I found this Oops in my logs:
divide error: 0000 [#1] PREEMPT SMP
CPU 0
Modules linked in: lis3lv02d hp_wmi input_polldev [...]
Pid: 390, comm: modprobe Tainted: G C 2.6.39-rc7-wl+
RIP: 0010:[<ffffffffa014b427>] [<ffffffffa014b427>]
lis3lv02d_poweron+0x4e/0x94 [lis3lv02d]
RSP: 0018:ffff8801d6407cf8 EFLAGS: 00010246
RAX: 0000000000000bb8 RBX: ffffffffa014e000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffea00066e4708 RDI: ffff8801df002700
RBP: ffff8801d6407d18 R08: ffffea00066c5a30 R09: ffffffff812498c9
R10: ffff8801d7bfcea0 R11: ffff8801d7bfce10 R12: 0000000000000bb8
R13: 00000000ffffffda R14: ffffffffa0154120 R15: ffffffffa0154030
FS: 00007fc0705db700(0000) GS:ffff8801dfa00000(0000) knlGS:0
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 00007f33549174f0 CR3: 00000001d65c9000 CR4: 00000000000406f0
Process modprobe (pid: 390, threadinfo ffff8801d6406000, task ffff8801d6b40000)
Stack:
ffffffffa0154120 62ffffffa0154030 ffffffffa014e000 00000000ffffffea
ffff8801d6407d58 ffffffffa014bcc1 0000000000000000 0000000000000048
ffff8801d8bae800 00000000ffffffea 00000000ffffffda ffffffffa0154120
Call Trace:
[<ffffffffa014bcc1>] lis3lv02d_init_device+0x1ce/0x496 [lis3lv02d]
[<ffffffffa01522ff>] lis3lv02d_add+0x10f/0x17c [hp_accel]
[<ffffffff81233e11>] acpi_device_probe+0x49/0x117
[...]
Code: 3a 75 06 80 4d ef 50 eb 04 80 4d ef 40 0f b6 55 ef be 21
00 00 00 48 89 df ff 53 18 44 8b 63 6c e8 3e fc ff ff 89 c1 44
89 e0 99<f7> f9 89 c7 e8 93 82 ef e0 48 83 7b 30 00 74 2d 45
31 e4 80 7b
RIP [<ffffffffa014b427>] lis3lv02d_poweron+0x4e/0x94 [lis3lv02d]
RSP<ffff8801d6407cf8>
From my POV, it looks like the hardware is not working as expected
and returns a bogus data rate. The driver doesn't check the result
and directly uses it as some sort of divisor in some places:
msleep(lis3->pwron_delay / lis3lv02d_get_odr());
Under this circumstances, this could very well cause the
"divide by zero" exception from above.
For now, I fixed it the easiest and most obvious way:
Check if the result is sane and if it isn't use a sane default
instead. I went for "100" in the latter case, simply because
/sys/devices/platform/lis3lv02d/rate returns it on a successful
boot.
Hi,
Good catch! We don't want to have a kernel oops just because a device is
sending bad data...
However, I'd fix it a bit differently: let lis3lv02d_get_odr() return
the raw data, and create a special function
lis3lv02d_get_pwron_delay_ms() which does the "lis3->pwron_delay /
lis3lv02d_get_odr()" with special handling for 0 (returning a large
value and also sending a printk_once() ).
As you have noted, we might want to check other parts of the driver to
validate the data from the device. So far, all the code considers that
the device is flawless :-S
Cheers,
Éric
Signed-off-by: Christian Lamparter<chunkeey@xxxxxxxxxxxxxx>
---
Oh, please keep the CC.
Note: This patch is a bit experimental, I can't get the
device to fail reliable so even with the patch, it might
fail in a different place.
Regards,
Chr
---
diff --git a/drivers/misc/lis3lv02d/lis3lv02d.c b/drivers/misc/lis3lv02d/lis3lv02d.c
index b928bc1..96102f0 100644
--- a/drivers/misc/lis3lv02d/lis3lv02d.c
+++ b/drivers/misc/lis3lv02d/lis3lv02d.c
@@ -198,12 +198,18 @@ static int lis3_3dc_rates[16] = {0, 1, 10, 25, 50, 100, 200, 400, 1600, 5000};
static int lis3lv02d_get_odr(void)
{
u8 ctrl;
- int shift;
+ int shift, res;
lis3_dev.read(&lis3_dev, CTRL_REG1,&ctrl);
ctrl&= lis3_dev.odr_mask;
shift = ffs(lis3_dev.odr_mask) - 1;
- return lis3_dev.odrs[(ctrl>> shift)];
+ res = lis3_dev.odrs[(ctrl>> shift)];
+
+ /*
+ * because the result is used as a divisor, it must
+ * not be a zero.
+ */
+ return (res != 0) ? res : 100;
}
static int lis3lv02d_set_odr(int rate)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/