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 <slongfi...@google.com> wrote: > > 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 > > > > >
