Re: [PATCH] hw/net/ftgmac100: Fix integer overflow in ftgmac100_do_tx()

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 10 Jul 2020 at 09:56, Mauro Matteo Cascella <mcascell@redhat.com> wrote:
>
> An integer overflow issue was reported by Mr. Ziming Zhang, CC'd here. It
> occurs while inserting the VLAN tag in packets whose length is less than
> 12 bytes, as (len-12) is passed to memmove() without proper checking.
> This patch is intended to fix this issue by checking the minimum
> Ethernet frame size during packet transmission.
>
> Reported-by: Ziming Zhang <ezrakiez@gmail.com>
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> ---
>  hw/net/ftgmac100.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
> index 043ba61b86..bcf4d84aea 100644
> --- a/hw/net/ftgmac100.c
> +++ b/hw/net/ftgmac100.c
> @@ -238,6 +238,11 @@ typedef struct {
>   */
>  #define FTGMAC100_MAX_FRAME_SIZE    9220
>
> +/*
> + * Min frame size
> + */
> +#define FTGMAC100_MIN_FRAME_SIZE    64
> +
>  /* Limits depending on the type of the frame
>   *
>   *   9216 for Jumbo frames (+ 4 for VLAN)
> @@ -507,6 +512,15 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
>          }
>
>          len = FTGMAC100_TXDES0_TXBUF_SIZE(bd.des0);
> +
> +        /* drop small packets */
> +        if (bd.des0 & FTGMAC100_TXDES0_FTS &&
> +            len < FTGMAC100_MIN_FRAME_SIZE) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too small: %d bytes\n",
> +                          __func__, len);
> +            break;
> +        }
> +

Andrew, Cedric: do you have the datasheet for this devic? Do you
know if we should also be flagging the error back to the
guest somehow?

I think a 'break' here means we'll never update the
descriptor flags to hand it back to the guest, which
is probably not what the hardware does.

thanks
-- PMM