On Mon, 2 Nov 2026 at 13:36, Peter Maydell <[email protected]> wrote:
>
> From: Sergey Kambalin <[email protected]>
>
> Implement the receive DMA ring handling: qemu_can_receive()/
> receive() callbacks for the NIC's netdev peer, copying incoming
> frame data into descriptor buffers on the active RX ring,
> classifying broadcast/multicast, and raising the RXDMA_MBDONE
> interrupt as descriptors complete.
Coverity suggests that the handling of lengths in the receive
code here may not be right (CID 1690161); Marcelo, could you
have a look, please?
> + while (len < size) {
> + size_t l = size - len;
> + size_t buf_size = MIN(ring->ring_buf_size & 0xffff,
> + sizeof(s->rx_packet));
> + uint8_t *dma_buf = s->rx_packet;
> + hwaddr dma_buf_addr =
> + bcm2838_genet_make_u64(desc->address_hi, desc->address_lo);
> + MemTxResult mem_tx_result = MEMTX_OK;
> + const size_t frame_off = sizeof(BCM2838GenetXmitStatus) + 2;
Here we calculate a frame_off assuming we have the xmit status
struct and another 2 bytes as well as the payload...
> + uint8_t *frame_buf = dma_buf + frame_off;
> +
> + bool crc_fwd = FIELD_EX32(s->regs.umac.cmd, GENET_UMAC_CMD, CRC_FWD);
> + size_t buflength;
> + uint32_t prod_index;
> +
> + if (l > ring->ring_buf_size) {
> + l = ring->ring_buf_size;
> + }
> + /* Never copy more than fits in rx_packet after the status block */
> + if (l > sizeof(s->rx_packet) - frame_off) {
> + l = sizeof(s->rx_packet) - frame_off;
> + }
...and here we drop the amount of payload to allow for that...
> +
> + memcpy(frame_buf, buf + len, l);
> +
> + len += l;
> +
> + desc->length_status = FIELD_DP32(desc->length_status,
> + GENET_RDMA_LENGTH_STATUS,
> + EOP, !!(len >= size));
> +
> + buflength = l + sizeof(BCM2838GenetXmitStatus) + 2;
> + if (crc_fwd) {
> + buflength += 4;
> + }
...but here we calculate buflen with payload + struct + 2 +
also potentially an extra 4 for the CRC. Shouldn't we have
allowed for the CRC in the earlier calculations somehow too?
> +
> + desc->length_status = FIELD_DP32(desc->length_status,
> + GENET_RDMA_LENGTH_STATUS,
> + BUFLENGTH, buflength);
The thing Coverity thinks happens is that buflength might not
fit in the 12 bit descriptor field.
thanks
-- PMM
