From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56723) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWLtc-0002gm-NW for qemu-devel@nongnu.org; Thu, 18 Feb 2016 05:33:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aWLtZ-0007zS-DC for qemu-devel@nongnu.org; Thu, 18 Feb 2016 05:33:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33479) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWLtZ-0007zM-8x for qemu-devel@nongnu.org; Thu, 18 Feb 2016 05:33:13 -0500 Date: Thu, 18 Feb 2016 16:03:04 +0530 (IST) From: P J P In-Reply-To: <87povujrf8.fsf@blackfin.pond.sub.org> Message-ID: References: <1455710904-13368-1-git-send-email-ppandit@redhat.com> <878u2jh4uz.fsf@blackfin.pond.sub.org> <87povujrf8.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Subject: Re: [Qemu-devel] [PATCH] net: check packet payload length List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Jason Wang , Qemu Developers , Liu Ling 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