| Message ID | Pine.LNX.4.64.0908070012490.11170@sister.anvils (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Thursday 06 August 2009 05:18:12 pm Hugh Dickins wrote: > It's up to Len to choose, but I thought we preferred... > > From: Hugh Dickins <hugh.dickins@tiscali.co.uk> > > acpi_device->pnp.hardware_id and unique_id are now allocated pointers, > replacing the previous arrays. acpi_device_install_notify_handler() > oopsed on the NULL hid when probing the video device, I'm sorry I wasn't aware of the acpi_device_install_notify_handler() issue until just now, since I added the code that oopses (46ec8598fd). I recently posted patches that removed the dependency on the HID in this path: http://marc.info/?l=linux-acpi&m=124907623701270&w=2 I thought Len said they had been applied to acpi-test, but I don't see them there. But I don't object to Hugh's patch, since it might be useful for other paths. Bjorn > and perhaps other > uses are vulnerable too. So initialize those pointers to empty strings > when there is no hid or uid. Also, free hardware_id and unique_id when > when acpi_device is going to be freed. > > Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk> > Signed-off-by: Lin Ming <ming.m.lin@intel.com> > --- > > drivers/acpi/scan.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > --- mmotm/drivers/acpi/scan.c 2009-07-17 12:53:20.000000000 +0100 > +++ linux/drivers/acpi/scan.c 2009-07-27 18:01:32.000000000 +0100 > @@ -309,6 +309,10 @@ static void acpi_device_release(struct d > struct acpi_device *acpi_dev = to_acpi_device(dev); > > kfree(acpi_dev->pnp.cid_list); > + if (acpi_dev->flags.hardware_id) > + kfree(acpi_dev->pnp.hardware_id); > + if (acpi_dev->flags.unique_id) > + kfree(acpi_dev->pnp.unique_id); > kfree(acpi_dev); > } > > @@ -1144,8 +1148,9 @@ static void acpi_device_set_id(struct ac > strcpy(device->pnp.hardware_id, hid); > device->flags.hardware_id = 1; > } > - } else > - device->pnp.hardware_id = NULL; > + } > + if (!device->flags.hardware_id) > + device->pnp.hardware_id = ""; > > if (uid) { > device->pnp.unique_id = ACPI_ALLOCATE_ZEROED(strlen (uid) + 1); > @@ -1153,8 +1158,9 @@ static void acpi_device_set_id(struct ac > strcpy(device->pnp.unique_id, uid); > device->flags.unique_id = 1; > } > - } else > - device->pnp.unique_id = NULL; > + } > + if (!device->flags.unique_id) > + device->pnp.unique_id = ""; > > if (cid_list || cid_add) { > struct acpica_device_id_list *list; > @@ -1369,10 +1375,8 @@ acpi_add_single_object(struct acpi_devic > end: > if (!result) > *child = device; > - else { > - kfree(device->pnp.cid_list); > - kfree(device); > - } > + else > + acpi_device_release(&device->dev); > > return result; > } > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 13 Aug 2009, Bjorn Helgaas wrote: > On Thursday 06 August 2009 05:18:12 pm Hugh Dickins wrote: > > It's up to Len to choose, but I thought we preferred... > > > > From: Hugh Dickins <hugh.dickins@tiscali.co.uk> > > > > acpi_device->pnp.hardware_id and unique_id are now allocated pointers, > > replacing the previous arrays. acpi_device_install_notify_handler() > > oopsed on the NULL hid when probing the video device, > > I'm sorry I wasn't aware of the acpi_device_install_notify_handler() > issue until just now, since I added the code that oopses (46ec8598fd). Sorry for not Cc'ing you: but you can hardly be blamed for not foreseeing future changes which might affect your code. > > I recently posted patches that removed the dependency on the HID > in this path: > > http://marc.info/?l=linux-acpi&m=124907623701270&w=2 > > I thought Len said they had been applied to acpi-test, but I don't > see them there. > > But I don't object to Hugh's patch, since it might be useful for other > paths. My original patch, when I hit the bug, was just to acpi_device_install_notify_handler(): I have never seen a problem anywhere else, and your patch would deal with that. But Lin Ming believes there might be issues elsewhere (I have no idea), posting a more general patch; then my patch here was a retort to that. What I'm saying is: so far as I know, your patch is more appropriate than mine; but probably Lin Ming knows better, and mine needed too. Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2009-08-14 at 00:46 +0800, Hugh Dickins wrote: > On Thu, 13 Aug 2009, Bjorn Helgaas wrote: > > On Thursday 06 August 2009 05:18:12 pm Hugh Dickins wrote: > > > It's up to Len to choose, but I thought we preferred... > > > > > > From: Hugh Dickins <hugh.dickins@tiscali.co.uk> > > > > > > acpi_device->pnp.hardware_id and unique_id are now allocated pointers, > > > replacing the previous arrays. acpi_device_install_notify_handler() > > > oopsed on the NULL hid when probing the video device, > > > > I'm sorry I wasn't aware of the acpi_device_install_notify_handler() > > issue until just now, since I added the code that oopses (46ec8598fd). > > Sorry for not Cc'ing you: but you can hardly be blamed for > not foreseeing future changes which might affect your code. > > > > > I recently posted patches that removed the dependency on the HID > > in this path: > > > > http://marc.info/?l=linux-acpi&m=124907623701270&w=2 > > > > I thought Len said they had been applied to acpi-test, but I don't > > see them there. > > > > But I don't object to Hugh's patch, since it might be useful for other > > paths. > > My original patch, when I hit the bug, was just to > acpi_device_install_notify_handler(): I have never seen a > problem anywhere else, and your patch would deal with that. > > But Lin Ming believes there might be issues elsewhere (I have no idea), > posting a more general patch; then my patch here was a retort to that. > > What I'm saying is: so far as I know, your patch is more appropriate > than mine; but probably Lin Ming knows better, and mine needed too. As below, there are many calls of acpi_device_hid without any check of NULL hid. So yes, we need a general patch, as yours. 0 drivers/acpi/button.c acpi_button_add 308 hid = acpi_device_hid(device); 1 drivers/acpi/processor_core.c acpi_processor_get_info 599 if (!strcmp(acpi_device_hid(device), ACPI_PROCESSOR_OBJECT_HID)) { 2 drivers/acpi/scan.c acpi_device_install_notify_handler 378 hid = acpi_device_hid(device); 3 drivers/acpi/scan.c acpi_device_remove_notify_handler 402 if (!strcmp(acpi_device_hid(device), ACPI_BUTTON_HID_POWERF)) 4 drivers/acpi/scan.c acpi_device_remove_notify_handler 405 else if (!strcmp(acpi_device_hid(device), ACPI_BUTTON_HID_SLEEPF)) 5 drivers/acpi/video.c acpi_video_bus_add 2248 "%s/video/input0", acpi_device_hid(video->device)); 6 drivers/gpu/drm/i915/intel_lvds.c check_lid_device 822 if (!strncmp(acpi_device_hid(acpi_dev), "PNP0C0D", 7)) 7 drivers/platform/x86/fujitsu-laptop.c acpi_fujitsu_add 681 "%s/video/input0", acpi_device_hid(device)); 8 drivers/platform/x86/fujitsu-laptop.c acpi_fujitsu_hotkey_add 852 "%s/video/input0", acpi_device_hid(device)); 9 drivers/pnp/pnpacpi/core.c pnpacpi_add_device 162 if (ACPI_FAILURE(status) || !ispnpidacpi(acpi_device_hid(device)) || a drivers/pnp/pnpacpi/core.c pnpacpi_add_device 166 dev = pnp_alloc_dev(&pnpacpi_protocol, num, acpi_device_hid(device)); Lin Ming > > Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 13 August 2009 07:28:56 pm Lin Ming wrote: > On Fri, 2009-08-14 at 00:46 +0800, Hugh Dickins wrote: > > My original patch, when I hit the bug, was just to > > acpi_device_install_notify_handler(): I have never seen a > > problem anywhere else, and your patch would deal with that. > > > > But Lin Ming believes there might be issues elsewhere (I have no idea), > > posting a more general patch; then my patch here was a retort to that. > > > > What I'm saying is: so far as I know, your patch is more appropriate > > than mine; but probably Lin Ming knows better, and mine needed too. > > As below, there are many calls of acpi_device_hid without any check of > NULL hid. > So yes, we need a general patch, as yours. I'm a little concerned because there's a deeper issue here, and I want to make sure we have a clean solution and not a temporary band-aid. In general, ACPI drivers bind to an acpi_device based on the HID. There are a few exceptions where drivers walk the namespace themselves and look for things other than the HID, but IMHO, this is a problem because those drivers won't work for devices added or removed after the driver loads. I think we'd be better off if we made a rule that "every acpi_device has a non-empty HID." That way, we could get drivers out of the business of walking the namespace, which would fix some hotplug issues. We already synthesize a number of Linux-specific HIDs (LNXPOWER, LNXCPU, LNXVIDEO, etc), and I think the Linux/ACPI core should probably do this whenever we build an acpi_device for an object with no _HID method. I don't quite understand how this oops happens, though. It seems that we crashed in this path: acpi_device_probe acpi_bus_driver_init driver->ops.add (calls acpi_video_bus_add) acpi_device_install_notify_handler hid = acpi_device_hid(device) strcmp(hid, ACPI_BUTTON_HID_POWERF)) *** OOPS, hid == NULL *** But the acpi_video_bus driver claims devices using ACPI_VIDEO_HID ("LNXVIDEO"), and acpi_device_set_id() already does synthesize that HID, so acpi_device_hid() should have been valid. So why did we oops? Bjorn > 0 drivers/acpi/button.c acpi_button_add 308 hid = acpi_device_hid(device); > 1 drivers/acpi/processor_core.c acpi_processor_get_info 599 if (!strcmp(acpi_device_hid(device), ACPI_PROCESSOR_OBJECT_HID)) { > 2 drivers/acpi/scan.c acpi_device_install_notify_handler 378 hid = acpi_device_hid(device); > 3 drivers/acpi/scan.c acpi_device_remove_notify_handler 402 if (!strcmp(acpi_device_hid(device), ACPI_BUTTON_HID_POWERF)) > 4 drivers/acpi/scan.c acpi_device_remove_notify_handler 405 else if (!strcmp(acpi_device_hid(device), ACPI_BUTTON_HID_SLEEPF)) > 5 drivers/acpi/video.c acpi_video_bus_add 2248 "%s/video/input0", acpi_device_hid(video->device)); > 6 drivers/gpu/drm/i915/intel_lvds.c check_lid_device 822 if (!strncmp(acpi_device_hid(acpi_dev), "PNP0C0D", 7)) > 7 drivers/platform/x86/fujitsu-laptop.c acpi_fujitsu_add 681 "%s/video/input0", acpi_device_hid(device)); > 8 drivers/platform/x86/fujitsu-laptop.c acpi_fujitsu_hotkey_add 852 "%s/video/input0", acpi_device_hid(device)); > 9 drivers/pnp/pnpacpi/core.c pnpacpi_add_device 162 if (ACPI_FAILURE(status) || !ispnpidacpi(acpi_device_hid(device)) || > a drivers/pnp/pnpacpi/core.c pnpacpi_add_device 166 dev = pnp_alloc_dev(&pnpacpi_protocol, num, acpi_device_hid(device)); -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 14 Aug 2009, Bjorn Helgaas wrote: > On Thursday 13 August 2009 07:28:56 pm Lin Ming wrote: > > On Fri, 2009-08-14 at 00:46 +0800, Hugh Dickins wrote: > > > My original patch, when I hit the bug, was just to > > > acpi_device_install_notify_handler(): I have never seen a > > > problem anywhere else, and your patch would deal with that. > > > > > > But Lin Ming believes there might be issues elsewhere (I have no idea), > > > posting a more general patch; then my patch here was a retort to that. > > > > > > What I'm saying is: so far as I know, your patch is more appropriate > > > than mine; but probably Lin Ming knows better, and mine needed too. > > > > As below, there are many calls of acpi_device_hid without any check of > > NULL hid. > > So yes, we need a general patch, as yours. > > I'm a little concerned because there's a deeper issue here, and I > want to make sure we have a clean solution and not a temporary band-aid. Yes, without appreciating any of the details, I believe you're right to have that concern. And, maybe irrelevant, but I'd noticed a line in acpi_device_register: if(!strcmp(acpi_device_bus_id->bus_id, device->flags.hardware_id? device->pnp.hardware_id : "device")) which made me wonder if the band-aid should say "device" rather than "". > > In general, ACPI drivers bind to an acpi_device based on the HID. > There are a few exceptions where drivers walk the namespace themselves > and look for things other than the HID, but IMHO, this is a problem > because those drivers won't work for devices added or removed after > the driver loads. > > I think we'd be better off if we made a rule that "every acpi_device > has a non-empty HID." That way, we could get drivers out of the > business of walking the namespace, which would fix some hotplug issues. > We already synthesize a number of Linux-specific HIDs (LNXPOWER, > LNXCPU, LNXVIDEO, etc), and I think the Linux/ACPI core should > probably do this whenever we build an acpi_device for an object > with no _HID method. I don't know my way around here, I cannot comment. > > I don't quite understand how this oops happens, though. It seems that > we crashed in this path: > > acpi_device_probe > acpi_bus_driver_init > driver->ops.add (calls acpi_video_bus_add) > acpi_device_install_notify_handler > hid = acpi_device_hid(device) > strcmp(hid, ACPI_BUTTON_HID_POWERF)) > *** OOPS, hid == NULL *** > > But the acpi_video_bus driver claims devices using ACPI_VIDEO_HID > ("LNXVIDEO"), and acpi_device_set_id() already does synthesize > that HID, so acpi_device_hid() should have been valid. So why > did we oops? The backtrace I jotted down, from Intel 915 graphics, looked like this (acpi_device_install_notify_handler is inlined into acpi_device_probe): strcmp acpi_device_probe really_probe driver_probe_device __driver_attach bus_for_each_dev driver_attach bus_add_driver driver_register acpi_bus_register acpi_video_register intel_opregion_init i915_driver_load drm_get_dev drm_init i915_init do_one_initcall do_basic_setup kernel_init kernel_thread_helper Valdis reported a similar but shorter hand-copied backtrace from nVidia Corporation G72M [Quadro NVS 110M/GeForce Go 7300] (rev a1): 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 Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 14 Aug 2009 10:44:50 MDT, Bjorn Helgaas said: > I don't quite understand how this oops happens, though. It seems that > we crashed in this path: > > acpi_device_probe > acpi_bus_driver_init > driver->ops.add (calls acpi_video_bus_add) > acpi_device_install_notify_handler > hid = acpi_device_hid(device) > strcmp(hid, ACPI_BUTTON_HID_POWERF)) > *** OOPS, hid == NULL *** > > But the acpi_video_bus driver claims devices using ACPI_VIDEO_HID > ("LNXVIDEO"), and acpi_device_set_id() already does synthesize > that HID, so acpi_device_hid() should have been valid. So why > did we oops? When I tripped over it, acpi_device_set_id() *wasn't* synthesizing an HID, it was explicitly setting a NULL pointer.
On Friday 14 August 2009 01:53:40 pm Valdis.Kletnieks@vt.edu wrote: > On Fri, 14 Aug 2009 10:44:50 MDT, Bjorn Helgaas said: > > > I don't quite understand how this oops happens, though. It seems that > > we crashed in this path: > > > > acpi_device_probe > > acpi_bus_driver_init > > driver->ops.add (calls acpi_video_bus_add) > > acpi_device_install_notify_handler > > hid = acpi_device_hid(device) > > strcmp(hid, ACPI_BUTTON_HID_POWERF)) > > *** OOPS, hid == NULL *** > > > > But the acpi_video_bus driver claims devices using ACPI_VIDEO_HID > > ("LNXVIDEO"), and acpi_device_set_id() already does synthesize > > that HID, so acpi_device_hid() should have been valid. So why > > did we oops? > > When I tripped over it, acpi_device_set_id() *wasn't* synthesizing an HID, > it was explicitly setting a NULL pointer. What I don't understand is why the acpi_video_bus driver could get bound to the device if the device didn't have a HID. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 14 August 2009 22:10:53 Bjorn Helgaas wrote: > On Friday 14 August 2009 01:53:40 pm Valdis.Kletnieks@vt.edu wrote: > > On Fri, 14 Aug 2009 10:44:50 MDT, Bjorn Helgaas said: > > > > > I don't quite understand how this oops happens, though. It seems that > > > we crashed in this path: > > > > > > acpi_device_probe > > > acpi_bus_driver_init > > > driver->ops.add (calls acpi_video_bus_add) > > > acpi_device_install_notify_handler > > > hid = acpi_device_hid(device) > > > strcmp(hid, ACPI_BUTTON_HID_POWERF)) > > > *** OOPS, hid == NULL *** > > > > > > But the acpi_video_bus driver claims devices using ACPI_VIDEO_HID > > > ("LNXVIDEO"), and acpi_device_set_id() already does synthesize > > > that HID, so acpi_device_hid() should have been valid. So why > > > did we oops? > > > > When I tripped over it, acpi_device_set_id() *wasn't* synthesizing an HID, > > it was explicitly setting a NULL pointer. > > What I don't understand is why the acpi_video_bus driver could get > bound to the device if the device didn't have a HID. This was also puzzling me while I hit the original issue so I did some more research at that time and it turned out that binding is not controlled by HID but by ->ops.add method of ACPI driver. In the case of ACPI video driver it goes like that (for 2.6.31-rc6): acpi_device_probe() acpi_bus_driver_init() driver->ops.add() [ acpi_video_bus_add() ] acpi_video_bus_check() and in acpi_video_bus_check() we have: ... /* Since there is no HID, CID and so on for VGA driver, we have * to check well known required nodes. */ /* Does this device support video switching? */ if (video->cap._DOS) { video->flags.multihead = 1; status = 0; } /* Does this device support retrieving a video ROM? */ if (video->cap._ROM) { video->flags.rom = 1; status = 0; } /* Does this device support configuring which video device to POST? */ if (video->cap._GPD && video->cap._SPD && video->cap._VPO) { video->flags.post = 1; status = 0; } return status; ... IOW HID is not required at all for ACPI device drivers at the moment which becomes a problem after "ACPICA: Major update for acpi_get_object_info external interface" commit since acpi_device_hid() will no longer return a pointer to an empty static buffer but a NULL instead.. I think that ideally we should always have a valid HID for ACPI devices (synthesized in case of ACPI video and similar devices) in the long-term but could we get Hugh's patch applied for now, please? While your patch also looks valid and most likely fixes ACPI video oops Hugh's patch is still needed for fully fixing the problem at the moment (i.e. ACPI button driver seems to need it). This problem has been in ACPI tree (and thus in -next) for way too long already (three weeks with the known fix) and the current situation is not moving things forward (we have just people rediscovering the issue the hard way).. Thanks, Bart -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Saturday 15 August 2009 08:14:22 am Bartlomiej Zolnierkiewicz wrote: > On Friday 14 August 2009 22:10:53 Bjorn Helgaas wrote: > > On Friday 14 August 2009 01:53:40 pm Valdis.Kletnieks@vt.edu wrote: > > > On Fri, 14 Aug 2009 10:44:50 MDT, Bjorn Helgaas said: > > > > > > > I don't quite understand how this oops happens, though. It seems that > > > > we crashed in this path: > > > > > > > > acpi_device_probe > > > > acpi_bus_driver_init > > > > driver->ops.add (calls acpi_video_bus_add) > > > > acpi_device_install_notify_handler > > > > hid = acpi_device_hid(device) > > > > strcmp(hid, ACPI_BUTTON_HID_POWERF)) > > > > *** OOPS, hid == NULL *** > > > > > > > > But the acpi_video_bus driver claims devices using ACPI_VIDEO_HID > > > > ("LNXVIDEO"), and acpi_device_set_id() already does synthesize > > > > that HID, so acpi_device_hid() should have been valid. So why > > > > did we oops? > > > > > > When I tripped over it, acpi_device_set_id() *wasn't* synthesizing an HID, > > > it was explicitly setting a NULL pointer. > > > > What I don't understand is why the acpi_video_bus driver could get > > bound to the device if the device didn't have a HID. > > This was also puzzling me while I hit the original issue so I did some more > research at that time and it turned out that binding is not controlled by > HID but by ->ops.add method of ACPI driver. > > In the case of ACPI video driver it goes like that (for 2.6.31-rc6): > > acpi_device_probe() > acpi_bus_driver_init() > driver->ops.add() [ acpi_video_bus_add() ] > acpi_video_bus_check() > > and in acpi_video_bus_check() we have: > > ... > /* Since there is no HID, CID and so on for VGA driver, we have > * to check well known required nodes. > */ > > /* Does this device support video switching? */ > if (video->cap._DOS) { > video->flags.multihead = 1; > status = 0; > } > > /* Does this device support retrieving a video ROM? */ > if (video->cap._ROM) { > video->flags.rom = 1; > status = 0; > } > > /* Does this device support configuring which video device to POST? */ > if (video->cap._GPD && video->cap._SPD && video->cap._VPO) { > video->flags.post = 1; > status = 0; > } > > return status; > ... > > IOW HID is not required at all for ACPI device drivers at the moment which > becomes a problem after "ACPICA: Major update for acpi_get_object_info > external interface" commit since acpi_device_hid() will no longer return > a pointer to an empty static buffer but a NULL instead.. > > I think that ideally we should always have a valid HID for ACPI devices > (synthesized in case of ACPI video and similar devices) in the long-term > but could we get Hugh's patch applied for now, please? > > While your patch also looks valid and most likely fixes ACPI video oops > Hugh's patch is still needed for fully fixing the problem at the moment > (i.e. ACPI button driver seems to need it). Oh, I think I see what happened. acpi_device_set_id() synthesizes ACPI_VIDEO_HID, but puts it on the CID list rather than making it the HID, so we can end up with a device with a CID but no HID. As far as I can see, there's no value in maintaining the HID separately from the CID list. Keeping a single list, with the HID (if present) being the first element, would simplify the code in a few places and avoid issues like this. I think it's a good idea to put in Hugh's patch for now, and I'll work on a patch to unify HID/CID and guarantee that every acpi_device has a valid ID. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
http://patchwork.kernel.org/patch/39735/ applied to acpica branch in acpi-test tree thanks, Len Brown, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- mmotm/drivers/acpi/scan.c 2009-07-17 12:53:20.000000000 +0100 +++ linux/drivers/acpi/scan.c 2009-07-27 18:01:32.000000000 +0100 @@ -309,6 +309,10 @@ static void acpi_device_release(struct d struct acpi_device *acpi_dev = to_acpi_device(dev); kfree(acpi_dev->pnp.cid_list); + if (acpi_dev->flags.hardware_id) + kfree(acpi_dev->pnp.hardware_id); + if (acpi_dev->flags.unique_id) + kfree(acpi_dev->pnp.unique_id); kfree(acpi_dev); } @@ -1144,8 +1148,9 @@ static void acpi_device_set_id(struct ac strcpy(device->pnp.hardware_id, hid); device->flags.hardware_id = 1; } - } else - device->pnp.hardware_id = NULL; + } + if (!device->flags.hardware_id) + device->pnp.hardware_id = ""; if (uid) { device->pnp.unique_id = ACPI_ALLOCATE_ZEROED(strlen (uid) + 1); @@ -1153,8 +1158,9 @@ static void acpi_device_set_id(struct ac strcpy(device->pnp.unique_id, uid); device->flags.unique_id = 1; } - } else - device->pnp.unique_id = NULL; + } + if (!device->flags.unique_id) + device->pnp.unique_id = ""; if (cid_list || cid_add) { struct acpica_device_id_list *list; @@ -1369,10 +1375,8 @@ acpi_add_single_object(struct acpi_devic end: if (!result) *child = device; - else { - kfree(device->pnp.cid_list); - kfree(device); - } + else + acpi_device_release(&device->dev); return result; }