On Tue, Dec 20, 2022 at 11:30 PM Cédric Le Goater <c...@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 <slongfi...@google.com> > > Reviewed-by: Hao Wu <wuhao...@google.com> > > > Reviewed-by: Cédric Le Goater <c...@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 > > >
