On Tue, Jun 04, 2013 at 10:49:48AM +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.
>
> v2:
> - also cleanup coding style issues in the touched lines
>
> Signed-off-by: Andrew Jones <drjo...@redhat.com>
I've picked this one up.
Thanks,
MST
> ---
> hw/net/e1000.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index e6f46f0c511e8..620f947134780 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,16 @@ 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);
> + sz = tp->size + bytes;
> + if (sz >= 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,8 +634,9 @@ 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;
> tp->vlan_needed = 0;
> --
> 1.8.1.4
