[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

[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 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

[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

[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

[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
> >