From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39158) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVTJe-0001Ar-Ax for qemu-devel@nongnu.org; Thu, 25 Apr 2013 17:02:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UVTJb-00069I-VT for qemu-devel@nongnu.org; Thu, 25 Apr 2013 17:02:54 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58249) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVTJb-000698-P5 for qemu-devel@nongnu.org; Thu, 25 Apr 2013 17:02:51 -0400 Date: Fri, 26 Apr 2013 00:02:42 +0300 From: "Michael S. Tsirkin" Message-ID: <20130425210242.GB2908@redhat.com> References: <1366875807-3491-1-git-send-email-jasowang@redhat.com> <87fvyebbwb.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87fvyebbwb.fsf@codemonkey.ws> Subject: Re: [Qemu-devel] [PATCH] virtio: abort on zero config length List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Jason Wang , qemu-devel@nongnu.org On Thu, Apr 25, 2013 at 03:20:20PM -0500, Anthony Liguori wrote: > Jason Wang writes: > > > In fact we don't support zero length config length for virtio device. > > virtio-rng? It has config_len == 0? In that case guest using virtio-rng can crash qemu or read qemu memory: uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr) { uint8_t val; vdev->get_config(vdev, vdev->config); if (addr > (vdev->config_len - sizeof(val))) ^^^^^^^^^ quiz: spot a bug above if config_len is 0 :) return (uint32_t)-1; val = ldub_p(vdev->config + addr); return val; } how about corrupting qemu memory? That too: void virtio_config_writeb(VirtIODevice *vdev, uint32_t addr, uint32_t data) { uint8_t val = data; if (addr > (vdev->config_len - sizeof(val))) return; stb_p(vdev->config + addr, val); if (vdev->set_config) vdev->set_config(vdev, vdev->config); } > > And it can lead outbound memory access. So abort on zero config length > > to catch the bug earlier. > > Not sure what you mean, but virtio-rng has a zero length config space. > > Regards, > > Anthony Liguori > > > > > Signed-off-by: Jason Wang > > --- > > hw/virtio/virtio.c | 7 ++----- > > 1 files changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > > index 1c2282c..a6fa667 100644 > > --- a/hw/virtio/virtio.c > > +++ b/hw/virtio/virtio.c > > @@ -923,6 +923,7 @@ void virtio_init(VirtIODevice *vdev, const char *name, > > uint16_t device_id, size_t config_size) > > { > > int i; > > + assert(config_size); > > vdev->device_id = device_id; > > vdev->status = 0; > > vdev->isr = 0; > > @@ -938,11 +939,7 @@ void virtio_init(VirtIODevice *vdev, const char *name, > > > > vdev->name = name; > > vdev->config_len = config_size; > > - if (vdev->config_len) { > > - vdev->config = g_malloc0(config_size); > > - } else { > > - vdev->config = NULL; > > - } > > + vdev->config = g_malloc0(config_size); > > vdev->vmstate = qemu_add_vm_change_state_handler(virtio_vmstate_change, > > vdev); > > } > > -- > > 1.7.1