From patchwork Thu Oct 26 14:45:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Westphal X-Patchwork-Id: 13437642 X-Patchwork-Delegate: kuba@kernel.org Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E51D61D52B for ; Thu, 26 Oct 2023 14:51:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=none Received: from Chamillionaire.breakpoint.cc (Chamillionaire.breakpoint.cc [IPv6:2a0a:51c0:0:237:300::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E94A31B2 for ; Thu, 26 Oct 2023 07:51:44 -0700 (PDT) Received: from fw by Chamillionaire.breakpoint.cc with local (Exim 4.92) (envelope-from ) id 1qw1i3-0007WM-3B; Thu, 26 Oct 2023 16:51:43 +0200 From: Florian Westphal To: Cc: steffen.klassert@secunet.com, herbert@gondor.apana.org.au, Florian Westphal , Antony Antony Subject: [PATCH ipsec-next v2] xfrm: policy: fix layer 4 flowi decoding Date: Thu, 26 Oct 2023 16:45:42 +0200 Message-ID: <20231026144610.26347-1-fw@strlen.de> X-Mailer: git-send-email 2.41.0 In-Reply-To: References: Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org The commit shipped with two bugs: fl4->fl4_icmp_type = flkeys->icmp.type; fl4->fl4_icmp_type = flkeys->icmp.code; ~~~~ should have been "code". But the more severe bug is that I got fooled by flowi member defines: fl4_icmp_type, fl4_gre_key and fl4_dport share the same union/address. Fix typo and make gre/icmp key setting depend on the l4 protocol. Fixes: 7a0207094f1b ("xfrm: policy: replace session decode with flow dissector") Reported-and-tested-by: Antony Antony Signed-off-by: Florian Westphal --- v2: decode_session6 must use IPPROTO_ICMPV6, not IPPROTO_ICMP. I normally do not resend immediately but in this case it seems like the lesser evil. Alternative is to defer this until after -next merges have completed and then handle it via ipsec.git. net/xfrm/xfrm_policy.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 6aea8b2f45e0..d2dddc570f4f 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -3400,11 +3400,18 @@ decode_session4(const struct xfrm_flow_keys *flkeys, struct flowi *fl, bool reve fl4->fl4_dport = flkeys->ports.dst; } + switch (flkeys->basic.ip_proto) { + case IPPROTO_GRE: + fl4->fl4_gre_key = flkeys->gre.keyid; + break; + case IPPROTO_ICMP: + fl4->fl4_icmp_type = flkeys->icmp.type; + fl4->fl4_icmp_code = flkeys->icmp.code; + break; + } + fl4->flowi4_proto = flkeys->basic.ip_proto; fl4->flowi4_tos = flkeys->ip.tos; - fl4->fl4_icmp_type = flkeys->icmp.type; - fl4->fl4_icmp_type = flkeys->icmp.code; - fl4->fl4_gre_key = flkeys->gre.keyid; } #if IS_ENABLED(CONFIG_IPV6) @@ -3427,10 +3434,17 @@ decode_session6(const struct xfrm_flow_keys *flkeys, struct flowi *fl, bool reve fl6->fl6_dport = flkeys->ports.dst; } + switch (flkeys->basic.ip_proto) { + case IPPROTO_GRE: + fl6->fl6_gre_key = flkeys->gre.keyid; + break; + case IPPROTO_ICMPV6: + fl6->fl6_icmp_type = flkeys->icmp.type; + fl6->fl6_icmp_code = flkeys->icmp.code; + break; + } + fl6->flowi6_proto = flkeys->basic.ip_proto; - fl6->fl6_icmp_type = flkeys->icmp.type; - fl6->fl6_icmp_type = flkeys->icmp.code; - fl6->fl6_gre_key = flkeys->gre.keyid; } #endif