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 unsigned int similarly to what is already done in
dmar_scope_add_buses().

Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
v2:
- fix free_pdev() as well
- style fixes
---
 xen/drivers/passthrough/pci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index fc4fa2e..d65cda8 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -363,8 +363,7 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
     /* update bus2bridge */
     switch ( pdev->type = pdev_type(pseg->nr, bus, devfn) )
     {
-        u16 cap;
-        u8 sec_bus, sub_bus;
+        unsigned int cap, sec_bus, sub_bus;
 
         case DEV_TYPE_PCIe2PCI_BRIDGE:
         case DEV_TYPE_LEGACY_PCI_BRIDGE:
@@ -431,7 +430,7 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
     /* update bus2bridge */
     switch ( pdev->type )
     {
-        uint8_t sec_bus, sub_bus;
+        unsigned int sec_bus, sub_bus;
 
         case DEV_TYPE_PCIe2PCI_BRIDGE:
         case DEV_TYPE_LEGACY_PCI_BRIDGE:
-- 
2.7.4

On 24.09.2021 11:10, Igor Druzhinin wrote:
> 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 unsigned int similarly to what is already done in
> dmar_scope_add_buses().
> 
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Hi,

> On 24 Sep 2021, at 10:10, Igor Druzhinin <igor.druzhinin@citrix.com> wrote:
> 
> 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 unsigned int similarly to what is already done in
> dmar_scope_add_buses().
> 
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand


> ---
> v2:
> - fix free_pdev() as well
> - style fixes
> ---
> xen/drivers/passthrough/pci.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index fc4fa2e..d65cda8 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -363,8 +363,7 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
>     /* update bus2bridge */
>     switch ( pdev->type = pdev_type(pseg->nr, bus, devfn) )
>     {
> -        u16 cap;
> -        u8 sec_bus, sub_bus;
> +        unsigned int cap, sec_bus, sub_bus;
> 
>         case DEV_TYPE_PCIe2PCI_BRIDGE:
>         case DEV_TYPE_LEGACY_PCI_BRIDGE:
> @@ -431,7 +430,7 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
>     /* update bus2bridge */
>     switch ( pdev->type )
>     {
> -        uint8_t sec_bus, sub_bus;
> +        unsigned int sec_bus, sub_bus;
> 
>         case DEV_TYPE_PCIe2PCI_BRIDGE:
>         case DEV_TYPE_LEGACY_PCI_BRIDGE:
> -- 
> 2.7.4
> 
>