From patchwork Tue Jul 30 10:58:55 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 13747256 Received: from mailout3.hostsharing.net (mailout3.hostsharing.net [176.9.242.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 701301DA32; Tue, 30 Jul 2024 10:59:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=176.9.242.54 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722337143; cv=none; b=PKdUG3tmWFRuY9CF6ebe/4/nWY0dysuHrRU49J4LcOfeDgaO/nbhiFLXFCyRUmzQtimiaq5/04NUd8XPJhBzw9HoTefP/aVFTecpNfJdO/HMYkB96Md9XHpro1eYgJKjQBWUnbGTSp4TkQF2FQAyP4+aO5AadbFEIAF8/PzIp4E= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722337143; c=relaxed/simple; bh=Bhw2lI10PnuzWlB8lHFHTeH/r+O45N+/VNZ1JfjHtO8=; h=Message-ID:From:Date:Subject:MIME-Version:Content-Type:To:Cc; b=Ku9B+bCfNBeHIZY3JhAoTpt7ntijBi83j3uluIlAwo8bSBpDgi/MD4D4crZB6K20VAIxBYQUxbc2PV8gkYq9E/AbDHLuJZNaPpyI9xOsjT7IDAoPhax1yYDwRB19zGuSbQo1nRKN85mZv4za7H+lNKM5JgBRtnB2DitpuqpRTrU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=wunner.de; spf=pass smtp.mailfrom=wunner.de; arc=none smtp.client-ip=176.9.242.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=wunner.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=wunner.de Received: from h08.hostsharing.net (h08.hostsharing.net [IPv6:2a01:37:1000::53df:5f1c:0]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "*.hostsharing.net", Issuer "RapidSSL TLS RSA CA G1" (verified OK)) by mailout3.hostsharing.net (Postfix) with ESMTPS id 92F03101E6B67; Tue, 30 Jul 2024 12:58:58 +0200 (CEST) Received: from localhost (unknown [89.246.108.87]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (prime256v1) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 52696609C800; Tue, 30 Jul 2024 12:58:58 +0200 (CEST) X-Mailbox-Line: From ecc9e4e0552069fb3cffb46a45e5430d88d71e80 Mon Sep 17 00:00:00 2001 Message-ID: From: Lukas Wunner Date: Tue, 30 Jul 2024 12:58:55 +0200 Subject: [PATCH 5.15-stable] PCI/DPC: Fix use-after-free on concurrent DPC and hot-removal Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: Greg Kroah-Hartman , Sasha Levin Cc: stable@vger.kernel.org, linux-pci@vger.kernel.org, Keith Busch , Mika Westerberg , Bjorn Helgaas , Krzysztof Wilczynski commit 11a1f4bc47362700fcbde717292158873fb847ed upstream. Keith reports a use-after-free when a DPC event occurs concurrently to hot-removal of the same portion of the hierarchy: The dpc_handler() awaits readiness of the secondary bus below the Downstream Port where the DPC event occurred. To do so, it polls the config space of the first child device on the secondary bus. If that child device is concurrently removed, accesses to its struct pci_dev cause the kernel to oops. That's because pci_bridge_wait_for_secondary_bus() neglects to hold a reference on the child device. Before v6.3, the function was only called on resume from system sleep or on runtime resume. Holding a reference wasn't necessary back then because the pciehp IRQ thread could never run concurrently. (On resume from system sleep, IRQs are not enabled until after the resume_noirq phase. And runtime resume is always awaited before a PCI device is removed.) However starting with v6.3, pci_bridge_wait_for_secondary_bus() is also called on a DPC event. Commit 53b54ad074de ("PCI/DPC: Await readiness of secondary bus after reset"), which introduced that, failed to appreciate that pci_bridge_wait_for_secondary_bus() now needs to hold a reference on the child device because dpc_handler() and pciehp may indeed run concurrently. The commit was backported to v5.10+ stable kernels, so that's the oldest one affected. Add the missing reference acquisition. Abridged stack trace: BUG: unable to handle page fault for address: 00000000091400c0 CPU: 15 PID: 2464 Comm: irq/53-pcie-dpc 6.9.0 RIP: pci_bus_read_config_dword+0x17/0x50 pci_dev_wait() pci_bridge_wait_for_secondary_bus() dpc_reset_link() pcie_do_recovery() dpc_handler() Fixes: 53b54ad074de ("PCI/DPC: Await readiness of secondary bus after reset") Closes: https://lore.kernel.org/r/20240612181625.3604512-3-kbusch@meta.com/ Link: https://lore.kernel.org/linux-pci/8e4bcd4116fd94f592f2bf2749f168099c480ddf.1718707743.git.lukas@wunner.de Reported-by: Keith Busch Tested-by: Keith Busch Signed-off-by: Lukas Wunner Signed-off-by: Krzysztof Wilczyński Reviewed-by: Keith Busch Reviewed-by: Mika Westerberg Cc: stable@vger.kernel.org # v5.10+ --- drivers/pci/pci.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 67216f4ea215..a88909f2ae65 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4916,7 +4916,7 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type, int timeout) { struct pci_dev *child; - int delay; + int delay, ret = 0; if (pci_dev_is_disconnected(dev)) return 0; @@ -4944,8 +4944,8 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type, return 0; } - child = list_first_entry(&dev->subordinate->devices, struct pci_dev, - bus_list); + child = pci_dev_get(list_first_entry(&dev->subordinate->devices, + struct pci_dev, bus_list)); up_read(&pci_bus_sem); /* @@ -4955,7 +4955,7 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type, if (!pci_is_pcie(dev)) { pci_dbg(dev, "waiting %d ms for secondary bus\n", 1000 + delay); msleep(1000 + delay); - return 0; + goto put_child; } /* @@ -4976,7 +4976,7 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type, * until the timeout expires. */ if (!pcie_downstream_port(dev)) - return 0; + goto put_child; if (pcie_get_speed_cap(dev) <= PCIE_SPEED_5_0GT) { pci_dbg(dev, "waiting %d ms for downstream link\n", delay); @@ -4987,11 +4987,16 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type, if (!pcie_wait_for_link_delay(dev, true, delay)) { /* Did not train, no need to wait any further */ pci_info(dev, "Data Link Layer Link Active not set in 1000 msec\n"); - return -ENOTTY; + ret = -ENOTTY; + goto put_child; } } - return pci_dev_wait(child, reset_type, timeout - delay); + ret = pci_dev_wait(child, reset_type, timeout - delay); + +put_child: + pci_dev_put(child); + return ret; } void pci_reset_secondary_bus(struct pci_dev *dev)