lkml.org 
[lkml]   [2009]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: mmotm 2009-07-16-14-32 - sudden OOPS at boot in ACPI code
On Mon, 20 Jul 2009, Valdis.Kletnieks@vt.edu wrote:
> On Thu, 16 Jul 2009 14:34:02 PDT, akpm@linux-foundation.org said:
> > The mm-of-the-moment snapshot 2009-07-16-14-32 has been uploaded to
>
> Dies a horrid death during early boot. Dell Latitude D820, and this graphics:
>
> 01:00.0 VGA compatible controller: nVidia Corporation G72M [Quadro NVS 110M/GeForce Go 7300] (rev a1)

Oh yes, I was getting just the same with Intel graphics (i915);
but promptly forgot about it once I'd a workaround in place,
and moved on to other things, sorry.

>
> Traceback (hand-copied from a very crappy cell-phone picture)
>
> strcmp+0x4/0x1f
> acpi_device+probe+0xac/0x13e
> driver_probe_device+0xc9/0x14e
> __driver_attach+0x58/0x7c
> ? __driver_attach+0x58/0x7c
> ? __driver_attach+0x58/0x7c
> bus_for_each_dev+0x54/0x89
> driver_attach+0x19/0x1b
> bus_add_driver+0xv4/0x1fe
> driver_register+0xb7/0x128
> ? acpi_video_init+0x0/0x17
> acpi_bus_register_driver+0x3e/0x42
> acpi_video_register+0x42/0x6e
> acpi_video_init+0x15/0x17
> do_one_initcall+0x56/0x130
>
> Analysis shows it's the following code from (inlined) acpi_device_install_notify_handler
>
> static int acpi_device_install_notify_handler(struct acpi_device *device)
> {
> acpi_status status;
> char *hid;
>
> hid = acpi_device_hid(device);
> if (!strcmp(hid, ACPI_BUTTON_HID_POWERF))
>
> but we never check if hid is non-trash before feeding it to strcmp. Looks
> like something in this linux-next commit is involved:
>
> commit ed444824932d2a563858d82ec1ea29b0aa775e91
> Author: Bob Moore <robert.moore@intel.com>
> Date: Mon Jun 29 13:39:29 2009 +0800
>
> I suspect something in acpi_get_object_info() is going astray, causing
> acpi_device_set_id() to set the ->pnp.hardware_id to NULL in this code:
>
> if (hid) {
> device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
> if (device->pnp.hardware_id) {
> strcpy(device->pnp.hardware_id, hid);
> device->flags.hardware_id = 1;
> }
> } else
> device->pnp.hardware_id = NULL;
>
> The else clause is new in this commit.

I think pnp.hardware_id has changed from being a builtin array to
an allocated pointer: so before there was always a zeroed array to
strcmp against, whereas now there's a NULL pointer if you come to
use acpi_device_install_notify_handler() "too early".

Patch that works for me at the bottom.

>
> Looking at the old code, it *may* be that the ACPI code on my laptop is just
> busticated and/or there's no _HID method for the graphics card, and the old
> code Just Happened To Work in previous kernels because ->pnp.hardware_id
> wouldn't actually get set *at all* in acpi_device_set_id, so we'd get random
> stale data that was bogus, but didn't give strcmp() indigestion...
>
> Any wisdom on debugging this further (including how to tell if the ACPI
> tables have a sane _HID method for the graphics card) would be appreciated...
>
> Or is the correct fix in fact to just add a 'if (!hid) return -EINVAL;' to
> acpi_device_install_notify_handler()?

[PATCH mmotm] acpi: work around NULL hardware_id

Work around NULL pnp.hardware_id in acpi_device_install_notify_handler()
when probing video device.

Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
---
Signoff provided to handle the unlikely event that this hack
is actually the right fix!
drivers/acpi/scan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- mmotm/drivers/acpi/scan.c 2009-07-17 12:53:20.000000000 +0100
+++ linux/drivers/acpi/scan.c 2009-07-17 21:19:10.000000000 +0100
@@ -376,12 +376,12 @@ static int acpi_device_install_notify_ha
char *hid;

hid = acpi_device_hid(device);
- if (!strcmp(hid, ACPI_BUTTON_HID_POWERF))
+ if (hid && !strcmp(hid, ACPI_BUTTON_HID_POWERF))
status =
acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
acpi_device_notify_fixed,
device);
- else if (!strcmp(hid, ACPI_BUTTON_HID_SLEEPF))
+ else if (hid && !strcmp(hid, ACPI_BUTTON_HID_SLEEPF))
status =
acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
acpi_device_notify_fixed,

\
 
 \ /
  Last update: 2009-07-21 05:37    [from the cache]
©2003-2011 Jasper Spaans