+Laurent/Finn On 11/24/20 10:24 AM, Mauro Matteo Cascella wrote: > An integer underflow could occur during packet transmission due to 'tx_len' > not > being updated if SONIC_TFC register is set to zero. Check for negative > 'tx_len' > when removing existing FCS. > > RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1899722 > Signed-off-by: Mauro Matteo Cascella <mcasc...@redhat.com> > Reported-by: Gaoning Pan <p...@zju.edu.cn> > --- > hw/net/dp8393x.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > index 674b04b354..205c0decc5 100644 > --- a/hw/net/dp8393x.c > +++ b/hw/net/dp8393x.c > @@ -495,6 +495,10 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > } else { > /* Remove existing FCS */ > tx_len -= 4; > + if (tx_len < 0) { > + SONIC_ERROR("tx_len is %d\n", tx_len); > + break; > + } > } > > if (s->regs[SONIC_RCR] & (SONIC_RCR_LB1 | SONIC_RCR_LB0)) { >
Doesn't it make more sense to check 'tx_len >= 4' and skip tx/rx when 'tx_len == 0'? -- >8 -- @@ -488,25 +488,29 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) } } - /* Handle Ethernet checksum */ - if (!(s->regs[SONIC_TCR] & SONIC_TCR_CRCI)) { - /* Don't append FCS there, to look like slirp packets - * which don't have one */ - } else { - /* Remove existing FCS */ - tx_len -= 4; + if (tx_len >= 4) { + /* Handle Ethernet checksum */ + if (!(s->regs[SONIC_TCR] & SONIC_TCR_CRCI)) { + /* Don't append FCS there, to look like slirp packets + * which don't have one */ + } else { + /* Remove existing FCS */ + tx_len -= 4; + } } - if (s->regs[SONIC_RCR] & (SONIC_RCR_LB1 | SONIC_RCR_LB0)) { - /* Loopback */ - s->regs[SONIC_TCR] |= SONIC_TCR_CRSL; - if (nc->info->can_receive(nc)) { - s->loopback_packet = 1; - nc->info->receive(nc, s->tx_buffer, tx_len); + if (tx_len > 0) { + if (s->regs[SONIC_RCR] & (SONIC_RCR_LB1 | SONIC_RCR_LB0)) { + /* Loopback */ + s->regs[SONIC_TCR] |= SONIC_TCR_CRSL; + if (nc->info->can_receive(nc)) { + s->loopback_packet = 1; + nc->info->receive(nc, s->tx_buffer, tx_len); + } + } else { + /* Transmit packet */ + qemu_send_packet(nc, s->tx_buffer, tx_len); } - } else { - /* Transmit packet */ - qemu_send_packet(nc, s->tx_buffer, tx_len); } s->regs[SONIC_TCR] |= SONIC_TCR_PTX; ---