From patchwork Wed Jul 24 12:22:50 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rafael Wysocki X-Patchwork-Id: 2832665 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E5AB89F4D4 for ; Wed, 24 Jul 2013 12:13:05 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A59AC201DA for ; Wed, 24 Jul 2013 12:13:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C07A0201E0 for ; Wed, 24 Jul 2013 12:13:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751250Ab3GXMMs (ORCPT ); Wed, 24 Jul 2013 08:12:48 -0400 Received: from hydra.sisk.pl ([212.160.235.94]:37650 "EHLO hydra.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750909Ab3GXMMr (ORCPT ); Wed, 24 Jul 2013 08:12:47 -0400 Received: from vostro.rjw.lan (aemg108.neoplus.adsl.tpnet.pl [79.191.58.108]) by hydra.sisk.pl (Postfix) with ESMTPSA id 4EECEE3DB8; Wed, 24 Jul 2013 14:08:11 +0200 (CEST) From: "Rafael J. Wysocki" To: Yinghai Lu Cc: ACPI Devel Maling List , Bjorn Helgaas , LKML , Linux PCI , Jiang Liu , Mika Westerberg , "Kirill A. Shutemov" Subject: Re: [PATCH 0/30] ACPI / hotplug / PCI: Major rework + Thunderbolt workarounds Date: Wed, 24 Jul 2013 14:22:50 +0200 Message-ID: <3731352.ObxOHG6eQe@vostro.rjw.lan> User-Agent: KMail/4.9.5 (Linux/3.10.0+; KDE/4.9.5; x86_64; ; ) In-Reply-To: References: <26431283.HJCKsss0rt@vostro.rjw.lan> <3215120.cRBBS5jciE@vostro.rjw.lan> MIME-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tuesday, July 23, 2013 07:20:53 PM Yinghai Lu wrote: > On Tue, Jul 23, 2013 at 2:39 PM, Rafael J. Wysocki 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 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 Signed-off-by: Rafael J. Wysocki --- drivers/pci/hotplug/acpiphp_glue.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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);