Re: [PATCH 03/25] hw/net: Fix read of uninitialized memory in ftgmac100

[Joel Stanley]

On Thu, 19 Jan 2023 at 12:39, Cédric Le Goater <clg@kaod.org> wrote:
>
> From: Stephen Longfield <slongfield@google.com>
>
> With the `size += 4` before the call to `crc32`, the CRC calculation
> would overrun the buffer. Size is used in the while loop starting on
> line 1009 to determine how much data to write back, with the last
> four bytes coming from `crc_ptr`, so do need to increase it, but should
> do this after the computation.
>
> I'm unsure why this use of uninitialized memory in the CRC doesn't
> result in CRC errors, but it seems clear to me that it should not be
> included in the calculation.

Does this affect the error counters observed under Linux?

>
> Signed-off-by: Stephen Longfield <slongfield@google.com>
> Reviewed-by: Hao Wu <wuhaotsh@google.com>
> Message-Id: <20221220221437.3303721-1-slongfield@google.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Joel Stanley <joel@jms.id.au>

> ---
>  hw/net/ftgmac100.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
> index 83ef0a783e..d3bf14be53 100644
> --- a/hw/net/ftgmac100.c
> +++ b/hw/net/ftgmac100.c
> @@ -980,9 +980,9 @@ static ssize_t ftgmac100_receive(NetClientState *nc, 
> const uint8_t *buf,
>          return size;
>      }
>
> -    /* 4 bytes for the CRC.  */
> -    size += 4;
>      crc = cpu_to_be32(crc32(~0, buf, size));
> +    /* Increase size by 4, loop below reads the last 4 bytes from crc_ptr. */
> +    size += 4;
>      crc_ptr = (uint8_t *) &crc;
>
>      /* Huge frames are truncated.  */
> --
> 2.39.0
>
>