Re: [Qemu-devel] [PATCH] cadence_gem: fix buffer overflow

[Peter Crosthwaite]

On Sun, Jan 17, 2016 at 10:50 PM, Jason Wang <address@hidden> wrote:
>
>
> On 01/14/2016 05:43 PM, Michael S. Tsirkin wrote:
>> gem_receive copies a packet received from network into an rxbuf[2048]
>> array on stack, with size limited by descriptor length set by guest.  If
>> guest is malicious and specifies a descriptor length that is too large,
>> and should packet size exceed array size, this results in a buffer
>> overflow.
>>
>> Reported-by: 刘令 <address@hidden>
>> Signed-off-by: Michael S. Tsirkin <address@hidden>
>> ---
>>  hw/net/cadence_gem.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>
> Apply to my -net with tweak on commit log (changing receive to transmit
> as noticed).
>

As this is actually an unimplemented feature you should change the
message to a LOG_UNIMP rather than a debug printf.

Regards,
Peter

> Thanks
>
>>
>> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
>> index 3639fc1..15a0786 100644
>> --- a/hw/net/cadence_gem.c
>> +++ b/hw/net/cadence_gem.c
>> @@ -862,6 +862,14 @@ static void gem_transmit(CadenceGEMState *s)
>>              break;
>>          }
>>
>> +        if (tx_desc_get_length(desc) > sizeof(tx_packet) - (p - tx_packet)) 
>> {
>> +            DB_PRINT("TX descriptor @ 0x%x too large: size 0x%x space 
>> 0x%x\n",
>> +                     (unsigned)packet_desc_addr,
>> +                     (unsigned)tx_desc_get_length(desc),
>> +                     sizeof(tx_packet) - (p - tx_packet));
>> +            break;
>> +        }
>> +
>>          /* Gather this fragment of the packet from "dma memory" to our 
>> contig.
>>           * buffer.
>>           */
>