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

[Jason Wang <jasowang@redhat.com>]

On 01/18/2016 03:04 PM, Peter Crosthwaite wrote:
> On Sun, Jan 17, 2016 at 10:50 PM, Jason Wang <jasowang@redhat.com> 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: 刘令 <liuling-it@360.cn>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>  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 for the reminding. But we need know the whether real device could
send packet whose length is greater than 2048. Do you know the link to
the manual? (Haven't fond it in cadence page.) A hint is the linux
driver (drivers/net/ethernet/cadence/macb.c), use 1500 as its MTU.

>
>> 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.
>>>           */