Re: [Qemu-devel] [PATCH] net: cadence_gem: check packet size in gem_recieve

[Jason Wang <jasowang@redhat.com>]

On 01/15/2016 03:00 PM, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> While receiving packets in 'gem_receive' routine, if Frame Check
> Sequence(FCS) is enabled, it copies the packet into a local
> buffer without checking its size. Add check to validate packet
> length against the buffer size to avoid buffer overflow.
>
> Reported-by: Ling Liu <liuling-it@360.cn>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/net/cadence_gem.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 3639fc1..c3a273b 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -677,10 +677,14 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>      } else {
>          unsigned crc_val;
>  
> +        if (size > sizeof(rxbuf) - sizeof(crc_val)) {
> +            size = sizeof(rxbuf) - sizeof(crc_val);
> +        }
> +        bytes_to_copy = size;
> +
>          /* The application wants the FCS field, which QEMU does not provide.
>           * We must try and calculate one.
>           */
> -

Unnecessary whitespace change.

>          memcpy(rxbuf, buf, size);

We probably need more check, is there any guarantee that size <= 2048?
If not, need fix.

Thanks

>          memset(rxbuf + size, 0, sizeof(rxbuf) - size);
>          rxbuf_ptr = rxbuf;