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

Isaku Yamahata
2010-08-24T02:07:50Z

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