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

[Stephen Longfield]

Does anything more need to happen with this patch before it can be
applied? Not sure if it had gotten lost over the holidays.

Best,

--Stephen


On Wed, Dec 21, 2022 at 9:58 AM Stephen Longfield <slongfield@google.com> wrote:
>
> On Tue, Dec 20, 2022 at 11:30 PM Cédric Le Goater <clg@kaod.org> wrote:
> >
> > On 12/20/22 23:14, Stephen Longfield wrote:
> > > 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.
> > >
> > > Signed-off-by: Stephen Longfield <slongfield@google.com>
> > > Reviewed-by: Hao Wu <wuhaotsh@google.com>
> >
> >
> > Reviewed-by: Cédric Le Goater <clg@kaod.org>
> >
> > I think imx_fec.c is impacted also.
> >
> > Thanks,
> >
> > C.
> >
>
> Thanks for pointing that out, looks to be exactly the same. I'll send
> out a separate patch that fixes the issue in that file.
>
> Best,
>
> --Stephen
>
> >
> > > ---
> > >   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.314.g84b9a713c41-goog
> > >
> >