From patchwork Mon Dec 9 06:27:43 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "fan.du" X-Patchwork-Id: 298938 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id F30262C00A6 for ; Mon, 9 Dec 2013 17:28:00 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932352Ab3LIG15 (ORCPT ); Mon, 9 Dec 2013 01:27:57 -0500 Received: from mail1.windriver.com ([147.11.146.13]:64247 "EHLO mail1.windriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932338Ab3LIG14 (ORCPT ); Mon, 9 Dec 2013 01:27:56 -0500 Received: from ALA-HCA.corp.ad.wrs.com (ala-hca.corp.ad.wrs.com [147.11.189.40]) by mail1.windriver.com (8.14.5/8.14.5) with ESMTP id rB96Rmrf029912 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=FAIL); Sun, 8 Dec 2013 22:27:48 -0800 (PST) Received: from [128.224.162.161] (128.224.162.161) by ALA-HCA.corp.ad.wrs.com (147.11.189.50) with Microsoft SMTP Server (TLS) id 14.2.347.0; Sun, 8 Dec 2013 22:27:48 -0800 Message-ID: <52A562DF.4090302@windriver.com> Date: Mon, 9 Dec 2013 14:27:43 +0800 From: Fan Du User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101208 Thunderbird/3.1.7 MIME-Version: 1.0 To: Steffen Klassert CC: , Subject: Re: [PATCH net-next 2/3] xfrm: clamp down spi range for IPComp when allocating spi References: <1385607161-27597-1-git-send-email-fan.du@windriver.com> <1385607161-27597-3-git-send-email-fan.du@windriver.com> <20131206114248.GG31491@secunet.com> In-Reply-To: <20131206114248.GG31491@secunet.com> X-Originating-IP: [128.224.162.161] Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 2013年12月06日 19:42, Steffen Klassert wrote: > On Thu, Nov 28, 2013 at 10:52:40AM +0800, Fan Du wrote: >> otherwise xfrm state can not be found properly by peers. >> >> Signed-off-by: Fan Du >> --- >> net/xfrm/xfrm_state.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c >> index 68c2f35..a6716d7 100644 >> --- a/net/xfrm/xfrm_state.c >> +++ b/net/xfrm/xfrm_state.c >> @@ -1506,6 +1506,19 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high) >> __be32 maxspi = htonl(high); >> u32 mark = x->mark.v& x->mark.m; >> >> + /* Compression Parameter Index(CPI) is 16bits wide >> + * An 32 bits spi value will hash xfrm_state into wrong hash slot. >> + * When the upper 16bits of spi values is used as CPI for the peer >> + * to look up xfrm state, it would generate XfrmOutNoStates error, >> + * as apparently we are looking for the wrong hash slot. >> + * >> + * So clamp down the spi range into only 16bits valid wide. >> + */ >> + if (x->id.proto == IPPROTO_COMP) { >> + minspi = htonl(0xc00); >> + maxspi = htonl(0xff00); >> + } > > This does not make sense. The spi is chosen based on minspi only > if minspi is equal to maxspi. This will be never the case for > IPPROTO_COMP with your patch. > > Also, the spi range is user defined, we should respect the > users configuration if the range is valid. Ok, then, speaking of respect user defined range, how about below informal patch which only check the validity of the range? My original thoughts is CPI is only 16bits wide, kernel itself can keep the CPI's validity. btw, v2 will also fix patch1/3 align issue. diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index 6a9c402..2c6fb99 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -1507,6 +1507,9 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high) err = -ENOENT; + if ((x->id.proto == IPPROTO_COMP) && (high > 0xFFFF)) + goto unlock; + if (minspi == maxspi) { x0 = xfrm_state_lookup(net, mark, &x->id.daddr, minspi, x->id.proto, x->props.family); if (x0) {