From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <thomas@monjalon.net>
Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com
 [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 53BEC2D13;
 Mon, 14 Jan 2019 23:26:11 +0100 (CET)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
 by mailout.nyi.internal (Postfix) with ESMTP id D72B828DA5;
 Mon, 14 Jan 2019 17:26:10 -0500 (EST)
Received: from mailfrontend1 ([10.202.2.162])
 by compute1.internal (MEProxy); Mon, 14 Jan 2019 17:26:10 -0500
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
 from:to:cc:subject:date:message-id:in-reply-to:references
 :mime-version:content-transfer-encoding:content-type; s=mesmtp;
 bh=P07T47aeBP6dgBGs8vKDagqzS59+90LxmcAuoN6ApWo=; b=LWSCkUwgXET/
 tbWZJCQX8JbtaCdh1quEyNC9C1G4ixyC8uCEzQ4fzV/4CMHVhIryK7FLqMqR2krm
 sKS1ireOFly1HCKhwmDa+1U8V/ISSmTCP+I4iHFDJ1uAE0DpCoesws/qvfQJa3AQ
 A6115tK8EvNawxz5YeIua5O2IGn9qm8=
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
 messagingengine.com; h=cc:content-transfer-encoding:content-type
 :date:from:in-reply-to:message-id:mime-version:references
 :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender
 :x-sasl-enc; s=fm1; bh=P07T47aeBP6dgBGs8vKDagqzS59+90LxmcAuoN6Ap
 Wo=; b=FHIjk7jw5wDlVwOjQRe37y+Y6Jda4qSHSDivg91zlt4rUHKfZZhlErtEV
 6ZUjbuWr5YqrSk0Jk2473Csx0jPNYNkvPlPtso9MM54PPgoaaCI2h2FhUs9brDH1
 ZF/F5UTHnDv9rHolHJgMJT7pIOW46wVHz9pMZKntal6aI74yvVcx9CZCkYBvD8i/
 N2/UjeJQGQvqvRAgdNGshYdBIOjzlfUV5wpzA4PQesG9EiF3QGFMtzGcHU4SucIg
 USojbJ9xLiXqml2GDhnnNhPcQss7fuBIFIt5ajBFfnzmZeEt1bes0/S99K7w10Nm
 MgIDeOrekz0eR4qkQ798Ke+ZU6Akw==
X-ME-Sender: <xms:gQw9XKre2MJ4lDEEEt2BNtY2AQtu6caACQhBY3gj86eKzUs-vFO_tQ>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedtledrgedugdduheekucetufdoteggodetrfdotf
 fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfquhhtnecuuegrihhlohhuthemucef
 tddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfjughrpefhvffufffkjg
 hfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghsucfoohhnjhgrlhhonhcu
 oehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucfkphepjeejrddufeegrddvtd
 efrddukeegnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghl
 ohhnrdhnvghtnecuvehluhhsthgvrhfuihiivgeptd
X-ME-Proxy: <xmx:gQw9XM5_QCC6prvbK5C0mBbkW_Lxt5hPeSPhCfpdfuT0yHHToZ2vdw>
 <xmx:gQw9XKNr3OW9NaKp9tHK4rPC2KvvzT8UpNBtQKjrdZEB_1sdWEfBVg>
 <xmx:gQw9XONP0Ypxf4TGdokb1xuEc1LY0WpQEPAFgkVv1UFE68pEYb57mA>
 <xmx:ggw9XL5yrfAuf_DCa2ZJmT9f0bTzxG-t1UuOWoGq8vwxqmS8knAvtA>
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
 by mail.messagingengine.com (Postfix) with ESMTPA id 7E242E4043;
 Mon, 14 Jan 2019 17:26:08 -0500 (EST)
From: Thomas Monjalon <thomas@monjalon.net>
To: dev@dpdk.org
Cc: stable@dpdk.org, Jiayu Hu <jiayu.hu@intel.com>,
 konstantin.ananyev@intel.com,
 Morten =?ISO-8859-1?Q?Br=F8rup?= <mb@smartsharesystems.com>,
 stephen@networkplumber.org
Date: Mon, 14 Jan 2019 23:26:07 +0100
Message-ID: <1679955.9WYREdNOYL@xps>
In-Reply-To: <1547132768-2384-1-git-send-email-jiayu.hu@intel.com>
References: <1546927725-68831-1-git-send-email-jiayu.hu@intel.com>
 <1547132768-2384-1-git-send-email-jiayu.hu@intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [dpdk-stable] [PATCH v2] gro: add missing invalid packet checks
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
 <mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
 <mailto:stable-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 14 Jan 2019 22:26:11 -0000

Any review, please?

10/01/2019 16:06, Jiayu Hu:
> Currently, GRO library doesn't check if input packets have
> invalid headers. The packets with invalid headers will also
> be processed by GRO.
> 
> However, GRO shouldn't process invalid packets. This patch adds
> missing invalid packet checks.
> 
> Fixes: 0d2cbe59b719 ("lib/gro: support TCP/IPv4")
> Fixes: 9e0b9d2ec0f4 ("gro: support VxLAN GRO")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> ---
> changes in v2:
> - fix VxLAN header length check bug for VxLAN GRO;
> - fix ethernet header length check bug;
> - use sizeof() and macro to present valid header length;
> - add VLAN related comments since GRO cannot process VLAN tagged packets.
> 
>  lib/librte_gro/gro_tcp4.c       | 12 ++++++++++++
>  lib/librte_gro/gro_tcp4.h       | 10 ++++++++++
>  lib/librte_gro/gro_vxlan_tcp4.c | 15 +++++++++++++++
>  3 files changed, 37 insertions(+)
> 
> diff --git a/lib/librte_gro/gro_tcp4.c b/lib/librte_gro/gro_tcp4.c
> index 2fe9aab..48076e0 100644
> --- a/lib/librte_gro/gro_tcp4.c
> +++ b/lib/librte_gro/gro_tcp4.c
> @@ -208,6 +208,18 @@ gro_tcp4_reassemble(struct rte_mbuf *pkt,
>  	int cmp;
>  	uint8_t find;
>  
> +	/*
> +	 * Don't process the packet whose Ethernet, IPv4 and TCP header
> +	 * lengths are invalid.
> +	 *
> +	 * In addition, GRO doesn't process the packet that is VLAN
> +	 * tagged or whose the IPv4 header contains Options.
> +	 */
> +	if (unlikely(ILLEGAL_ETHER_HDRLEN(pkt->l2_len) ||
> +			ILLEGAL_IPV4_HDRLEN(pkt->l3_len) ||
> +			ILLEGAL_TCP_HDRLEN(pkt->l4_len)))
> +		return -1;
> +
>  	eth_hdr = rte_pktmbuf_mtod(pkt, struct ether_hdr *);
>  	ipv4_hdr = (struct ipv4_hdr *)((char *)eth_hdr + pkt->l2_len);
>  	tcp_hdr = (struct tcp_hdr *)((char *)ipv4_hdr + pkt->l3_len);
> diff --git a/lib/librte_gro/gro_tcp4.h b/lib/librte_gro/gro_tcp4.h
> index 6bb30cd..65bcae8 100644
> --- a/lib/librte_gro/gro_tcp4.h
> +++ b/lib/librte_gro/gro_tcp4.h
> @@ -17,6 +17,16 @@
>   */
>  #define MAX_IPV4_PKT_LENGTH UINT16_MAX
>  
> +/* The maximum TCP header length */
> +#define TCP_MAX_HLEN 60
> +
> +#define ILLEGAL_ETHER_HDRLEN(len) ((len) != ETHER_HDR_LEN)
> +#define ILLEGAL_ETHER_VXLAN_HDRLEN(len) \
> +	((len) != (ETHER_VXLAN_HLEN + ETHER_HDR_LEN))
> +#define ILLEGAL_IPV4_HDRLEN(len) ((len) != sizeof(struct ipv4_hdr))
> +#define ILLEGAL_TCP_HDRLEN(len) \
> +	(((len) < sizeof(struct tcp_hdr)) || ((len) > TCP_MAX_HLEN))
> +
>  /* Header fields representing a TCP/IPv4 flow */
>  struct tcp4_flow_key {
>  	struct ether_addr eth_saddr;
> diff --git a/lib/librte_gro/gro_vxlan_tcp4.c b/lib/librte_gro/gro_vxlan_tcp4.c
> index 955ae4b..72d63bc 100644
> --- a/lib/librte_gro/gro_vxlan_tcp4.c
> +++ b/lib/librte_gro/gro_vxlan_tcp4.c
> @@ -306,6 +306,21 @@ gro_vxlan_tcp4_reassemble(struct rte_mbuf *pkt,
>  	uint16_t hdr_len;
>  	uint8_t find;
>  
> +	/*
> +	 * Don't process the packet whose outer Ethernet, outer IPv4,
> +	 * VxLAN header, inner Ethernet, inner IPv4 and inner TCP
> +	 * header lengths are invalid.
> +	 *
> +	 * In addition, GRO doesn't process the packet that is VLAN
> +	 * tagged or whose IPv4 header contains Options.
> +	 */
> +	if (unlikely(ILLEGAL_ETHER_HDRLEN(pkt->outer_l2_len) ||
> +				ILLEGAL_IPV4_HDRLEN(pkt->outer_l3_len) ||
> +				ILLEGAL_ETHER_VXLAN_HDRLEN(pkt->l2_len) ||
> +				ILLEGAL_IPV4_HDRLEN(pkt->l3_len) ||
> +				ILLEGAL_TCP_HDRLEN(pkt->l4_len)))
> +		return -1;
> +
>  	outer_eth_hdr = rte_pktmbuf_mtod(pkt, struct ether_hdr *);
>  	outer_ipv4_hdr = (struct ipv4_hdr *)((char *)outer_eth_hdr +
>  			pkt->outer_l2_len);
>