Skip to contentSkip to conversation

PCI: define max devices number on PCIBus as PCIBus_MAX_DEVICES in pci.h

PATCH In review Ken CC 3 participants 6 messages Aug 23, 2010 – Aug 24, 2010 KVM

lore
Share & tools
lore

Ken CC

author
#0 lore raw

PATCH 1/2 PCI: define max devices number on PCIBus as PCIBus_MAX_DEVICES in pci.h

Signed-off-by Ken CC <[email protected]>
---
 hw/pci.c |    2 +-
 hw/pci.h |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)
hw/pci.c +1 -1
index a98d6f3..a09fbac 100644
@@ -44,7 +44,7 @@ struct PCIBus {
     pci_hotplug_fn hotplug;
     DeviceState *hotplug_qdev;
     void *irq_opaque;
-    PCIDevice *devices[256];
+    PCIDevice *devices[PCIBUS_MAX_DEVICES];
     PCIDevice *parent_dev;
     target_phys_addr_t mem_base;
hw/pci.h +1 -0
index 1eab7e7..5f6c85b 100644
@@ -92,6 +92,7 @@ typedef struct PCIIORegion {
 
 #define PCI_ROM_SLOT 6
 #define PCI_NUM_REGIONS 7
+#define PCIBUS_MAX_DEVICES 256
 
 #include "pci_regs.h"
#1 lore raw

PATCH 1/1 pci init: Check if devfn exceeding the max devices number supported on bus

Check before trying subindexing.

Signed-off-by Ken CC <[email protected]>
---
 hw/pci.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
hw/pci.c +4 -0
index a09fbac..f6f00c6 100644
@@ -675,6 +675,10 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
         error_report("PCI: no slot/function available for %s, all in use", name);
         return NULL;
     found: ;
+    } else if (devfn > PCIBUS_MAX_DEVICES - 1) {
+        error_report("PCI: devfn is out of bus capacity."
+                        " Only %d devices supported.", PCIBUS_MAX_DEVICES);
+        return NULL;
     } else if (bus->devices[devfn]) {
         error_report("PCI: slot %d function %d not available for %s, in use by %s",
                      PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name);
author
#2 lore raw

PATCH 2/2 pci init: Check if devfn exceeding the max devices number supported on bus

In reply to Ken CC

Check before trying subindexing.

Signed-off-by Ken CC <[email protected]>
---
 hw/pci.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
hw/pci.c +4 -0
index a09fbac..f6f00c6 100644
@@ -675,6 +675,10 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
         error_report("PCI: no slot/function available for %s, all in use", name);
         return NULL;
     found: ;
+    } else if (devfn > PCIBUS_MAX_DEVICES - 1) {
+        error_report("PCI: devfn is out of bus capacity."
+                        " Only %d devices supported.", PCIBUS_MAX_DEVICES);
+        return NULL;
     } else if (bus->devices[devfn]) {
         error_report("PCI: slot %d function %d not available for %s, in use by %s",
                      PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name);
#3 lore raw

Re: [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus

How did you trigger the bug?
I suppose parse_pci_devfn() in qdev-properties should check the error. Although I'm not objecting this patch itself, it's caller's bug. Just assert(devfn < PCIBUS_MAX_DEVICES)?

Quoted 26 lines · Ken CC

Check before trying subindexing.

Signed-off-by Ken CC <[email protected]>
---
 hw/pci.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
hw/pci.c +4 -0
index a09fbac..f6f00c6 100644
@@ -675,6 +675,10 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
         error_report("PCI: no slot/function available for %s, all in use", name);
         return NULL;
     found: ;
+    } else if (devfn > PCIBUS_MAX_DEVICES - 1) {
+        error_report("PCI: devfn is out of bus capacity."
+                        " Only %d devices supported.", PCIBUS_MAX_DEVICES);
+        return NULL;
     } else if (bus->devices[devfn]) {
         error_report("PCI: slot %d function %d not available for %s, in use by %s",
                      PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name);
Signature · 1 line
yamahata
author
#4 lore raw

Re: [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus

Quoted 6 lines · Isaku Yamahata

How did you trigger the bug?
I suppose parse_pci_devfn() in qdev-properties should check the error. Although I'm not objecting this patch itself, it's caller's bug. Just assert(devfn < PCIBUS_MAX_DEVICES)?

assign pci addr > 30,

e.g.
qemu-system-x86_64 -drive \
file='./fedora.13-x86_64-virtio.qcow2',if=none,id=drv-virtio-d1,media=disk,cache=none,boot=on,format=qcow2 \

-device virtio-blk-pci,bus=pci.0,addr=0x90,drive=drv-virtio-d1,id=virtio-disk1 \
                                      ^^^^

-m 2048 -vnc :11

assert() is good to me.
and i think checking in the caller should be done too.

Quoted 30 lines · Ken CC
Quoted 26 lines

Check before trying subindexing.

Signed-off-by Ken CC <[email protected]>
---
 hw/pci.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
hw/pci.c +4 -0
index a09fbac..f6f00c6 100644
@@ -675,6 +675,10 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
         error_report("PCI: no slot/function available for %s, all in use", name);
         return NULL;
     found: ;
+    } else if (devfn > PCIBUS_MAX_DEVICES - 1) {
+        error_report("PCI: devfn is out of bus capacity."
+                        " Only %d devices supported.", PCIBUS_MAX_DEVICES);
+        return NULL;
     } else if (bus->devices[devfn]) {
         error_report("PCI: slot %d function %d not available for %s, in use by %s",
                      PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name);
Signature · 1 line
yamahata
#5 lore raw

Re: [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus

Quoted 13 lines · Ken CC
Quoted 6 lines · Isaku Yamahata

How did you trigger the bug?
I suppose parse_pci_devfn() in qdev-properties should check the error. Although I'm not objecting this patch itself, it's caller's bug. Just assert(devfn < PCIBUS_MAX_DEVICES)?

assign pci addr > 30,

e.g.
qemu-system-x86_64 -drive \
file='./fedora.13-x86_64-virtio.qcow2',if=none,id=drv-virtio-d1,media=disk,cache=none,boot=on,format=qcow2 \

Review

Quoted 3 lines
(quoted diff) +0 -2
-device virtio-blk-pci,bus=pci.0,addr=0x90,drive=drv-virtio-d1,id=virtio-disk1 \
                                      ^^^^
-m 2048 -vnc :11

So parse_pci_devfn() in qdev-properties.c lacks the necessary check. It checks fn range, but doesn't check slot range.
So please add the slot check into parse_pci_devfn(), and add assert() into do_pci_register_device().

thanks,

Quoted 38 lines

assert() is good to me.
and i think checking in the caller should be done too.

Quoted 30 lines · Ken CC
Quoted 26 lines

Check before trying subindexing.

Signed-off-by Ken CC <[email protected]>
---
 hw/pci.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
hw/pci.c +4 -0
index a09fbac..f6f00c6 100644
@@ -675,6 +675,10 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
         error_report("PCI: no slot/function available for %s, all in use", name);
         return NULL;
     found: ;
+    } else if (devfn > PCIBUS_MAX_DEVICES - 1) {
+        error_report("PCI: devfn is out of bus capacity."
+                        " Only %d devices supported.", PCIBUS_MAX_DEVICES);
+        return NULL;
     } else if (bus->devices[devfn]) {
         error_report("PCI: slot %d function %d not available for %s, in use by %s",
                      PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name);
Signature · 1 line
yamahata
Signature · 1 line
yamahata