From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42487) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ujp7D-0005R0-Dk for qemu-devel@nongnu.org; Tue, 04 Jun 2013 07:09:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UjowA-000683-ON for qemu-devel@nongnu.org; Tue, 04 Jun 2013 06:58:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43951) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UjowA-00067c-Af for qemu-devel@nongnu.org; Tue, 04 Jun 2013 06:57:58 -0400 Date: Tue, 4 Jun 2013 13:58:30 +0300 From: "Michael S. Tsirkin" Message-ID: <20130604105830.GA13194@redhat.com> References: <1370272838-15373-1-git-send-email-drjones@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1370272838-15373-1-git-send-email-drjones@redhat.com> Subject: Re: [Qemu-devel] [PATCH] e1000: cleanup process_tx_desc List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andrew Jones Cc: jasowang@redhat.com, pbonzini@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, stefanha@redhat.com On Mon, Jun 03, 2013 at 05:20:38PM +0200, Andrew Jones wrote: > Coverity complains about two overruns in process_tx_desc(). The > complaints are false positives, but we might as well eliminate > them. The problem is that "hdr" is defined as an unsigned int, > but then used to offset an array of size 65536, and another of > size 256 bytes. hdr will actually never be greater than 255 > though, as it's assigned only once and to the value of > tp->hdr_len, which is an uint8_t. This patch simply gets rid of > hdr, replacing it with tp->hdr_len, which makes it consistent > with all other tp member use in the function. > > Signed-off-by: Andrew Jones Applied, thanks. > --- > hw/net/e1000.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/hw/net/e1000.c b/hw/net/e1000.c > index e6f46f0c511e8..eec3e7a4524d1 100644 > --- a/hw/net/e1000.c > +++ b/hw/net/e1000.c > @@ -556,7 +556,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp) > uint32_t txd_lower = le32_to_cpu(dp->lower.data); > uint32_t dtype = txd_lower & (E1000_TXD_CMD_DEXT | E1000_TXD_DTYP_D); > unsigned int split_size = txd_lower & 0xffff, bytes, sz, op; > - unsigned int msh = 0xfffff, hdr = 0; > + unsigned int msh = 0xfffff; > uint64_t addr; > struct e1000_context_desc *xp = (struct e1000_context_desc *)dp; > struct e1000_tx *tp = &s->tx; > @@ -603,8 +603,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp) > > addr = le64_to_cpu(dp->buffer_addr); > if (tp->tse && tp->cptse) { > - hdr = tp->hdr_len; > - msh = hdr + tp->mss; > + msh = tp->hdr_len + tp->mss; > do { > bytes = split_size; > if (tp->size + bytes > msh) > @@ -612,14 +611,15 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp) > > bytes = MIN(sizeof(tp->data) - tp->size, bytes); > pci_dma_read(&s->dev, addr, tp->data + tp->size, bytes); > - if ((sz = tp->size + bytes) >= hdr && tp->size < hdr) > - memmove(tp->header, tp->data, hdr); > + if ((sz = tp->size + bytes) >= tp->hdr_len > + && tp->size < tp->hdr_len) > + memmove(tp->header, tp->data, tp->hdr_len); > tp->size = sz; > addr += bytes; > if (sz == msh) { > xmit_seg(s); > - memmove(tp->data, tp->header, hdr); > - tp->size = hdr; > + memmove(tp->data, tp->header, tp->hdr_len); > + tp->size = tp->hdr_len; > } > } while (split_size -= bytes); > } else if (!tp->tse && tp->cptse) { > @@ -633,7 +633,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp) > > if (!(txd_lower & E1000_TXD_CMD_EOP)) > return; > - if (!(tp->tse && tp->cptse && tp->size < hdr)) > + if (!(tp->tse && tp->cptse && tp->size < tp->hdr_len)) > xmit_seg(s); > tp->tso_frames = 0; > tp->sum_needed = 0; > -- > 1.8.1.4