On Fri, Apr 26, 2013 at 04:27:55PM +0200, Petr Matousek wrote: > On Fri, Apr 26, 2013 at 04:34:02PM +0800, Jason Wang wrote: > > There are several several issues in the current checking: > > > > - The check was based on the minus of unsigned values which can overflow > > - It was done after .{set|get}_config() which can lead crash when > > config_len is > > zero since vdev->config is NULL > > > > Fix this by: > > > > - Validate the address in virtio_pci_config_{read|write}() before > > .{set|get}_config > > - Use addition instead minus to do the validation > > > > Cc: Michael S. Tsirkin <m...@redhat.com> > > Cc: Petr Matousek <pmato...@redhat.com> > > Signed-off-by: Jason Wang <jasow...@redhat.com> > > --- > > hw/virtio/virtio-pci.c | 9 +++++++++ > > hw/virtio/virtio.c | 18 ------------------ > > 2 files changed, 9 insertions(+), 18 deletions(-) > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > index a1f15a8..7f6c7d1 100644 > > --- a/hw/virtio/virtio-pci.c > > +++ b/hw/virtio/virtio-pci.c > > @@ -400,6 +400,10 @@ static uint64_t virtio_pci_config_read(void *opaque, > > hwaddr addr, > > } > > addr -= config; > > > > + if (addr + size > proxy->vdev->config_len) { > > + return (uint32_t)-1; > > + } > > + > > What is the range of values addr can be? I guess it's not arbitrary and > not fully in guests hands. Can it be higher than corresponding pci > config space size? > > IOW, can guest touch anything interesting or will all accesses end in > the first page in the qemu address space, considering vdev->config being > NULL?
Good point. I think it's only the first page of the qemu address space. > -- > Petr Matousek / Red Hat Security Response Team