From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48117) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYsMC-0003cv-Jl for qemu-devel@nongnu.org; Mon, 07 Sep 2015 05:04:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZYsM9-0002Ki-CZ for qemu-devel@nongnu.org; Mon, 07 Sep 2015 05:04:56 -0400 References: <1441383666-6590-1-git-send-email-stefanha@redhat.com> From: Thomas Huth Message-ID: <55ED532F.2090905@redhat.com> Date: Mon, 7 Sep 2015 11:04:47 +0200 MIME-Version: 1.0 In-Reply-To: <1441383666-6590-1-git-send-email-stefanha@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] e1000: Avoid infinite loop in processing transmit descriptor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , qemu-devel@nongnu.org Cc: "Michael S. Tsirkin" , P J P , Jason Wang , qemu-stable@nongnu.org, P J P , Huzaifa Sidhpurwala On 04/09/15 18:21, Stefan Hajnoczi wrote: > From: P J P >=20 > While processing transmit descriptors, it could lead to an infinite > loop if 'bytes' was to become zero; Add a check to avoid it. >=20 > [The guest can force 'bytes' to 0 by setting the hdr_len and mss > descriptor fields to 0. > --Stefan] I wonder whether we should log an LOG_GUEST_ERROR in that case since this sounds like a problem in the guest ... ? > diff --git a/hw/net/e1000.c b/hw/net/e1000.c > index 5c6bcd0..09c9e9d 100644 > --- a/hw/net/e1000.c > +++ b/hw/net/e1000.c > @@ -740,7 +740,8 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc= *dp) > memmove(tp->data, tp->header, tp->hdr_len); > tp->size =3D tp->hdr_len; > } > - } while (split_size -=3D bytes); > + split_size -=3D bytes; > + } while (bytes && split_size); > } else if (!tp->tse && tp->cptse) { > // context descriptor TSE is not set, while data descriptor TS= E is set > DBGOUT(TXERR, "TCP segmentation error\n"); Looks sane ... (but IMHO code would be more readable though if it would break out of the loop already earlier, as soon as it is clear that bytes =3D=3D 0, so that e.g. the pci_dma_read(..., 0) is not called at al= l). Reviewed-by: Thomas Huth