From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60040) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGoVJ-0008UC-N3 for qemu-devel@nongnu.org; Tue, 03 Sep 2013 07:10:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VGoVC-0006OF-Uy for qemu-devel@nongnu.org; Tue, 03 Sep 2013 07:10:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54081) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGoVC-0006O5-F9 for qemu-devel@nongnu.org; Tue, 03 Sep 2013 07:10:30 -0400 Date: Tue, 3 Sep 2013 16:40:21 +0530 From: Amit Shah Message-ID: <20130903111021.GA2464@grmbl.mre> References: <2952D48C-BC95-446D-92E8-04DE2BC3B873@cs2c.com.cn> <20130819143054.GA9866@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20130819143054.GA9866@stefanha-thinkpad.redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [qemu-devel]question on virtqueue_get_avail_bytes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: yinyin , "qemu-devel@nongnu.org" , "Michael S. Tsirkin" On (Mon) 19 Aug 2013 [16:30:54], Stefan Hajnoczi wrote: > On Mon, Aug 19, 2013 at 05:28:44PM +0800, yinyin wrote: > > Hi,all: > > in func virtqueue_get_avail_bytes=EF=BC=8C when found a indirect des= c, we need loop over it. > > /* loop over the indirect descriptor table */ > > indirect =3D 1; > > max =3D vring_desc_len(desc_pa, i) / sizeof(VRingDesc); > > num_bufs =3D i =3D 0; > > desc_pa =3D vring_desc_addr(desc_pa, i); > > But, It init i to 0, then use i to update desc_pa. so we will always= get : > > desc_pa =3D vring_desc_addr(desc_pa, 0); > > is it right=EF=BC=9For should we update desc_pa first, then init i t= o 0? >=20 > Is there a way to trigger a crash or erorr from a normal running guest? >=20 > Affected devices: serial, rng, and net - they call > virtqueue_get_avail_bytes() directly or indirectly. >=20 > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > > index 09f62c6..554ae6f 100644 > > --- a/hw/virtio/virtio.c > > +++ b/hw/virtio/virtio.c > > @@ -377,8 +377,8 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, uns= igned int *in_bytes, > > /* loop over the indirect descriptor table */ > > indirect =3D 1; > > max =3D vring_desc_len(desc_pa, i) / sizeof(VRingDesc); > > - num_bufs =3D i =3D 0; > > desc_pa =3D vring_desc_addr(desc_pa, i); > > + num_bufs =3D i =3D 0; >=20 > I agree, this looks wrong. git-blame(1) doesn't reveal anything > interesting. Looks like this bug has been around since 2009! Hm, why hasn't this bitten anyone yet? Amit