On Fri, 10 Jul 2020 at 09:56, Mauro Matteo Cascella <mcasc...@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 <ezrak...@gmail.com>
> Signed-off-by: Mauro Matteo Cascella <mcasc...@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
