[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] net: check packet payload length
From: |
Jason Wang |
Subject: |
Re: [Qemu-devel] [PATCH] net: check packet payload length |
Date: |
Tue, 23 Feb 2016 15:57:25 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 |
On 02/18/2016 06:33 PM, P J P wrote:
> 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);
Let's avoid adding assert() here since it could be triggered by guest.
>
> | /*
> | * 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
I think you need audit all the callers to see if the issue mentioned by
Markus existed first.
- [Qemu-devel] [PATCH] net: check packet payload length, P J P, 2016/02/17
- Re: [Qemu-devel] [PATCH] net: check packet payload length, Markus Armbruster, 2016/02/17
- Re: [Qemu-devel] [PATCH] net: check packet payload length, P J P, 2016/02/17
- Re: [Qemu-devel] [PATCH] net: check packet payload length, P J P, 2016/02/18
- Re: [Qemu-devel] [PATCH] net: check packet payload length, Markus Armbruster, 2016/02/18
- Re: [Qemu-devel] [PATCH] net: check packet payload length, P J P, 2016/02/18
- Re: [Qemu-devel] [PATCH] net: check packet payload length,
Jason Wang <=
- Re: [Qemu-devel] [PATCH] net: check packet payload length, P J P, 2016/02/23
- Re: [Qemu-devel] [PATCH] net: check packet payload length, Jason Wang, 2016/02/23
- Re: [Qemu-devel] [PATCH] net: check packet payload length, P J P, 2016/02/23