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

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]

Message-Id: <200910040056.00247.rjw@sisk.pl>
Date:	Sun, 4 Oct 2009 00:56:00 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Alex Chiang <achiang@...com>, Danny Feng <dfeng@...hat.com>
Cc:	lenb@...nel.org, bjorn.helgaas@...com, andrew.patterson@...com,
	jbarnes@...tuousgeek.org, linux-acpi@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] acpi: pci_root: fix NULL pointer deref after resume from suspend

On Thursday 01 October 2009, Alex Chiang wrote:
> Hi Danny,
> 
> * Danny Feng <dfeng@...hat.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)
--
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/