netdev - Re: [PATCH ipsec-next v3 0/3] xfrm: policy: replace session decode with flow dissector

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZTpXmUH_GQ0FVD7a@Antony2201.local>
Date: Thu, 26 Oct 2023 14:12:09 +0200
From: Antony Antony <antony@...nome.org>
To: Steffen Klassert <steffen.klassert@...unet.com>
Cc: Florian Westphal <fw@...len.de>, netdev@...r.kernel.org,
	herbert@...dor.apana.org.au
Subject: Re: [PATCH ipsec-next v3 0/3] xfrm: policy: replace session decode
 with flow dissector


On Tue, Oct 10, 2023 at 09:51:16AM +0200, Steffen Klassert wrote:
> On Wed, Oct 04, 2023 at 06:09:50PM +0200, Florian Westphal wrote:
> > Remove the ipv4+ipv6 session decode functions and use generic flow
> > dissector to populate the flowi for the policy lookup.
> > 
> > Changes since v2:
> > - first patch broke CONFIG_XFRM=n builds
> > 
> > Changes since v1:
> > - Can't use skb_flow_dissect(), we might see skbs that have neither
> >   skb->sk nor skb->dev set. Flow dissector WARN()s in this case, it
> >   tries to check for a bpf program assigned in that net namespace.
> > 
> > Add a preparation patch to pass down 'struct net' in
> > xfrm_decode_session so its available for use in patch 3.
> > 
> > Changes since RFC:
> > 
> >  - Drop mobility header support.  I don't think that anyone uses
> >    this.  MOBIKE doesn't appear to need this either.
> >  - Drop fl6->flowlabel assignment, original code leaves it as 0.
> > 
> > There is no reason for this change other than to remove code.
> > 
> > Florian Westphal (3):
> >   xfrm: pass struct net to xfrm_decode_session wrappers
> >   xfrm: move mark and oif flowi decode into common code
> >   xfrm: policy: replace session decode with flow dissector
> 
> Series applied, thanks a lot Florian!
> 

Hi Steffen,

I would like to report a potential bug that I've encountered while working 
on a related patch involving xfrm and ICMP packet decoding using wrapped 
xfrm_decode_session. This issue came to my attention while testing my my 
patch "xfrm: introduce forwarding of ICMP Error messages"

Here is the output from gdb after xfrm_decode_session.

before this series applied
p fl.u.ip4.uli
$3 = {ports = {dport = 11, sport = 0}, icmpt = {type = 11 '\v', code = 0 '\000'}, gre_key = 11, mht = { type = 11 '\v'}}

after this series applied.
p fl.u.ip4.uli
$11 = {ports = {dport = 0, sport = 0}, icmpt = {type = 0 '\000', code = 0 '\000'}, gre_key = 0, mht = { type = 0 '\000'}}

I believe this discrepancy may indicate an issue with the decoding of ICMP 
packets following the application above patches. 

While I could further debug the issue and create a generic test case to 
replicate it without my patch, I wanted to bring it to your attention before 
the ipsec-next branch gets merged into net-next.

regards,
-antony