Ping for review, please? thanks -- PMM
On Mon, 6 Jul 2026 at 17:27, Peter Maydell <[email protected]> wrote: > > The xilinx_axienet device has ethernet checksum offloading, with a > mode where the guest provides the offsets within the packet where > the data to be checksummed starts, and where the final checksum > should be written into the packet. > > We don't sanity check the TX_CSINSERT offset before writing the > checksum data into it, which means the guest can pass us a value that > is larger than the packet itself and cause us to write the checksum > off the end of the buffer. We also don't explicitly check the > TX_CSBEGIN offset; this doesn't currently cause any problems because > we will pass a negative length to net_checksum_add() which does > nothing, but it's a potential trap for the future if the type > used for the length gets changed to be unsigned. > > Explicitly check the offsets. The datasheet doesn't say what happens > if the guest misprograms this, so we choose to log an error and send > the packet as-is. > > Cc: [email protected] > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3599 > Signed-off-by: Peter Maydell <[email protected]> > --- > hw/net/xilinx_axienet.c | 29 ++++++++++++++++++----------- > 1 file changed, 18 insertions(+), 11 deletions(-) > > diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c > index 970732b162..a44dda5385 100644 > --- a/hw/net/xilinx_axienet.c > +++ b/hw/net/xilinx_axienet.c > @@ -919,20 +919,27 @@ xilinx_axienet_data_stream_push(StreamSink *obj, > uint8_t *buf, size_t size, > if (s->hdr[0] & 1) { > unsigned int start_off = s->hdr[1] >> 16; > unsigned int write_off = s->hdr[1] & 0xffff; > - uint32_t tmp_csum; > - uint16_t csum; > > - tmp_csum = net_checksum_add(s->txpos - start_off, > - buf + start_off); > - /* Accumulate the seed. */ > - tmp_csum += s->hdr[2] & 0xffff; > + if (start_off > s->txpos || write_off + 2 > s->txpos) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: offsets outside packet, skipping checksum\n", > + TYPE_XILINX_AXI_ENET); > + } else { > + uint32_t tmp_csum; > + uint16_t csum; > > - /* Fold the 32bit partial checksum. */ > - csum = net_checksum_finish(tmp_csum); > + tmp_csum = net_checksum_add(s->txpos - start_off, > + buf + start_off); > + /* Accumulate the seed. */ > + tmp_csum += s->hdr[2] & 0xffff; > > - /* Writeback. */ > - buf[write_off] = csum >> 8; > - buf[write_off + 1] = csum & 0xff; > + /* Fold the 32bit partial checksum. */ > + csum = net_checksum_finish(tmp_csum); > + > + /* Writeback. */ > + buf[write_off] = csum >> 8; > + buf[write_off + 1] = csum & 0xff; > + } > } > > qemu_send_packet(qemu_get_queue(s->nic), buf, s->txpos); > -- > 2.43.0
