From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38021) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UWARA-0007u8-SN for qemu-devel@nongnu.org; Sat, 27 Apr 2013 15:05:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UWAR8-000588-MW for qemu-devel@nongnu.org; Sat, 27 Apr 2013 15:05:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53552) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UWAR8-000583-Ev for qemu-devel@nongnu.org; Sat, 27 Apr 2013 15:05:30 -0400 Date: Sat, 27 Apr 2013 22:05:20 +0300 From: "Michael S. Tsirkin" Message-ID: <20130427190520.GA14920@redhat.com> References: <1366965244-20542-1-git-send-email-jasowang@redhat.com> <20130426142754.GO8388@dhcp-25-225.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130426142754.GO8388@dhcp-25-225.brq.redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/3] virtio-pci: properly validate address before accessing config List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Petr Matousek Cc: Jason Wang , aliguori@us.ibm.com, qemu-devel@nongnu.org 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 > > Cc: Petr Matousek > > Signed-off-by: Jason Wang > > --- > > 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