On 04/09/15 18:21, Stefan Hajnoczi wrote:
> From: P J P <p...@fedoraproject.org>
>
> While processing transmit descriptors, it could lead to an infinite
> loop if 'bytes' was to become zero; Add a check to avoid it.
>
> [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 = tp->hdr_len;
> }
> - } while (split_size -= bytes);
> + split_size -= bytes;
> + } while (bytes && split_size);
> } else if (!tp->tse && tp->cptse) {
> // context descriptor TSE is not set, while data descriptor TSE 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 == 0, so that e.g. the pci_dma_read(..., 0) is not called at all).
Reviewed-by: Thomas Huth <th...@redhat.com>
