Re: [Qemu-devel] [PATCH] net: check packet payload length

[P J P <ppandit@redhat.com>]

  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)) {
             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.
--
 - P J P
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F