On 02/23/2016 07:11 PM, P J P wrote:
> Hello Jason,
>
> +-- On Tue, 23 Feb 2016, Jason Wang wrote --+
> | Let's avoid adding assert() here since it could be triggered by guest.
>
> Okay.
>
> | I think you need audit all the callers to see if the issue mentioned by
> | Markus existed first.
>
> Yes, I did. As mentioned earlier, some have check for minimum packet size,
> others check for length > 0.
>
> net_checksum_calculate is called as:
>
> - hw/net/cadence_gem.c
> if (tx_desc_get_length(desc) == 0))
> DB_PRINT("Invalid TX descriptor @ 0x%x\n",
> }
> total_bytes += tx_desc_get_length(desc);
> net_checksum_calculate(tx_packet, total_bytes);
>
> - hw/net/fsl_etsec/rings.c
> if (etsec->tx_buffer_len != 0
> net_checksum_calculate(etsec->tx_buffer + 8,
> etsec->tx_buffer_len - 8);
>
> - hw/net/virtio-net.c
> if (size > 27 && size < 1500) &&
> (buf[34] == 0 && buf[35] == 67)) {
Are we sure size of buf is >= 35 here?
> net_checksum_calculate(buf, size);
>
> - hw/net/xen_nic.c
> if (txreq.size < 14) {
> xen_be_printf(&netdev->xendev, 0, "bad packet size:
> }
> net_checksum_calculate(tmpbuf, txreq.size);
>
> It might be possible to send a packet with (length < 14+20), and cause an OOB
> read in net_checksum_calculate,
>
> if ((data[14] & 0xf0) != 0x40)
> return; /* not IPv4 */
> hlen = (data[14] & 0x0f) * 4;
> plen = (data[16] << 8 | data[17]) - hlen;
> proto = data[23];
>
> I'll send a revised patch with a check for minimum 'data' length to include
> complete layer-2(14) + layer-3(20) headers.
>
> + if (len < 14+20)
> + return;
>
>
> Thank you.
Ok.
> --
> - P J P
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
>
