From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 67D1F1B501; Tue, 8 Jan 2019 02:22:22 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Jan 2019 17:22:21 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,452,1539673200"; d="scan'208";a="124090471" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by FMSMGA003.fm.intel.com with ESMTP; 07 Jan 2019 17:22:21 -0800 Received: from fmsmsx153.amr.corp.intel.com (10.18.125.6) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 7 Jan 2019 17:22:20 -0800 Received: from shsmsx154.ccr.corp.intel.com (10.239.6.54) by FMSMSX153.amr.corp.intel.com (10.18.125.6) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 7 Jan 2019 17:22:20 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.63]) by SHSMSX154.ccr.corp.intel.com ([169.254.7.46]) with mapi id 14.03.0415.000; Tue, 8 Jan 2019 09:22:18 +0800 From: "Hu, Jiayu" To: "Richardson, Bruce" CC: "dev@dpdk.org" , "Bie, Tiwei" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] gro: fix overflow of TCP Options length calculation Thread-Index: AQHUo9Dc3uq1Ni9CtkWjJUImazDgLqWjXSyAgAEsuqA= Date: Tue, 8 Jan 2019 01:22:18 +0000 Message-ID: References: <1546567036-29444-1-git-send-email-jiayu.hu@intel.com> <20190107142955.GC14912@bricha3-MOBL.ger.corp.intel.com> In-Reply-To: <20190107142955.GC14912@bricha3-MOBL.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTgzYTUxZTEtYWQwNS00NzJjLWE4NjktNmVjY2ZiMTUzNDkyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUDBldEVESjVyOEQ0MkJyUXdFWlJKdXZQUWROR2w4dGZoamxOZzlDUFwvSVB2NjA1WlhGWmlVOEVrXC9XTUFERnJTIn0= x-ctpclassification: CTP_NT x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] gro: fix overflow of TCP Options length calculation 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: , X-List-Received-Date: Tue, 08 Jan 2019 01:22:23 -0000 > -----Original Message----- > From: Richardson, Bruce > Sent: Monday, January 7, 2019 10:30 PM > To: Hu, Jiayu > Cc: dev@dpdk.org; Bie, Tiwei ; stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] gro: fix overflow of TCP Options length > calculation >=20 > On Fri, Jan 04, 2019 at 09:57:16AM +0800, Jiayu Hu wrote: > > If we receive a packet with an invalid TCP header, whose > > TCP header length is less than 20 bytes (the minimal TCP > > header length), the calculated TCP Options length will > > overflow and result in incorrect reassembly behaviors. >=20 > Please explain how changing the "len" type fixes this behaviour. Originally, 'uint16_t len =3D RTE_MAX(tcp_hl, tcp_hl_orig) - sizeof(struct = tcp_hdr)'. When the TCP header length of an input packet is less than 20, which is the= value of sizeof(struct tcp_hdr), the value of len will overflow. For example, if TCP= header lengths of input packets are 14, the value of 'len' will be 65529 (65535-6). After = then, we will compare TCP options via memcmp(tcp_hdr+1,..., len), which would cause segme= nt fault. Changing the type of 'len' can just avoid segment fault. After the modifica= tion, when applications input TCP packets which have illegal TCP headers, GRO jus= t inserts them into the table; it will not access TCP options and merge them. When us= ers flush the table or rte_gro_reassemble_burst() completes, all those invalid packet= s will be evicted from the table. In fact, if add minimal header lengths check in gro_vxlan_tcp4_reassemble()= and gro_tcp4_reassemble(), those packets with invalid headers will directly ret= urn to applications; they will not go to check_seq_option() or insert into the tab= le. Maybe it's a better way for GRO to solve the issue from invalid packets. I will s= end a new patch to solve the issue. Thanks, Jiayu >=20 > > > > Fixes: 0d2cbe59b719 ("lib/gro: support TCP/IPv4") > > Fixes: 9e0b9d2ec0f4 ("gro: support VxLAN GRO") > > Cc: stable@dpdk.org > > > > Signed-off-by: Jiayu Hu > > --- > > lib/librte_gro/gro_tcp4.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/lib/librte_gro/gro_tcp4.h b/lib/librte_gro/gro_tcp4.h > > index 6bb30cd..189cea3 100644 > > --- a/lib/librte_gro/gro_tcp4.h > > +++ b/lib/librte_gro/gro_tcp4.h > > @@ -266,7 +266,8 @@ check_seq_option(struct gro_tcp4_item *item, > > struct rte_mbuf *pkt_orig =3D item->firstseg; > > struct ipv4_hdr *iph_orig; > > struct tcp_hdr *tcph_orig; > > - uint16_t len, tcp_hl_orig; > > + uint16_t tcp_hl_orig; > > + int32_t len; > > > > iph_orig =3D (struct ipv4_hdr *)(rte_pktmbuf_mtod(pkt_orig, char *) + > > l2_offset + pkt_orig->l2_len); > > -- > > 2.7.4 > >