* [Qemu-devel] [PATCH] PCI: define max devices number on PCIBus as PCIBus_MAX_DEVICES in pci.h @ 2010-08-12 9:25 Ken CC 2010-08-12 10:00 ` [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus Ken CC 0 siblings, 1 reply; 9+ messages in thread From: Ken CC @ 2010-08-12 9:25 UTC (permalink / raw) To: qemu-devel Signed-off-by: Ken CC <ken.ccao@gmail.com> --- hw/pci.c | 2 +- hw/pci.h | 1 + 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index a98d6f3..a09fbac 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -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; diff --git a/hw/pci.h b/hw/pci.h index 1eab7e7..5f6c85b 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -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" ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus 2010-08-12 9:25 [Qemu-devel] [PATCH] PCI: define max devices number on PCIBus as PCIBus_MAX_DEVICES in pci.h Ken CC @ 2010-08-12 10:00 ` Ken CC 0 siblings, 0 replies; 9+ messages in thread From: Ken CC @ 2010-08-12 10:00 UTC (permalink / raw) To: qemu-devel Check before trying subindexing. Signed-off-by: Ken CC <ken.ccao@gmail.com> --- hw/pci.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index a09fbac..f6f00c6 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -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); ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 1/2] PCI: define max devices number on PCIBus as PCIBus_MAX_DEVICES in pci.h @ 2010-08-23 5:46 Ken CC 2010-08-23 5:56 ` [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus Ken CC 0 siblings, 1 reply; 9+ messages in thread From: Ken CC @ 2010-08-23 5:46 UTC (permalink / raw) To: kvm, mtosatti; +Cc: qemu-devel Signed-off-by: Ken CC <ken.ccao@gmail.com> --- hw/pci.c | 2 +- hw/pci.h | 1 + 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index a98d6f3..a09fbac 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -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; diff --git a/hw/pci.h b/hw/pci.h index 1eab7e7..5f6c85b 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -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" ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus 2010-08-23 5:46 [PATCH 1/2] PCI: define max devices number on PCIBus as PCIBus_MAX_DEVICES in pci.h Ken CC @ 2010-08-23 5:56 ` Ken CC 2010-08-23 9:56 ` Isaku Yamahata 0 siblings, 1 reply; 9+ messages in thread From: Ken CC @ 2010-08-23 5:56 UTC (permalink / raw) To: kvm, mtosatti; +Cc: qemu-devel Check before trying subindexing. Signed-off-by: Ken CC <ken.ccao@gmail.com> --- hw/pci.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index a09fbac..f6f00c6 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -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); ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus 2010-08-23 5:56 ` [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus Ken CC @ 2010-08-23 9:56 ` Isaku Yamahata 0 siblings, 0 replies; 9+ messages in thread From: Isaku Yamahata @ 2010-08-23 9:56 UTC (permalink / raw) To: Ken CC; +Cc: kvm, mtosatti, qemu-devel 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)? On Mon, Aug 23, 2010 at 01:56:31PM +0800, Ken CC wrote: > > Check before trying subindexing. > > Signed-off-by: Ken CC <ken.ccao@gmail.com> > --- > hw/pci.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index a09fbac..f6f00c6 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -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); > > > -- yamahata ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus @ 2010-08-23 9:56 ` Isaku Yamahata 0 siblings, 0 replies; 9+ messages in thread From: Isaku Yamahata @ 2010-08-23 9:56 UTC (permalink / raw) To: Ken CC; +Cc: mtosatti, qemu-devel, kvm 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)? On Mon, Aug 23, 2010 at 01:56:31PM +0800, Ken CC wrote: > > Check before trying subindexing. > > Signed-off-by: Ken CC <ken.ccao@gmail.com> > --- > hw/pci.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index a09fbac..f6f00c6 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -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); > > > -- yamahata ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus 2010-08-23 9:56 ` Isaku Yamahata @ 2010-08-24 1:43 ` Ken CC -1 siblings, 0 replies; 9+ messages in thread From: Ken CC @ 2010-08-24 1:43 UTC (permalink / raw) To: Isaku Yamahata; +Cc: kvm, mtosatti, qemu-devel On Mon, Aug 23, 2010 at 06:56:56PM +0900, Isaku Yamahata wrote: > > 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. > On Mon, Aug 23, 2010 at 01:56:31PM +0800, Ken CC wrote: > > > > Check before trying subindexing. > > > > Signed-off-by: Ken CC <ken.ccao@gmail.com> > > --- > > hw/pci.c | 4 ++++ > > 1 files changed, 4 insertions(+), 0 deletions(-) > > > > diff --git a/hw/pci.c b/hw/pci.c > > index a09fbac..f6f00c6 100644 > > --- a/hw/pci.c > > +++ b/hw/pci.c > > @@ -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); > > > > > > > > -- > yamahata -- ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus @ 2010-08-24 1:43 ` Ken CC 0 siblings, 0 replies; 9+ messages in thread From: Ken CC @ 2010-08-24 1:43 UTC (permalink / raw) To: Isaku Yamahata; +Cc: mtosatti, qemu-devel, kvm On Mon, Aug 23, 2010 at 06:56:56PM +0900, Isaku Yamahata wrote: > > 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. > On Mon, Aug 23, 2010 at 01:56:31PM +0800, Ken CC wrote: > > > > Check before trying subindexing. > > > > Signed-off-by: Ken CC <ken.ccao@gmail.com> > > --- > > hw/pci.c | 4 ++++ > > 1 files changed, 4 insertions(+), 0 deletions(-) > > > > diff --git a/hw/pci.c b/hw/pci.c > > index a09fbac..f6f00c6 100644 > > --- a/hw/pci.c > > +++ b/hw/pci.c > > @@ -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); > > > > > > > > -- > yamahata -- ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus 2010-08-24 1:43 ` Ken CC @ 2010-08-24 2:18 ` Isaku Yamahata -1 siblings, 0 replies; 9+ messages in thread From: Isaku Yamahata @ 2010-08-24 2:18 UTC (permalink / raw) To: Ken CC; +Cc: kvm, mtosatti, qemu-devel On Tue, Aug 24, 2010 at 09:43:25AM +0800, Ken CC wrote: > On Mon, Aug 23, 2010 at 06:56:56PM +0900, Isaku Yamahata wrote: > > > > 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 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, > > > assert() is good to me. > and i think checking in the caller should be done too. > > > On Mon, Aug 23, 2010 at 01:56:31PM +0800, Ken CC wrote: > > > > > > Check before trying subindexing. > > > > > > Signed-off-by: Ken CC <ken.ccao@gmail.com> > > > --- > > > hw/pci.c | 4 ++++ > > > 1 files changed, 4 insertions(+), 0 deletions(-) > > > > > > diff --git a/hw/pci.c b/hw/pci.c > > > index a09fbac..f6f00c6 100644 > > > --- a/hw/pci.c > > > +++ b/hw/pci.c > > > @@ -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); > > > > > > > > > > > > > -- > > yamahata > > -- > -- yamahata ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus @ 2010-08-24 2:18 ` Isaku Yamahata 0 siblings, 0 replies; 9+ messages in thread From: Isaku Yamahata @ 2010-08-24 2:18 UTC (permalink / raw) To: Ken CC; +Cc: mtosatti, qemu-devel, kvm On Tue, Aug 24, 2010 at 09:43:25AM +0800, Ken CC wrote: > On Mon, Aug 23, 2010 at 06:56:56PM +0900, Isaku Yamahata wrote: > > > > 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 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, > > > assert() is good to me. > and i think checking in the caller should be done too. > > > On Mon, Aug 23, 2010 at 01:56:31PM +0800, Ken CC wrote: > > > > > > Check before trying subindexing. > > > > > > Signed-off-by: Ken CC <ken.ccao@gmail.com> > > > --- > > > hw/pci.c | 4 ++++ > > > 1 files changed, 4 insertions(+), 0 deletions(-) > > > > > > diff --git a/hw/pci.c b/hw/pci.c > > > index a09fbac..f6f00c6 100644 > > > --- a/hw/pci.c > > > +++ b/hw/pci.c > > > @@ -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); > > > > > > > > > > > > > -- > > yamahata > > -- > -- yamahata ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-08-24 2:07 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-08-12 9:25 [Qemu-devel] [PATCH] PCI: define max devices number on PCIBus as PCIBus_MAX_DEVICES in pci.h Ken CC 2010-08-12 10:00 ` [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus Ken CC 2010-08-23 5:46 [PATCH 1/2] PCI: define max devices number on PCIBus as PCIBus_MAX_DEVICES in pci.h Ken CC 2010-08-23 5:56 ` [Qemu-devel] [PATCH 2/2] pci init: Check if devfn exceeding the max devices number supported on bus Ken CC 2010-08-23 9:56 ` Isaku Yamahata 2010-08-23 9:56 ` Isaku Yamahata 2010-08-24 1:43 ` Ken CC 2010-08-24 1:43 ` Ken CC 2010-08-24 2:18 ` Isaku Yamahata 2010-08-24 2:18 ` Isaku Yamahata
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.