From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH 0/2] prevent out of bounds read with checksum Date: Thu, 20 Dec 2018 19:09:11 +0000 Message-ID: <06be3a8e-d281-bb79-7147-30f4f9c2eeaf@intel.com> References: <20181217155005.13457-1-bruce.richardson@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, Hemant Agrawal , Shreyansh Jain To: Bruce Richardson , Olivier Matz , Keith Wiles Return-path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 0059A1BCE3 for ; Thu, 20 Dec 2018 20:09:14 +0100 (CET) In-Reply-To: <20181217155005.13457-1-bruce.richardson@intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 12/17/2018 3:50 PM, Bruce Richardson wrote: > The functions for checksumming the packet payload don't perform bounds > checks, and are used by the TAP driver which does not do any bounds checks > on the incoming packet either. This means a packet received with an > incorrect IP header can read beyond the end of the mbuf. > > In the worst case, where the length is specified as being smaller than the > IPv4 header, 32-bit wrap-around on subtraction occurs, meaning that approx > 4GB of memory will be read. > > To fix this, we can introduce a sanity check into the ipv4 function to > ensure that underflow does not occur. Since the checksum function does not > take the mbuf length as a parameter, we cannot check for overflow there, > so we instead perform the checks in the TAP driver directly. > > Ideally, in a future release, all checksum functions should be modified to > take a max buffer length parameter to fix this issue globally. > > NOTE: It appears that the dpaa driver also uses these functions, but from > what I can see there, they are only used on TX, which means that there > should be less need for parameter length checking, as the data does not > come from an untrusted source. Perhaps maintainers, Hemant and Shreyansh, > can confirm? > > CC: Hemant Agrawal > CC: Shreyansh Jain > > Bruce Richardson (2): > net: fix underflow for checksum of invalid IPv4 packets > net/tap: add buffer overflow checks before checksum Series applied to dpdk-next-net/master, thanks.