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

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

  Hello Markus,

+-- On Thu, 18 Feb 2016, Markus Armbruster wrote --+
| 
|           if ((data[14] & 0xf0) != 0x40)
| Buffer overrun when length <= 14.
| 
|           proto = data[23];
| Buffer overrun when length <= 23.
| 
| I think we should check that we got at least an IPv4 header without
| options (length >= 14 + 20) before accessing said header.

  Right. Some callers do have checks in place to ensure length is >= minium 
packet length. Still it'll help to add an assert(length >= 14+20);

|     /*
|      * Compute and store checksum of IPv4 TCP or UDP packet.
|      * @data holds @length bytes.  It must be a complete packet.
|      * If this is an IPv4 TCP packet, compute its checksum and store it
|      * in the TCP header.
|      * Else if this is a complete IPv4 UDP packet, compute its checksum
|      * and store it in the UDP header.
|      * Else do nothing.
|      */
|     void net_checksum_calculate(uint8_t *data, int length)
| 
| I find it simpler.
| 
| If the other buffer overrun I mentioned above needs fixing: yes.
| Else: depends on the maintainer.

  Okay, I'll wait for Jason's inputs and will send a revised patch including 
your suggestions above.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F