From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34415) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrLLu-0007yE-8k for qemu-devel@nongnu.org; Thu, 20 Nov 2014 01:36:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XrLLo-00019F-EP for qemu-devel@nongnu.org; Thu, 20 Nov 2014 01:36:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36150) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrLLo-00018v-6R for qemu-devel@nongnu.org; Thu, 20 Nov 2014 01:36:20 -0500 Message-ID: <546D8BDA.9080205@redhat.com> Date: Thu, 20 Nov 2014 07:36:10 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1416463034-8264-1-git-send-email-arei.gonglei@huawei.com> <1416463034-8264-4-git-send-email-arei.gonglei@huawei.com> In-Reply-To: <1416463034-8264-4-git-send-email-arei.gonglei@huawei.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/4] pcnet: fix Negative array index read List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: arei.gonglei@huawei.com, qemu-devel@nongnu.org Cc: peter.huangpeng@huawei.com, stefanha@redhat.com On 20/11/2014 06:57, arei.gonglei@huawei.com wrote: > From: Gonglei > > s->xmit_pos maybe assigned to a negative value (-1), > but in this branch variable s->xmit_pos as an index to > array s->buffer. Let's add a check for s->xmit_pos. > > Signed-off-by: Gonglei > --- > hw/net/pcnet.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c > index d344c15..2bb6417 100644 > --- a/hw/net/pcnet.c > +++ b/hw/net/pcnet.c > @@ -1247,7 +1247,7 @@ static void pcnet_transmit(PCNetState *s) > s->xmit_pos = -1; > goto txdone; > } > - if (!GET_FIELD(tmd.status, TMDS, ENP)) { > + if (!GET_FIELD(tmd.status, TMDS, ENP) && (s->xmit_pos >= 0)) { > int bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT); > s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr), > s->buffer + s->xmit_pos, bcnt, CSR_BSWP(s)); > Even better: if (s->xmit_pos < 0) { goto txdone; } bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT); s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr), s->buffer + s->xmit_pos, bcnt, CSR_BSWP(s)); s->xmit_pos += bcnt; if (FIELD(tmd.status, TMDS, ENP)) { #ifdef PCNET_DEBUG printf("pcnet_transmit size=%d\n", s->xmit_pos); #endif if (CSR_LOOP(s)) { if (BCR_SWSTYLE(s) == 1) ... } since there is duplicated code in the two "if" arms. But the call is Stefan's, as he's the maintainer. Paolo