[PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook

[PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook

[Xiang Mei (Microsoft)]

When nf_hooks_lwtunnel is enabled, the End.DX4/End.DX6 actions dispatch
the decapsulated inner packet through the NF_INET_PRE_ROUTING hook chain
with input_action_end_dx{4,6}_finish() as the okfn. Both functions read
the lwtunnel state via orig_dst = skb_dst(skb) and dereference
orig_dst->lwtstate.

A hook in that chain can legitimately drop the dst: nf_nat_ipv{4,6}_in()
calls skb_dst_drop(skb) when a DNAT rule rewrites the destination
address, leaving skb_dst(skb) NULL. The okfn then dereferences a NULL
orig_dst, causing a general protection fault and a panic (the fault
happens in softirq NAPI receive context).

Free the skb and bail out when the dst was dropped, instead of
proceeding with a lost lwtunnel state.

  Oops: general protection fault, probably for non-canonical address...
  KASAN: null-ptr-deref in range [0x0000000000000080-0x0000000000000087]
  RIP: 0010:input_action_end_dx6_finish (net/ipv6/seg6_local.c:912)
  Call Trace:
   input_action_end_dx6 (net/ipv6/seg6_local.c:946)
   seg6_local_input_core (net/ipv6/seg6_local.c:1621)
   seg6_local_input (net/ipv6/seg6_local.c:1643)
   lwtunnel_input (net/core/lwtunnel.c:465)
   ipv6_rcv (net/ipv6/ip6_input.c:351)
   __netif_receive_skb_core.constprop.0 (net/core/dev.c:6165)
  Kernel panic - not syncing: Fatal exception in interrupt

Fixes: 7a3f5b0de364 ("netfilter: add netfilter hooks to SRv6 data plane")
Reported-by: AutonomousCodeSecurity@microsoft.com
Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
---
 net/ipv6/seg6_local.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index 2b41e4c0dddd..45e66ba306ca 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -909,6 +909,11 @@ static int input_action_end_dx6_finish(struct net *net, struct sock *sk,
 	struct in6_addr *nhaddr = NULL;
 	struct seg6_local_lwt *slwt;
 
+	if (!orig_dst) {
+		kfree_skb(skb);
+		return -EINVAL;
+	}
+
 	slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
 
 	/* The inner packet is not associated to any local interface,
@@ -962,6 +967,11 @@ static int input_action_end_dx4_finish(struct net *net, struct sock *sk,
 	struct iphdr *iph;
 	__be32 nhaddr;
 
+	if (!orig_dst) {
+		kfree_skb(skb);
+		return -EINVAL;
+	}
+
 	slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
 
 	iph = ip_hdr(skb);
-- 
2.43.0

Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook

[Jakub Kicinski]

On Mon, 20 Jul 2026 20:44:29 +0000 Xiang Mei (Microsoft) wrote:
> When nf_hooks_lwtunnel is enabled, the End.DX4/End.DX6 actions dispatch
> the decapsulated inner packet through the NF_INET_PRE_ROUTING hook chain
> with input_action_end_dx{4,6}_finish() as the okfn. Both functions read
> the lwtunnel state via orig_dst = skb_dst(skb) and dereference
> orig_dst->lwtstate.

Per Sashiko's feedback - we need to validate not only that the dst 
is there but also that it is of the expected type, no?
-- 
pw-bot: cr

Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook

[Pablo Neira Ayuso]

On Mon, Jul 20, 2026 at 08:44:29PM +0000, Xiang Mei (Microsoft) wrote:
> When nf_hooks_lwtunnel is enabled, the End.DX4/End.DX6 actions dispatch
> the decapsulated inner packet through the NF_INET_PRE_ROUTING hook chain
> with input_action_end_dx{4,6}_finish() as the okfn. Both functions read
> the lwtunnel state via orig_dst = skb_dst(skb) and dereference
> orig_dst->lwtstate.
> 
> A hook in that chain can legitimately drop the dst: nf_nat_ipv{4,6}_in()
> calls skb_dst_drop(skb) when a DNAT rule rewrites the destination
> address, leaving skb_dst(skb) NULL. The okfn then dereferences a NULL
> orig_dst, causing a general protection fault and a panic (the fault
> happens in softirq NAPI receive context).
> 
> Free the skb and bail out when the dst was dropped, instead of
> proceeding with a lost lwtunnel state.
> 
>   Oops: general protection fault, probably for non-canonical address...
>   KASAN: null-ptr-deref in range [0x0000000000000080-0x0000000000000087]
>   RIP: 0010:input_action_end_dx6_finish (net/ipv6/seg6_local.c:912)
>   Call Trace:
>    input_action_end_dx6 (net/ipv6/seg6_local.c:946)
>    seg6_local_input_core (net/ipv6/seg6_local.c:1621)
>    seg6_local_input (net/ipv6/seg6_local.c:1643)
>    lwtunnel_input (net/core/lwtunnel.c:465)
>    ipv6_rcv (net/ipv6/ip6_input.c:351)
>    __netif_receive_skb_core.constprop.0 (net/core/dev.c:6165)
>   Kernel panic - not syncing: Fatal exception in interrupt
> 
> Fixes: 7a3f5b0de364 ("netfilter: add netfilter hooks to SRv6 data plane")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
> ---
>  net/ipv6/seg6_local.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
> index 2b41e4c0dddd..45e66ba306ca 100644
> --- a/net/ipv6/seg6_local.c
> +++ b/net/ipv6/seg6_local.c
> @@ -909,6 +909,11 @@ static int input_action_end_dx6_finish(struct net *net, struct sock *sk,
>  	struct in6_addr *nhaddr = NULL;
>  	struct seg6_local_lwt *slwt;
>  
> +	if (!orig_dst) {

This should be:

        if (!skb_valid_dst(orig_dst)) {

> +		kfree_skb(skb);
> +		return -EINVAL;
> +	}
> +
>  	slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
>  
>  	/* The inner packet is not associated to any local interface,
> @@ -962,6 +967,11 @@ static int input_action_end_dx4_finish(struct net *net, struct sock *sk,
>  	struct iphdr *iph;
>  	__be32 nhaddr;
>  
> +	if (!orig_dst) {

Same here.

> +		kfree_skb(skb);
> +		return -EINVAL;
> +	}
> +
>  	slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
>  
>  	iph = ip_hdr(skb);
> -- 
> 2.43.0
> 

Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook

[Andrea Mayer]

On Thu, 23 Jul 2026 10:09:49 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Mon, 20 Jul 2026 20:44:29 +0000 Xiang Mei (Microsoft) wrote:
> > When nf_hooks_lwtunnel is enabled, the End.DX4/End.DX6 actions dispatch
> > the decapsulated inner packet through the NF_INET_PRE_ROUTING hook chain
> > with input_action_end_dx{4,6}_finish() as the okfn. Both functions read
> > the lwtunnel state via orig_dst = skb_dst(skb) and dereference
> > orig_dst->lwtstate.
> 
> Per Sashiko's feedback - we need to validate not only that the dst 
> is there but also that it is of the expected type, no?
> -- 
> pw-bot: cr

Right, and seg6_input_core/seg6_output_core in seg6_iptunnel.c (added by
7a3f5b0de364) read skb_dst()->lwtstate after a NF_HOOK too, and this patch does
not touch them. A hook can leave a valid dst whose lwtstate is NULL there
(reproducible with SNAT and an XFRM policy), so they need fixing as well.

seg6/seg6local behaviors carry their processing state in the route's
lwtunnel_state. That state should be preserved across the hook, and after the
hook skb_dst() may not carry it, or may point to a different instance of the
same type.

Thanks,
Andrea

Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook

[Xiang Mei]

On Thu, Jul 23, 2026 at 10:10 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 20 Jul 2026 20:44:29 +0000 Xiang Mei (Microsoft) wrote:
> > When nf_hooks_lwtunnel is enabled, the End.DX4/End.DX6 actions dispatch
> > the decapsulated inner packet through the NF_INET_PRE_ROUTING hook chain
> > with input_action_end_dx{4,6}_finish() as the okfn. Both functions read
> > the lwtunnel state via orig_dst = skb_dst(skb) and dereference
> > orig_dst->lwtstate.
>
> Per Sashiko's feedback - we need to validate not only that the dst
> is there but also that it is of the expected type, no?
> --
> pw-bot: cr

Thanks for the review. You are right, I'll do the following check in v2:

  if (!skb_valid_dst(skb) || !dst->lwtstate ||
      dst->lwtstate->type != LWTUNNEL_ENCAP_SEG6_LOCAL)
          return NULL;

v2 also covers three more functions with the same problem, so it is now a
series of two patches. Details in the reply to Andrea.

Xiang

Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook

[Xiang Mei]

On Thu, Jul 23, 2026 at 10:22 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Mon, Jul 20, 2026 at 08:44:29PM +0000, Xiang Mei (Microsoft) wrote:
> > When nf_hooks_lwtunnel is enabled, the End.DX4/End.DX6 actions dispatch
> > the decapsulated inner packet through the NF_INET_PRE_ROUTING hook chain
> > with input_action_end_dx{4,6}_finish() as the okfn. Both functions read
> > the lwtunnel state via orig_dst = skb_dst(skb) and dereference
> > orig_dst->lwtstate.
> >
> > A hook in that chain can legitimately drop the dst: nf_nat_ipv{4,6}_in()
> > calls skb_dst_drop(skb) when a DNAT rule rewrites the destination
> > address, leaving skb_dst(skb) NULL. The okfn then dereferences a NULL
> > orig_dst, causing a general protection fault and a panic (the fault
> > happens in softirq NAPI receive context).
> >
> > Free the skb and bail out when the dst was dropped, instead of
> > proceeding with a lost lwtunnel state.
> >
> >   Oops: general protection fault, probably for non-canonical address...
> >   KASAN: null-ptr-deref in range [0x0000000000000080-0x0000000000000087]
> >   RIP: 0010:input_action_end_dx6_finish (net/ipv6/seg6_local.c:912)
> >   Call Trace:
> >    input_action_end_dx6 (net/ipv6/seg6_local.c:946)
> >    seg6_local_input_core (net/ipv6/seg6_local.c:1621)
> >    seg6_local_input (net/ipv6/seg6_local.c:1643)
> >    lwtunnel_input (net/core/lwtunnel.c:465)
> >    ipv6_rcv (net/ipv6/ip6_input.c:351)
> >    __netif_receive_skb_core.constprop.0 (net/core/dev.c:6165)
> >   Kernel panic - not syncing: Fatal exception in interrupt
> >
> > Fixes: 7a3f5b0de364 ("netfilter: add netfilter hooks to SRv6 data plane")
> > Reported-by: AutonomousCodeSecurity@microsoft.com
> > Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
> > ---
> >  net/ipv6/seg6_local.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
> > index 2b41e4c0dddd..45e66ba306ca 100644
> > --- a/net/ipv6/seg6_local.c
> > +++ b/net/ipv6/seg6_local.c
> > @@ -909,6 +909,11 @@ static int input_action_end_dx6_finish(struct net *net, struct sock *sk,
> >       struct in6_addr *nhaddr = NULL;
> >       struct seg6_local_lwt *slwt;
> >
> > +     if (!orig_dst) {
>
> This should be:
>
>         if (!skb_valid_dst(orig_dst)) {
>

That's correct, thanks. v2 gonna use it. Two small notes.

skb_valid_dst() takes the skb, not the dst.

The lwtstate NULL test is still needed on top. A hook can leave a valid
route that has no lwtunnel state, and seg6_local_lwtunnel() would then
read lwt->data off NULL.

So v2 has:

  static struct seg6_local_lwt *seg6_local_lwt_from_skb(struct sk_buff *skb)
  {
        struct dst_entry *dst = skb_dst(skb);

        if (!skb_valid_dst(skb) || !dst->lwtstate ||
            dst->lwtstate->type != LWTUNNEL_ENCAP_SEG6_LOCAL)
                return NULL;

        return seg6_local_lwtunnel(dst->lwtstate);
  }

Xiang

> > +             kfree_skb(skb);
> > +             return -EINVAL;
> > +     }
> > +
> >       slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
> >
> >       /* The inner packet is not associated to any local interface,
> > @@ -962,6 +967,11 @@ static int input_action_end_dx4_finish(struct net *net, struct sock *sk,
> >       struct iphdr *iph;
> >       __be32 nhaddr;
> >
> > +     if (!orig_dst) {
>
> Same here.
>
> > +             kfree_skb(skb);
> > +             return -EINVAL;
> > +     }
> > +
> >       slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
> >
> >       iph = ip_hdr(skb);
> > --
> > 2.43.0
> >

Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook

[Xiang Mei]

On Fri, Jul 24, 2026 at 6:11 AM Andrea Mayer <andrea.mayer@uniroma2.it> wrote:
>
> On Thu, 23 Jul 2026 10:09:49 -0700
> Jakub Kicinski <kuba@kernel.org> wrote:
>
> > On Mon, 20 Jul 2026 20:44:29 +0000 Xiang Mei (Microsoft) wrote:
> > > When nf_hooks_lwtunnel is enabled, the End.DX4/End.DX6 actions dispatch
> > > the decapsulated inner packet through the NF_INET_PRE_ROUTING hook chain
> > > with input_action_end_dx{4,6}_finish() as the okfn. Both functions read
> > > the lwtunnel state via orig_dst = skb_dst(skb) and dereference
> > > orig_dst->lwtstate.
> >
> > Per Sashiko's feedback - we need to validate not only that the dst
> > is there but also that it is of the expected type, no?
> > --
> > pw-bot: cr
>
> Right, and seg6_input_core/seg6_output_core in seg6_iptunnel.c (added by
> 7a3f5b0de364) read skb_dst()->lwtstate after a NF_HOOK too, and this patch does
> not touch them. A hook can leave a valid dst whose lwtstate is NULL there
> (reproducible with SNAT and an XFRM policy), so they need fixing as well.
>

You are right. Following your hint, I also found seg6_local_input_core(),
which has the same shape. v2 is a series of two patches and covers all
five sites:

seg6_local.c:
  seg6_local_input_core()        LOCAL_IN
  input_action_end_dx4_finish()  PRE_ROUTING
  input_action_end_dx6_finish()  PRE_ROUTING
seg6_iptunnel.c
  seg6_input_core()              POST_ROUTING
  seg6_output_core()             POST_ROUTING

In v2:
  1/2 is for seg6_local.c; 2/2 is for seg6_iptunnel.c.

> seg6/seg6local behaviors carry their processing state in the route's
> lwtunnel_state. That state should be preserved across the hook, and after the
> hook skb_dst() may not carry it, or may point to a different instance of the
> same type.
>
> Thanks,
> Andrea

I think you are right about the model, and I spent some time trying to
follow it. These are the places the state could live across the hook:

  okfn argument   fixed signature, cannot change
  skb->cb         owned by IPCB/IP6CB during the chain
  out of band     stale after NF_QUEUE and a late reinject
  re-attach dst   defeats the rule that dropped it
  skb extension   works

So an skb extension really would do it. SKB_EXT_* is refcounted, rides
with the skb, and nf_queue leaves it alone. I want to be clear that your
suggestion is workable; I just did not take it in this series.

My reasoning is scope. It means a new skb_ext type plus an lwtstate
reference on every seg6 packet, which feels like net-next material.

There is also one open question. I want your view on it, and that of
the other maintainers.

Say a DNAT rule in PRE_ROUTING rewrites the inner packet's destination,
and End.DX4 then runs with its state preserved. Then which one is intended:

1) send the packet to slwt->nh4, the nexthop from the SRv6 route.
     This ignores the DNAT rule the admin installed.

 2) route on the new destination instead. This ignores the nexthop the
     SRv6 route asked for.

v2 does neither. It drops the packet. If you have a suggestion for
this, I can do it in v3.

I left the "different instance of the same type" case alone for the same
reason. The type test passes, and the behavior uses another route's
parameters. It is wrong, but it is not a memory bug, and it predates the
crash.

Thanks again,
Xiang

Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook

[Andrea Mayer]

On Tue, 28 Jul 2026 14:49:08 -0700
Xiang Mei <xmei5@asu.edu> wrote:

> [snip]
>
> You are right. Following your hint, I also found seg6_local_input_core(),
> which has the same shape. v2 is a series of two patches and covers all
> five sites:
>
> seg6_local.c:
>   seg6_local_input_core()        LOCAL_IN
>   input_action_end_dx4_finish()  PRE_ROUTING
>   input_action_end_dx6_finish()  PRE_ROUTING
> seg6_iptunnel.c
>   seg6_input_core()              POST_ROUTING
>   seg6_output_core()             POST_ROUTING

Thanks for covering all five sites.

I ran into this pattern during the MUP v2 review [1] and have been
working on the skb_ext approach since. My work covers these sites and
also includes selftests for DNAT. I will post it as an RFC for
net-next.

> [snip]
>
> So an skb extension really would do it. SKB_EXT_* is refcounted, rides
> with the skb, and nf_queue leaves it alone. I want to be clear that your
> suggestion is workable; I just did not take it in this series.
>
> My reasoning is scope. It means a new skb_ext type plus an lwtstate
> reference on every seg6 packet, which feels like net-next material.

As things stand, preserving state across the hook needs new machinery,
so type-check and drop is a reasonable fix for net.

> There is also one open question. I want your view on it, and that of
> the other maintainers.
>
> Say a DNAT rule in PRE_ROUTING rewrites the inner packet's destination,
> and End.DX4 then runs with its state preserved. Then which one is intended:
>
> 1) send the packet to slwt->nh4, the nexthop from the SRv6 route.
>      This ignores the DNAT rule the admin installed.
>
>  2) route on the new destination instead. This ignores the nexthop the
>      SRv6 route asked for.
>
> v2 does neither. It drops the packet. If you have a suggestion for
> this, I can do it in v3.

For net, dropping is the safe choice. Before the fix, the same scenario
panics the kernel (NULL deref), so a clean drop is not a regression.
I will look at v2.

With the lwtstate preserved (e.g., the skb_ext approach), the existing
code already handles both cases. Indeed, End.DX4 does
nhaddr = slwt->nh4.s_addr ?: iph->daddr, so a configured nexthop takes
precedence over the rewritten address, while an unconfigured one lets
the DNAT destination drive the lookup. End.DX6 follows the same pattern
through seg6_lookup_nexthop.

> I left the "different instance of the same type" case alone for the same
> reason. The type test passes, and the behavior uses another route's
> parameters. It is wrong, but it is not a memory bug, and it predates the
> crash.

Without saving the original lwtstate before NF_HOOK, in theory there is
no guarantee that the instance is the same after the hook returns. The
skb_ext approach addresses this too, since it preserves the original
lwtstate rather than re-reading it from the current dst.

[1] https://lore.kernel.org/netdev/20260516182556.66af27a9c63208435911990b@uniroma2.it/

> Thanks again,
> Xiang

Thanks,

Ciao,
Andrea

Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook

[Pablo Neira Ayuso]

On Thu, Jul 30, 2026 at 05:16:10PM +0200, Andrea Mayer wrote:
> On Tue, 28 Jul 2026 14:49:08 -0700
> Xiang Mei <xmei5@asu.edu> wrote:
> 
> > [snip]
> >
> > You are right. Following your hint, I also found seg6_local_input_core(),
> > which has the same shape. v2 is a series of two patches and covers all
> > five sites:
> >
> > seg6_local.c:
> >   seg6_local_input_core()        LOCAL_IN
> >   input_action_end_dx4_finish()  PRE_ROUTING
> >   input_action_end_dx6_finish()  PRE_ROUTING
> > seg6_iptunnel.c
> >   seg6_input_core()              POST_ROUTING
> >   seg6_output_core()             POST_ROUTING
> 
> Thanks for covering all five sites.
> 
> I ran into this pattern during the MUP v2 review [1] and have been
> working on the skb_ext approach since. My work covers these sites and
> also includes selftests for DNAT. I will post it as an RFC for
> net-next.
> 
> > [snip]
> >
> > So an skb extension really would do it. SKB_EXT_* is refcounted, rides
> > with the skb, and nf_queue leaves it alone. I want to be clear that your
> > suggestion is workable; I just did not take it in this series.
> >
> > My reasoning is scope. It means a new skb_ext type plus an lwtstate
> > reference on every seg6 packet, which feels like net-next material.
> 
> As things stand, preserving state across the hook needs new machinery,
> so type-check and drop is a reasonable fix for net.
> 
> > There is also one open question. I want your view on it, and that of
> > the other maintainers.
> >
> > Say a DNAT rule in PRE_ROUTING rewrites the inner packet's destination,
> > and End.DX4 then runs with its state preserved. Then which one is intended:
> >
> > 1) send the packet to slwt->nh4, the nexthop from the SRv6 route.
> >      This ignores the DNAT rule the admin installed.
> >
> >  2) route on the new destination instead. This ignores the nexthop the
> >      SRv6 route asked for.
> >
> > v2 does neither. It drops the packet. If you have a suggestion for
> > this, I can do it in v3.
> 
> For net, dropping is the safe choice. Before the fix, the same scenario
> panics the kernel (NULL deref), so a clean drop is not a regression.
> I will look at v2.

I would go for dropping the packet too, it is a simple fix for this crash.

> With the lwtstate preserved (e.g., the skb_ext approach), the existing
> code already handles both cases. Indeed, End.DX4 does
> nhaddr = slwt->nh4.s_addr ?: iph->daddr, so a configured nexthop takes
> precedence over the rewritten address, while an unconfigured one lets
> the DNAT destination drive the lookup. End.DX6 follows the same pattern
> through seg6_lookup_nexthop.

What is the usecase for a hook to clear lwtstate information?

> > I left the "different instance of the same type" case alone for the same
> > reason. The type test passes, and the behavior uses another route's
> > parameters. It is wrong, but it is not a memory bug, and it predates the
> > crash.
> 
> Without saving the original lwtstate before NF_HOOK, in theory there is
> no guarantee that the instance is the same after the hook returns. The
> skb_ext approach addresses this too, since it preserves the original
> lwtstate rather than re-reading it from the current dst.
> 
> [1] https://lore.kernel.org/netdev/20260516182556.66af27a9c63208435911990b@uniroma2.it/
> 
> > Thanks again,
> > Xiang
> 
> Thanks,
> 
> Ciao,
> Andrea

Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook

[Andrea Mayer]

On Thu, 30 Jul 2026 17:23:09 +0200
Pablo Neira Ayuso <pablo@netfilter.org> wrote:

> [snip]
>
> > For net, dropping is the safe choice. Before the fix, the same scenario
> > panics the kernel (NULL deref), so a clean drop is not a regression.
> > I will look at v2.
>
> I would go for dropping the packet too, it is a simple fix for this crash.
>
> > With the lwtstate preserved (e.g., the skb_ext approach), the existing
> > code already handles both cases. Indeed, End.DX4 does
> > nhaddr = slwt->nh4.s_addr ?: iph->daddr, so a configured nexthop takes
> > precedence over the rewritten address, while an unconfigured one lets
> > the DNAT destination drive the lookup. End.DX6 follows the same pattern
> > through seg6_lookup_nexthop.
>
> What is the usecase for a hook to clear lwtstate information?

Re-routing after NAT. DNAT changes the destination and the dst is dropped.
SNAT changes the source and, with a matching XFRM policy, the dst is
replaced. The lwtstate is not involved, it just lives on the dst.

seg6_local and seg6_iptunnel call NF_HOOK from inside lwtunnel processing,
and their okfns then read the per-route state from skb_dst(skb)->lwtstate.
The relevant part of the configuration that produces the crash:

  # SRv6 ingress node
  ip -4 route add 10.0.0.99/32 \
          encap seg6 mode encap segs fc00:12:100::6004 dev veth0

  # SRv6 egress node
  sysctl -w net.netfilter.nf_hooks_lwtunnel=1
  ip -6 route add fc00:12:100::6004/128 \
          encap seg6local action End.DX4 nh4 10.0.0.2 dev veth-t100
  iptables -t nat -A PREROUTING -d 10.0.0.99 \
          -j DNAT --to-destination 10.0.0.222

End.DX4 decapsulates the inner IPv4 packet, then runs the PRE_ROUTING chain
on it with input_action_end_dx4_finish() as the okfn. The packet enters the
chain with the dst of the SID route still attached. The DNAT rule matches,
so the dst is dropped, and the okfn then dereferences NULL. That dst is
where it reads the lwtstate holding the End.DX4 parameters, nh4 among them.

In seg6_iptunnel the same happens at POST_ROUTING, where the okfns are
seg6_input_core() and seg6_output_core(). With SNAT and a matching XFRM
policy they find a valid dst whose lwtstate is NULL.

In the current tree seg6_local and seg6_iptunnel are the only lwtunnels
that call NF_HOOK, so this is not a general lwtunnel problem. I think the
fix belongs on the SRv6 side.

Ciao,
Andrea