Re: [PATCH] acpi: pci_root: fix NULL pointer deref after resume from suspend

Danny Feng
2009-10-09T01:19:08Z

On 10/04/2009 06:56 AM, Rafael J. Wysocki wrote:
> On Thursday 01 October 2009, Alex Chiang wrote:
>
>> Hi Danny,
>>
>> * Danny Feng<dfeng@redhat.com>:
>>
>>> Call Trace:
>>> [<ffffffff81254193>] acpi_get_pci_dev+0x106/0x167
>>> [<ffffffff8125545a>] acpi_pci_bind+0x1c/0x86
>>> [<ffffffff8116230a>] ? sysfs_create_file+0x2a/0x2c
>>> [<ffffffff8125141f>] acpi_add_single_object+0x964/0xa0c
>>> [<ffffffff812515a7>] acpi_bus_check_add+0xe0/0x138
>>> [<ffffffff81251667>] acpi_bus_scan+0x68/0xa0
>>> [<ffffffff812516f4>] acpi_bus_add+0x2a/0x2e
>>> [<ffffffff81252c59>] hotplug_dock_devices+0x114/0x13e
>>> [<ffffffff8125301a>] acpi_dock_deferred_cb+0xbf/0x192
>>> [<ffffffff8124d6ca>] acpi_os_execute_deferred+0x29/0x36
>>> [<ffffffff8106a244>] worker_thread+0x251/0x347
>>> [<ffffffff8106a1ef>] ? worker_thread+0x1fc/0x347
>>> [<ffffffff8124d6a1>] ? acpi_os_execute_deferred+0x0/0x36
>>> [<ffffffff8106e426>] ? autoremove_wake_function+0x0/0x39
>>> [<ffffffff81069ff3>] ? worker_thread+0x0/0x347
>>> [<ffffffff8106e0e0>] kthread+0x7f/0x87
>>> [<ffffffff81012cea>] child_rip+0xa/0x20
>>> [<ffffffff81012650>] ? restore_args+0x0/0x30
>>> [<ffffffff8106e061>] ? kthread+0x0/0x87
>>> [<ffffffff81012ce0>] ? child_rip+0x0/0x20
>>> Code: ff 49 89 fc 41 89 f5 a9 00 ff ff 07 74 11 be 87 00 00 00 48 c7 c7
>>> 45 6d 5a 81 e8 f6 2b e3 ff 48 c7 c7 30 ab 68 81 e8 29 77 20 00<49> 8b
>>> 5c 24 28 49 83 c4 28 eb 09 44 39 6b 38 74 10 48 89 c3 48
>>> RIP [<ffffffff812217e7>] pci_get_slot+0x4c/0x8c
>>> RSP<ffff88022ee69aa0>
>>> CR2: 0000000000000028
>>> ---[ end trace b5a7793bd9db2a4d ]---
>>>
>> Can you please reproduce with this debug patch? I'm guessing that
>> we're dying because we have a NULL parent device, but I'm curious
>> as to what causes this situation to occur.
>>
> If we had a NULL parent, acpi_get_parent() would return an error. Also, if we
> one of the devices is NULL at the PCI level, pci_get_slot() will return NULL.
> The only possibility left is that one of the buses we find in the ACPI tables
> doesn't have a secondary PCI bus.
>
> I think what happens is that on resume we get a dock notification
> (via dock_acpi_notifier registered in dock_init()) for a dock station device
> that is present in the ACPI tables, but not physically accessible at the moment
> (I guess that falls into the "BIOS bug" category, but we can fix this easily in
> the kernel).
>
> So, IMO, the appended patch is the right fix.
>
> Danny, please test it and report back (in particular, please tell us if you see
> the "Secondary bus not present" message in dmesg).
>
> Thanks,
> Rafael
>
>
> ---
> drivers/acpi/pci_root.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> Index: linux-2.6/drivers/acpi/pci_root.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/pci_root.c
> +++ linux-2.6/drivers/acpi/pci_root.c
> @@ -389,6 +389,18 @@ struct pci_dev *acpi_get_pci_dev(acpi_ha
>
> pbus = pdev->subordinate;
> pci_dev_put(pdev);
> +
> + /*
> + * During resume from a sleep state we can get a dock
> + * notification for a device that is present in ACPI tables,
> + * but not physically accessible at the moment, so tell the
> + * caller it's not present.
> + */
> + if (!pbus) {
> + dev_info(&pdev->dev, "Secondary bus not present\n");
> + pdev = NULL;
> + break;
> + }
> }
> out:
> list_for_each_entry_safe(node, tmp,&device_list, node)
>
>
Sorry for the late response, just back from holidays. Will test it soon,
thanks.