[dpdk-dev] [PATCH 0/2] prevent out of bounds read with checksum
hemant.agrawal at nxp.com
Tue Dec 18 13:50:20 CET 2018
On 17-Dec-18 9:20 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?
In DPAA, we are using software based checksum calculation for self
generated packets largely.
They are mostly trust worthy unless someone is deliberately or
mistakenly trying to send a corrupt packet.
We will check, if we can also add some checks in DPAA driver in these
legs without making performance impact for self generated packets.
> CC: Hemant Agrawal <hemant.agrawal at nxp.com>
> CC: Shreyansh Jain <shreyansh.jain at nxp.com>
> Bruce Richardson (2):
> net: fix underflow for checksum of invalid IPv4 packets
> net/tap: add buffer overflow checks before checksum
> drivers/net/tap/rte_eth_tap.c | 14 ++++++++++++++
> lib/librte_net/rte_ip.h | 12 ++++++++----
> 2 files changed, 22 insertions(+), 4 deletions(-)
More information about the dev