PCI: Check BAR index for validity

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.

Link: https://lore.kernel.org/r/20250312080634.13731-4-phasta@kernel.org
Closes: https://lore.kernel.org/all/adb53b1f-29e1-3d14-0e61-351fd2d3ff0d@linux.intel.com/
Reported-by: Bingbu Cao <bingbu.cao@linux.intel.com>
Signed-off-by: Philipp Stanner <phasta@kernel.org>
[kwilczynski: correct if-statement condition the pci_bar_index_is_valid()
helper function uses, tidy up code comments]
Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
[bhelgaas: fix typo]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 728ed0c7f70ac1fc39a90c21dde25db15d83a42e..73047316889e50f8709a93b8405320da421f20f8 100644 (file)
--- 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 9fb7cacc15cdef4a2701e140c7fc485283ca9fbf..fe706ed946dfd260ad680c28ec1a9c8a143bf3f2 100644 (file)
--- a/drivers/pci/iomap.c
+++ b/drivers/pci/iomap.c
@@ -9,6 +9,8 @@
 
 #include <linux/export.h>
 
+#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 869d204a70a372d240b1a6988a2159a79999e1ca..da82d734d09c0449779bd09442b74a38e6213d88 100644 (file)
--- 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 01e51db8d285af54673db3ea526ceda073c94ec9..d22755de688b8139ec95787137252d49a5b16994 100644 (file)
--- 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 whether a BAR index is within valid range
+ * @bar: 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);