From patchwork Wed Mar 12 08:06:34 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philipp Stanner X-Patchwork-Id: 14013071 X-Patchwork-Delegate: bhelgaas@google.com Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 5250A232364; Wed, 12 Mar 2025 08:06:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741766811; cv=none; b=djv/YAAUKEltQsqORPFOl7ZXywDm4wasL/bp5xXBpDkU3QhH1ZeludPCF/kL7RXzNrD9wSzAsTBNiXRK280ApW6EQ9n72iylRq6lsq057As+R7ux5u96vQ2XNHHwyQPgpNuCPZi/45dhwZBGcrASKq95UCXcB2x79GZMLxeKvpU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741766811; c=relaxed/simple; bh=8/aqzVkQhaCsfezDRLPycVmFSKSk0ZnXQ8yaaEJW60Y=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=pr40HNpu7tIYIBascDd3JZO3G3WdvGK/ENIvIjBUOeC/p5YXyb41flW8IsRKgPjd4NW+jiwc5eb44pXZUQof7SsNzHtquxYQBRdxVIxqv+uovDWpJtxxZ2QuyVDFQZP6E473MFIFqBpvrQ6gMAWhaImIHIg5lIXLXm2fI2Lj5EQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=lsoMAARq; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="lsoMAARq" Received: by smtp.kernel.org (Postfix) with ESMTPSA id DE246C4CEEA; Wed, 12 Mar 2025 08:06:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741766810; bh=8/aqzVkQhaCsfezDRLPycVmFSKSk0ZnXQ8yaaEJW60Y=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lsoMAARqEI7pAj3h6fSaKPbdTl/tb1+CB5Rv1rlynbU8zn9TODlEvSqbd614Kl9L8 zq8VVaEx/6pRIx7+rorYfooD6ssaZGGqThufVWU0w5prMWDd7YMW66QIHPRLU1Zzsq y/XYSoXzCF2LAlIGJmeQfINYLI27bu+fejH9vuoIO7Nzb7Zmj5uc7F7MjAW6M5lMR/ GEI+osKsFd4bzTHuhPBBGgOsywD/+5j/yyz24irRqIuDbMwxLJkQyCVw7ujQJGWg9O Rq+tUqqSpCmqf8meSvc+rmtQkcXyVzlA896gETNvezdxNB4siPMXvWFd/fnbPidhA3 xa6FIaBGFyW3g== From: Philipp Stanner To: =?utf-8?q?Krzysztof_Wilczy=C5=84ski?= , Bjorn Helgaas Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Philipp Stanner , stable@vger.kernel.org Subject: [PATCH v3 1/2] PCI: Fix wrong length of devres array Date: Wed, 12 Mar 2025 09:06:34 +0100 Message-ID: <20250312080634.13731-3-phasta@kernel.org> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250312080634.13731-2-phasta@kernel.org> References: <20250312080634.13731-2-phasta@kernel.org> Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 The array for the iomapping cookie addresses has a length of PCI_STD_NUM_BARS. This constant, however, only describes standard BARs; while PCI can allow for additional, special BARs. The total number of PCI resources is described by constant PCI_NUM_RESOURCES, which is also used in, e.g., pci_select_bars(). Thus, the devres array has so far been too small. Change the length of the devres array to PCI_NUM_RESOURCES. Cc: # v6.11+ Fixes: bbaff68bf4a4 ("PCI: Add managed partial-BAR request and map infrastructure") Signed-off-by: Philipp Stanner --- drivers/pci/devres.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c index 3431a7df3e0d..728ed0c7f70a 100644 --- a/drivers/pci/devres.c +++ b/drivers/pci/devres.c @@ -40,7 +40,7 @@ * Legacy struct storing addresses to whole mapped BARs. */ struct pcim_iomap_devres { - void __iomem *table[PCI_STD_NUM_BARS]; + void __iomem *table[PCI_NUM_RESOURCES]; }; /* Used to restore the old INTx state on driver detach. */ From patchwork Wed Mar 12 08:06:35 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philipp Stanner X-Patchwork-Id: 14013072 X-Patchwork-Delegate: bhelgaas@google.com Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 62955232364; Wed, 12 Mar 2025 08:06:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741766814; cv=none; b=FMcTY6qjhO72QhoLCSABeMk2u9UVJiXHtd+eRsfToaaeCnWJxUMkgUHXiXsObp0+ehj/9BzERsf3JXs9NdkwXN3dpzR+gcdxqpbOWBn41hKCDYCk9QHo1Z2gOCnn2s6bYfZf+n8F7oskIznBdBG/7lxDVsFSi/UOc/alsR2eqSY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741766814; c=relaxed/simple; bh=hy1fPZC2FIGVsEIMGtrBPmDIOw6KnV62HzIoDTQYHLw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ocZpsBxy4INqb9H6hgrKaTAeEIuJUrgOBuzG546yZQajBVD+2Kh/uKlL1VlJByszBHQXAGVbWeTkn0TYHkuEnzAPrBByTxNxIqfhyZscZu+/I1sOPD/aIrIvcYuQlZwbOPhnmc/mQdgqTuYa1kHrRC/mh6/Wu3FP7HHbjk7ceTA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=m/vef+q8; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="m/vef+q8" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 764B6C4CEEE; Wed, 12 Mar 2025 08:06:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741766813; bh=hy1fPZC2FIGVsEIMGtrBPmDIOw6KnV62HzIoDTQYHLw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=m/vef+q8ptNAQIaH3VyG6fBhF7ecuTqBBMiXdTB2jQ0Q2VNtJytrUpn+WhTUwmvMR eW8X8QlFyCq25GtJRYU+sF2SzThP7n1/GefEXOrYUy0B27KRZsVRhPma0b4LvjYDMX DtkCnf724+RbJByyYc16BCmaeIsFb8YQ3+4lZO4x3OJXVOjpjyoFU/q6cHINhYTTvp 4a8k5qfKXNKBvzgWdgsQxPB4ZYquNp91aDvrY3pFuAfgZw0EQQJMT9QoGkM5RX1y69 3SsC3XhGPhfrXw92lfKsoG0CcNIUiYP+FEfP3tGAkjsXT0psRz69KFXAcETDWKZuVy B0Bb+8eCq5VCQ== From: Philipp Stanner To: =?utf-8?q?Krzysztof_Wilczy=C5=84ski?= , Bjorn Helgaas Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Philipp Stanner , Bingbu Cao Subject: [PATCH v3 2/2] PCI: Check BAR index for validity Date: Wed, 12 Mar 2025 09:06:35 +0100 Message-ID: <20250312080634.13731-4-phasta@kernel.org> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250312080634.13731-2-phasta@kernel.org> References: <20250312080634.13731-2-phasta@kernel.org> Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Many functions in PCI use accessor macros such as pci_resource_len(), which take a BAR index. That index, however, is never checked for validity, potentially resulting in undefined behavior by overflowing the array pci_dev.resource in the macro pci_resource_n(). Since many users of those macros directly assign the accessed value to an unsigned integer, the macros cannot be changed easily anymore to return -EINVAL for invalid indexes. Consequently, the problem has to be mitigated in higher layers. Add pci_bar_index_valid(). Use it where appropriate. Reported-by: Bingbu Cao Closes: https://lore.kernel.org/all/adb53b1f-29e1-3d14-0e61-351fd2d3ff0d@linux.intel.com/ Signed-off-by: Philipp Stanner --- drivers/pci/devres.c | 16 ++++++++++++++-- drivers/pci/iomap.c | 29 +++++++++++++++++++++-------- drivers/pci/pci.c | 6 ++++++ drivers/pci/pci.h | 16 ++++++++++++++++ 4 files changed, 57 insertions(+), 10 deletions(-) diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c index 728ed0c7f70a..73047316889e 100644 --- a/drivers/pci/devres.c +++ b/drivers/pci/devres.c @@ -577,7 +577,7 @@ static int pcim_add_mapping_to_legacy_table(struct pci_dev *pdev, { void __iomem **legacy_iomap_table; - if (bar >= PCI_STD_NUM_BARS) + if (!pci_bar_index_is_valid(bar)) return -EINVAL; legacy_iomap_table = (void __iomem **)pcim_iomap_table(pdev); @@ -622,7 +622,7 @@ static void pcim_remove_bar_from_legacy_table(struct pci_dev *pdev, int bar) { void __iomem **legacy_iomap_table; - if (bar >= PCI_STD_NUM_BARS) + if (!pci_bar_index_is_valid(bar)) return; legacy_iomap_table = (void __iomem **)pcim_iomap_table(pdev); @@ -655,6 +655,9 @@ void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen) void __iomem *mapping; struct pcim_addr_devres *res; + if (!pci_bar_index_is_valid(bar)) + return NULL; + res = pcim_addr_devres_alloc(pdev); if (!res) return NULL; @@ -722,6 +725,9 @@ void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, int ret; struct pcim_addr_devres *res; + if (!pci_bar_index_is_valid(bar)) + return IOMEM_ERR_PTR(-EINVAL); + res = pcim_addr_devres_alloc(pdev); if (!res) return IOMEM_ERR_PTR(-ENOMEM); @@ -823,6 +829,9 @@ static int _pcim_request_region(struct pci_dev *pdev, int bar, const char *name, int ret; struct pcim_addr_devres *res; + if (!pci_bar_index_is_valid(bar)) + return -EINVAL; + res = pcim_addr_devres_alloc(pdev); if (!res) return -ENOMEM; @@ -991,6 +1000,9 @@ void __iomem *pcim_iomap_range(struct pci_dev *pdev, int bar, void __iomem *mapping; struct pcim_addr_devres *res; + if (!pci_bar_index_is_valid(bar)) + return IOMEM_ERR_PTR(-EINVAL); + res = pcim_addr_devres_alloc(pdev); if (!res) return IOMEM_ERR_PTR(-ENOMEM); diff --git a/drivers/pci/iomap.c b/drivers/pci/iomap.c index 9fb7cacc15cd..fe706ed946df 100644 --- a/drivers/pci/iomap.c +++ b/drivers/pci/iomap.c @@ -9,6 +9,8 @@ #include +#include "pci.h" /* for pci_bar_index_is_valid() */ + /** * pci_iomap_range - create a virtual mapping cookie for a PCI BAR * @dev: PCI device that owns the BAR @@ -33,12 +35,19 @@ void __iomem *pci_iomap_range(struct pci_dev *dev, unsigned long offset, unsigned long maxlen) { - resource_size_t start = pci_resource_start(dev, bar); - resource_size_t len = pci_resource_len(dev, bar); - unsigned long flags = pci_resource_flags(dev, bar); + resource_size_t start, len; + unsigned long flags; + + if (!pci_bar_index_is_valid(bar)) + return NULL; + + start = pci_resource_start(dev, bar); + len = pci_resource_len(dev, bar); + flags = pci_resource_flags(dev, bar); if (len <= offset || !start) return NULL; + len -= offset; start += offset; if (maxlen && len > maxlen) @@ -77,16 +86,20 @@ void __iomem *pci_iomap_wc_range(struct pci_dev *dev, unsigned long offset, unsigned long maxlen) { - resource_size_t start = pci_resource_start(dev, bar); - resource_size_t len = pci_resource_len(dev, bar); - unsigned long flags = pci_resource_flags(dev, bar); + resource_size_t start, len; + unsigned long flags; - - if (flags & IORESOURCE_IO) + if (!pci_bar_index_is_valid(bar)) return NULL; + start = pci_resource_start(dev, bar); + len = pci_resource_len(dev, bar); + flags = pci_resource_flags(dev, bar); + if (len <= offset || !start) return NULL; + if (flags & IORESOURCE_IO) + return NULL; len -= offset; start += offset; diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 869d204a70a3..da82d734d09c 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3921,6 +3921,9 @@ EXPORT_SYMBOL(pci_enable_atomic_ops_to_root); */ void pci_release_region(struct pci_dev *pdev, int bar) { + if (!pci_bar_index_is_valid(bar)) + return; + /* * This is done for backwards compatibility, because the old PCI devres * API had a mode in which the function became managed if it had been @@ -3965,6 +3968,9 @@ EXPORT_SYMBOL(pci_release_region); static int __pci_request_region(struct pci_dev *pdev, int bar, const char *name, int exclusive) { + if (!pci_bar_index_is_valid(bar)) + return -EINVAL; + if (pci_is_managed(pdev)) { if (exclusive == IORESOURCE_EXCLUSIVE) return pcim_request_region_exclusive(pdev, bar, name); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 01e51db8d285..19af5491f674 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -167,6 +167,22 @@ static inline void pci_wakeup_event(struct pci_dev *dev) pm_wakeup_event(&dev->dev, 100); } +/** + * pci_bar_index_is_valid - check wehether a BAR index is within valid range + * @bar: the bar index + * + * Protects against overflowing &struct pci_dev.resource array. + * + * Return: true for valid index, false otherwise + */ +static inline bool pci_bar_index_is_valid(int bar) +{ + if (bar >= 0 || bar < PCI_NUM_RESOURCES) + return true; + + return false; +} + static inline bool pci_has_subordinate(struct pci_dev *pci_dev) { return !!(pci_dev->subordinate);