On Tuesday, July 23, 2013 07:20:53 PM Yinghai Lu wrote:
> On Tue, Jul 23, 2013 at 2:39 PM, Rafael J. Wysocki <rjw@xxxxxxx> wrote:
> >
> > Ugh, stupid bug, sorry about it. We try to unregister something that may have
> > not been registered.
> >
> > Can you please check if the appended patch helps (on top of
> > linux-pm.git/linux-next)?
> >
> > Rafael
> >
> >
> > ---
> > drivers/pci/hotplug/acpiphp_glue.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
> > ===================================================================
> > --- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
> > +++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
> > @@ -340,6 +340,7 @@ static acpi_status register_slot(acpi_ha
> >
> > retval = acpiphp_register_hotplug_slot(slot, sun);
> > if (retval) {
> > + slot->slot = NULL;
> > bridge->nr_slots--;
> > if (retval == -EBUSY)
> > warn("Slot %llu already registered by another "
> > @@ -429,7 +430,8 @@ static void cleanup_bridge(struct acpiph
> > err("failed to remove notify handler\n");
> > }
> > }
> > - acpiphp_unregister_hotplug_slot(slot);
> > + if (slot->slot)
> > + acpiphp_unregister_hotplug_slot(slot);
> > }
> >
> > mutex_lock(&bridge_mutex);
> >
>
> yes, that fixes the problem. Thanks
Great, thanks for testing!
I'm adding this to the series (after commit 7342798):
---
From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
Subject: ACPI / hotplug / PCI: Fix NULL pointer dereference in cleanup_bridge()
After commit bbd34fc (ACPI / hotplug / PCI: Register all devices
under the given bridge) register_slot() is called for all PCI
devices under a given bridge that have corresponding objects in
the ACPI namespace, but it calls acpiphp_register_hotplug_slot()
only for devices satisfying specific criteria. Still,
cleanup_bridge() calls acpiphp_unregister_hotplug_slot() for all
objects created by register_slot(), although it should only call it
for the ones that acpiphp_register_hotplug_slot() has been called
for (successfully). This causes a NULL pointer to be dereferenced
by the acpiphp_unregister_hotplug_slot() executed by cleanup_bridge()
if the object it is called for has not been passed to
acpiphp_register_hotplug_slot().
To fix this problem, check if the 'slot' field of the object passed
to acpiphp_unregister_hotplug_slot() in cleanup_bridge() is not NULL,
which only is the case if acpiphp_register_hotplug_slot() has been
executed for that object. In addition to that, make register_slot()
reset the 'slot' field to NULL if acpiphp_register_hotplug_slot() has
failed for the given object to prevent stale pointers from being
used by acpiphp_unregister_hotplug_slot().
Reported-and-tested-by: Yinghai Lu <yinghai@xxxxxxxxxx>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
---
drivers/pci/hotplug/acpiphp_glue.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
@@ -340,6 +340,7 @@ static acpi_status register_slot(acpi_ha
retval = acpiphp_register_hotplug_slot(slot, sun);
if (retval) {
+ slot->slot = NULL;
bridge->nr_slots--;
if (retval == -EBUSY)
warn("Slot %llu already registered by another "
@@ -429,7 +430,8 @@ static void cleanup_bridge(struct acpiph
err("failed to remove notify handler\n");
}
}
- acpiphp_unregister_hotplug_slot(slot);
+ if (slot->slot)
+ acpiphp_unregister_hotplug_slot(slot);
}
mutex_lock(&bridge_mutex);
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
