From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3FA47A0487 for ; Tue, 2 Jul 2019 11:06:57 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7F6AF4C8F; Tue, 2 Jul 2019 11:06:56 +0200 (CEST) Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 9BE842BA5 for ; Tue, 2 Jul 2019 11:06:55 +0200 (CEST) Received: from lfbn-lil-1-176-160.w90-45.abo.wanadoo.fr ([90.45.26.160] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hiEnU-00021H-Gm; Tue, 02 Jul 2019 11:09:58 +0200 Received: by droids-corp.org (sSMTP sendmail emulation); Tue, 02 Jul 2019 11:06:50 +0200 Date: Tue, 2 Jul 2019 11:06:50 +0200 From: Olivier Matz To: Marcin Smoczynski Cc: marko.kovacevic@intel.com, orika@mellanox.com, bruce.richardson@intel.com, pablo.de.lara.guarch@intel.com, radu.nicolau@intel.com, akhil.goyal@nxp.com, tomasz.kantecki@intel.com, konstantin.ananyev@intel.com, bernard.iremonger@intel.com, dev@dpdk.org Message-ID: <20190702090650.kumw2o22woe52vbk@platinum> References: <20190508104717.13448-1-marcinx.smoczynski@intel.com> <20190624134000.2456-1-marcinx.smoczynski@intel.com> <20190624134000.2456-2-marcinx.smoczynski@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190624134000.2456-2-marcinx.smoczynski@intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v2 1/4] net: new ipv6 header extension parsing function X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, On Mon, Jun 24, 2019 at 03:39:57PM +0200, Marcin Smoczynski wrote: > Introduce new function for IPv6 header extension parsing able to > determine extension length and next protocol number. > > This function is helpful when implementing IPv6 header traversing. > > Signed-off-by: Marcin Smoczynski > --- > lib/librte_net/rte_ip.h | 49 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h > index ae3b7e730..c2c67b85d 100644 > --- a/lib/librte_net/rte_ip.h > +++ b/lib/librte_net/rte_ip.h > @@ -428,6 +428,55 @@ rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr *ipv6_hdr, const void *l4_hdr) > return (uint16_t)cksum; > } > > +/* IPv6 fragmentation header size */ > +#define RTE_IPV6_FRAG_HDR_SIZE 8 > + > +/** > + * Parse next IPv6 header extension > + * > + * This function checks if proto number is an IPv6 extensions and parses its > + * data if so, providing information on next header and extension length. > + * > + * @param p > + * Pointer to an extension raw data. > + * @param proto > + * Protocol number extracted from the "next header" field from > + * the IPv6 header or the previous extension. > + * @param ext_len > + * Extension data length. > + * @return > + * next protocol number if proto is an IPv6 extension, -EINVAL otherwise > + */ > +static inline int __rte_experimental > +rte_ipv6_get_next_ext(uint8_t *p, int proto, size_t *ext_len) > +{ > + int next_proto; > + > + switch (proto) { > + case IPPROTO_AH: > + next_proto = *p++; > + *ext_len = (*p + 2) * sizeof(uint32_t); > + break; > + > + case IPPROTO_HOPOPTS: > + case IPPROTO_ROUTING: > + case IPPROTO_DSTOPTS: > + next_proto = *p++; > + *ext_len = (*p + 1) * sizeof(uint64_t); > + break; > + > + case IPPROTO_FRAGMENT: > + next_proto = *p; > + *ext_len = RTE_IPV6_FRAG_HDR_SIZE; > + break; > + > + default: > + return -EINVAL; > + } > + > + return next_proto; > +} > + > #ifdef __cplusplus > } > #endif I just want to highlight the potential danger of this kind of function: then length is read from the packet (controlled by the peer), and returned without check. My initial fear was that an attacker forge an IPv6 packet, resulting in *ext_len being 0. For instance, calling the function with (proto = IPPROTO_ROUTING, buffer = [ 0, 255, ... ]). Fortunatly, this does not happen, due to the implicit promotion of *p to a larger integer in the (*p + 1) expression. That said, what about adding some comments in the description, saying that: - the pointer p must point to a buffer whose length is at least 2 - the returned length must be checked by the caller to ensure it does not overflow the packet buffer You can add my ack in the next version. Thanks, Olivier