[PATCH] pci: fix handling of PCI bridges with subordinate bus number 0xff

Igor Druzhinin posted 1 patch 3 years, 2 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
xen/drivers/passthrough/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] pci: fix handling of PCI bridges with subordinate bus number 0xff
Posted by Igor Druzhinin 3 years, 2 months ago
Bus number 0xff is valid according to the PCI spec. Using u8 typed sub_bus
and assigning 0xff to it will result in the following loop getting stuck.

    for ( ; sec_bus <= sub_bus; sec_bus++ ) {...}

Just change its type to u16 the same way that is already handled in
dmar_scope_add_buses().

Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
 xen/drivers/passthrough/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index fc4fa2e..48b415c 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -364,7 +364,7 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
     switch ( pdev->type = pdev_type(pseg->nr, bus, devfn) )
     {
         u16 cap;
-        u8 sec_bus, sub_bus;
+        u16 sec_bus, sub_bus;
 
         case DEV_TYPE_PCIe2PCI_BRIDGE:
         case DEV_TYPE_LEGACY_PCI_BRIDGE:
-- 
2.7.4


Re: [PATCH] pci: fix handling of PCI bridges with subordinate bus number 0xff
Posted by Jan Beulich 3 years, 2 months ago
On 24.09.2021 03:06, Igor Druzhinin wrote:
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -364,7 +364,7 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
>      switch ( pdev->type = pdev_type(pseg->nr, bus, devfn) )
>      {
>          u16 cap;
> -        u8 sec_bus, sub_bus;
> +        u16 sec_bus, sub_bus;
>  
>          case DEV_TYPE_PCIe2PCI_BRIDGE:
>          case DEV_TYPE_LEGACY_PCI_BRIDGE:

First of all you want to also address the same issue in free_pdev()
then. Further, since we're switching away from u16, uint16_t would
be the legitimate replacement. Plus, since cap is then of the same
type, fold all three variable declarations into a single line.
Finally, as per ./CODING_STYLE, fixed width types should be used
only where strictly needed. I can't see a reason for any of these
to be other than "unsigned int".

Jan