HI Bruce, 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. Regards, Hemant > > CC: Hemant Agrawal <hemant.agra...@nxp.com> > CC: Shreyansh Jain <shreyansh.j...@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(-) >
