lkml.org 
[lkml]   [2013]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH 0/30] ACPI / hotplug / PCI: Major rework + Thunderbolt workarounds
Date
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@sisk.pl> 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@intel.com>
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@kernel.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
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);


\
 
 \ /
  Last update: 2013-07-24 15:01    [W:0.137 / U:0.360 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site