From: Pablo Neira Ayuso <pablo@netfilter•org>
To: Lorenzo Bianconi <lorenzo@kernel•org>
Cc: Florian Westphal <fw@strlen•de>, Phil Sutter <phil@nwl•cc>,
"David S. Miller" <davem@davemloft•net>,
Eric Dumazet <edumazet@google•com>,
Jakub Kicinski <kuba@kernel•org>, Paolo Abeni <pabeni@redhat•com>,
Simon Horman <horms@kernel•org>,
netfilter-devel@vger•kernel.org, coreteam@netfilter•org,
netdev@vger•kernel.org
Subject: Re: [PATCH nf] netfilter: flowtable: use pskb_may_pull() in nf_flow_ip6_tunnel_proto()
Date: Tue, 9 Jun 2026 13:48:32 +0200 [thread overview]
Message-ID: <aif9kL38LKNcX1Xu@chamomile> (raw)
In-Reply-To: <aifquGhK_Cijxq7m@lore-desk>
On Tue, Jun 09, 2026 at 12:28:08PM +0200, Lorenzo Bianconi wrote:
> On Jun 09, Pablo Neira Ayuso wrote:
> > Hi Lorenzo,
> >
> > On Mon, Jun 08, 2026 at 07:06:52PM +0200, Lorenzo Bianconi wrote:
> > > Switch nf_flow_ip6_tunnel_proto() from skb_header_pointer() to
> > > pskb_may_pull() for header validation, aligning it with the approach
> > > used in nf_flow_ip4_tunnel_proto().
> > > Move ctx->offset update inside the IPPROTO_IPV6 conditional block since
> > > it should only be adjusted when a tunnel is actually detected.
> > > While at it, use nexthdr instead of the hardcoded IPPROTO_IPV6 constant
> > > when setting ctx->tun.proto.
> > >
> > > Fixes: d98103575dcdd ("netfilter: flowtable: Add IP6IP6 rx sw acceleration")
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel•org>
> > > ---
> > > net/netfilter/nf_flow_table_ip.c | 10 +++++-----
> > > 1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
> > > index 9c05a50d6013..2946399ab715 100644
> > > --- a/net/netfilter/nf_flow_table_ip.c
> > > +++ b/net/netfilter/nf_flow_table_ip.c
> > > @@ -347,15 +347,15 @@ static bool nf_flow_ip6_tunnel_proto(struct nf_flowtable_ctx *ctx,
> > > struct sk_buff *skb)
> > > {
> > > #if IS_ENABLED(CONFIG_IPV6)
> > > - struct ipv6hdr *ip6h, _ip6h;
> > > + struct ipv6hdr *ip6h;
> > > __be16 frag_off;
> > > u8 nexthdr;
> > > int hdrlen;
> > >
> > > - ip6h = skb_header_pointer(skb, ctx->offset, sizeof(*ip6h), &_ip6h);
> > > - if (!ip6h)
> > > + if (!pskb_may_pull(skb, sizeof(*ip6h) + ctx->offset))
> > > return false;
> > >
> > > + ip6h = (struct ipv6hdr *)(skb_network_header(skb) + ctx->offset);
> > > if (ip6h->hop_limit <= 1)
> > > return false;
> >
> > Not shown in the patch, but is there still a corner case here that
> > needs to be covered?
> >
> > ipv6_skip_exthdr() uses skb_header_pointer() internal, then another
> > pskb_may_pull() is needed to make sure no other IPv6 extension header
> > sits between the outer and the inner IPPROTO_IPV6 header, allowing to
> > be in a non-linear area of the skb?
> >
> > > @@ -367,9 +367,9 @@ static bool nf_flow_ip6_tunnel_proto(struct nf_flowtable_ctx *ctx,
> > >
> >
> > I mean:
> >
> > if (!pskb_may_pull(skb, hdrlen))
> > return false;
> >
> > where hdrlen is what ipv6_skip_exthdr() returns.
> >
> > Then, I think it should be safe to call skb_pull() on
> > ctx->tun.hdr_size.
> >
> > Let me know, thanks.
>
> I think you are right, here we need to run:
>
> if (!pskb_may_pull(skb, hdrlen))
> return false;
>
> in order to be sure we can pull ctx->tun.hdr_size in nf_flow_ip_tunnel_pop().
> Doing so, we can roll-back to the original skb_header_pointer() to access the
> outer ip6 header here. What do you think?
Yes, initial skb_header_pointer() then pskb_may_pull(skb, hdrlen) to
ensure the entire should be fine.
I think this need one more fix: This needs to resort to classic path
if there are intermediate extension headers sitting in between the
outer and inner headers in IP6IP6, ie. ipv6_ext_hdr() == true. Those
extensions need to be handled by the IPv6 stack.
nf_flow_ip6_tunnel_proto() needs to be fixed to deal with this.
And I suspect nf_flow_ip4_tunnel_proto() with IP options have the same
problem, the flowtable need to resort to the classic stack path.
Thanks.
next prev parent reply other threads:[~2026-06-09 11:48 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-08 17:06 [PATCH nf] netfilter: flowtable: use pskb_may_pull() in nf_flow_ip6_tunnel_proto() Lorenzo Bianconi
2026-06-08 23:11 ` Pablo Neira Ayuso
2026-06-09 10:28 ` Lorenzo Bianconi
2026-06-09 11:48 ` Pablo Neira Ayuso [this message]
2026-06-09 12:31 ` Lorenzo Bianconi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aif9kL38LKNcX1Xu@chamomile \
--to=pablo@netfilter$(echo .)org \
--cc=coreteam@netfilter$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=fw@strlen$(echo .)de \
--cc=horms@kernel$(echo .)org \
--cc=kuba@kernel$(echo .)org \
--cc=lorenzo@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=netfilter-devel@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=phil@nwl$(echo .)cc \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox