From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [PATCH] gro: add missing invalid packet checks
Date: Mon, 7 Jan 2019 22:31:51 -0800
Message-ID: <20190107223151.18b185b7@hermes.lan>
References: <1546567036-29444-1-git-send-email-jiayu.hu@intel.com>
 <1546927725-68831-1-git-send-email-jiayu.hu@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: dev@dpdk.org, tiwei.bie@intel.com, bruce.richardson@intel.com,
 stable@dpdk.org
To: Jiayu Hu <jiayu.hu@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-pg1-f196.google.com (mail-pg1-f196.google.com
 [209.85.215.196]) by dpdk.org (Postfix) with ESMTP id 5B20D1B43C
 for <dev@dpdk.org>; Tue,  8 Jan 2019 07:32:02 +0100 (CET)
Received: by mail-pg1-f196.google.com with SMTP id z11so1284443pgu.0
 for <dev@dpdk.org>; Mon, 07 Jan 2019 22:32:02 -0800 (PST)
In-Reply-To: <1546927725-68831-1-git-send-email-jiayu.hu@intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Tue,  8 Jan 2019 14:08:45 +0800
Jiayu Hu <jiayu.hu@intel.com> wrote:

> +	/*
> +	 * Don't process the packet whose Ethernet, IPv4 and TCP header
> +	 * lengths are invalid. In addition, if the IPv4 header contains
> +	 * Options, the packet shouldn't be processed.
> +	 */
> +	if (unlikely(ILLEGAL_ETHER_HDRLEN(pkt->l2_len) ||
> +			ILLEGAL_IPV4_HDRLEN(pkt->l3_len) ||
> +			ILLEGAL_TCP_HDRLEN(pkt->l4_len)))
> +		return -1;

I like it when code is as picky as possible when doing optimizations because
it reduces possible security riskg.

To me this looks more confusing and not as careful as doing it like:

	if (unlikely(pkt->l2_len != ETHER_HDR_LEN))
		return -1;
	eth_hdr = rte_pktmbuf_mtod(pkt, struct ether_hdr *);
	ipv4_hdr = (struct ipv4_hdr *)((char *)eth_hdr + ETHER_HDR_LEN);

	if (pkt->l3_len != (ipv4->version_ihl & IPV4_HDR_IHL_MASK) << 4)
		return -1;

	if (pkt->l4_len < sizeof(struct tcp_hdr))
		return -1;

You should also check for TCP options as well.

And IPv6 has same issues.