e1000: bounds packet size against buffer size

Otherwise we can write beyond the buffer and corrupt memory.  This is tracked
as CVE-2012-0029.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/e1000.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

On Mon, 23 Jan 2012, Anthony Liguori wrote:
> Otherwise we can write beyond the buffer and corrupt memory.  This is tracked
> as CVE-2012-0029.

The stable-1.0 branch looks vulnerable too, shouldn't this patch be
backported?


> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  hw/e1000.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/e1000.c b/hw/e1000.c
> index a29c944..86c5416 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -466,6 +466,8 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>              bytes = split_size;
>              if (tp->size + bytes > msh)
>                  bytes = msh - tp->size;
> +
> +            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);
> @@ -481,6 +483,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>          // context descriptor TSE is not set, while data descriptor TSE is set
>          DBGOUT(TXERR, "TCP segmentaion Error\n");
>      } else {
> +        split_size = MIN(sizeof(tp->data) - tp->size, split_size);
>          pci_dma_read(&s->dev, addr, tp->data + tp->size, split_size);
>          tp->size += split_size;
>      }
> -- 
> 1.7.4.1
> 
>

On 02.02.2012 15:15, Stefano Stabellini wrote:
> On Mon, 23 Jan 2012, Anthony Liguori wrote:
>> Otherwise we can write beyond the buffer and corrupt memory.  This is tracked
>> as CVE-2012-0029.
> 
> The stable-1.0 branch looks vulnerable too, shouldn't this patch be
> backported?

This goes on since forever - for example, this patch applies to 0.12
too (modulo pci_dma_read() changes which makes the context differ).
It applies cleanly to 1.0 stable.

/mjt

>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  hw/e1000.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/e1000.c b/hw/e1000.c
>> index a29c944..86c5416 100644
>> --- a/hw/e1000.c
>> +++ b/hw/e1000.c
>> @@ -466,6 +466,8 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>>              bytes = split_size;
>>              if (tp->size + bytes > msh)
>>                  bytes = msh - tp->size;
>> +
>> +            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);
>> @@ -481,6 +483,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>>          // context descriptor TSE is not set, while data descriptor TSE is set
>>          DBGOUT(TXERR, "TCP segmentaion Error\n");
>>      } else {
>> +        split_size = MIN(sizeof(tp->data) - tp->size, split_size);
>>          pci_dma_read(&s->dev, addr, tp->data + tp->size, split_size);
>>          tp->size += split_size;
>>      }
>> -- 
>> 1.7.4.1
>>
>>
>

Am 02.02.2012 16:24, schrieb Michael Tokarev:
> On 02.02.2012 15:15, Stefano Stabellini wrote:
>> On Mon, 23 Jan 2012, Anthony Liguori wrote:
>>> Otherwise we can write beyond the buffer and corrupt memory.  This is tracked
>>> as CVE-2012-0029.
>>
>> The stable-1.0 branch looks vulnerable too, shouldn't this patch be
>> backported?
> 
> This goes on since forever - for example, this patch applies to 0.12
> too (modulo pci_dma_read() changes which makes the context differ).
> It applies cleanly to 1.0 stable.

Therefore we should cc qemu-stable. :)

Andreas

>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>> ---
>>>  hw/e1000.c |    3 +++
>>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/hw/e1000.c b/hw/e1000.c
>>> index a29c944..86c5416 100644
>>> --- a/hw/e1000.c
>>> +++ b/hw/e1000.c
>>> @@ -466,6 +466,8 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>>>              bytes = split_size;
>>>              if (tp->size + bytes > msh)
>>>                  bytes = msh - tp->size;
>>> +
>>> +            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);
>>> @@ -481,6 +483,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>>>          // context descriptor TSE is not set, while data descriptor TSE is set
>>>          DBGOUT(TXERR, "TCP segmentaion Error\n");
>>>      } else {
>>> +        split_size = MIN(sizeof(tp->data) - tp->size, split_size);
>>>          pci_dma_read(&s->dev, addr, tp->data + tp->size, split_size);
>>>          tp->size += split_size;
>>>      }
>>> -- 
>>> 1.7.4.1