* [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook
@ 2026-07-20 20:44 Xiang Mei (Microsoft)
2026-07-23 17:09 ` Jakub Kicinski
2026-07-23 17:22 ` Pablo Neira Ayuso
0 siblings, 2 replies; 9+ messages in thread
From: Xiang Mei (Microsoft) @ 2026-07-20 20:44 UTC (permalink / raw)
To: Andrea Mayer, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
Cc: netdev, linux-kernel, Pablo Neira Ayuso, Ryoga Saito,
AutonomousCodeSecurity, tgopinath, kys, 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook
2026-07-20 20:44 [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook Xiang Mei (Microsoft)
@ 2026-07-23 17:09 ` Jakub Kicinski
2026-07-24 13:11 ` Andrea Mayer
2026-07-28 21:16 ` Xiang Mei
2026-07-23 17:22 ` Pablo Neira Ayuso
1 sibling, 2 replies; 9+ messages in thread
From: Jakub Kicinski @ 2026-07-23 17:09 UTC (permalink / raw)
To: Xiang Mei (Microsoft)
Cc: Andrea Mayer, David S . Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, netdev, linux-kernel, Pablo Neira Ayuso,
Ryoga Saito, AutonomousCodeSecurity, tgopinath, kys
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook
2026-07-20 20:44 [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook Xiang Mei (Microsoft)
2026-07-23 17:09 ` Jakub Kicinski
@ 2026-07-23 17:22 ` Pablo Neira Ayuso
2026-07-28 21:18 ` Xiang Mei
1 sibling, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2026-07-23 17:22 UTC (permalink / raw)
To: Xiang Mei (Microsoft)
Cc: Andrea Mayer, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, netdev, linux-kernel, Ryoga Saito,
AutonomousCodeSecurity, tgopinath, kys
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook
2026-07-23 17:09 ` Jakub Kicinski
@ 2026-07-24 13:11 ` Andrea Mayer
2026-07-28 21:49 ` Xiang Mei
2026-07-28 21:16 ` Xiang Mei
1 sibling, 1 reply; 9+ messages in thread
From: Andrea Mayer @ 2026-07-24 13:11 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Xiang Mei (Microsoft), David S . Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, netdev, linux-kernel,
Pablo Neira Ayuso, Ryoga Saito, AutonomousCodeSecurity, tgopinath,
kys, stefano.salsano, 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook
2026-07-23 17:09 ` Jakub Kicinski
2026-07-24 13:11 ` Andrea Mayer
@ 2026-07-28 21:16 ` Xiang Mei
1 sibling, 0 replies; 9+ messages in thread
From: Xiang Mei @ 2026-07-28 21:16 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrea Mayer, David S . Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, netdev, linux-kernel, Pablo Neira Ayuso,
Ryoga Saito, AutonomousCodeSecurity, tgopinath, kys
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook
2026-07-23 17:22 ` Pablo Neira Ayuso
@ 2026-07-28 21:18 ` Xiang Mei
0 siblings, 0 replies; 9+ messages in thread
From: Xiang Mei @ 2026-07-28 21:18 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Andrea Mayer, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, netdev, linux-kernel, Ryoga Saito,
AutonomousCodeSecurity, tgopinath, kys
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
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook
2026-07-24 13:11 ` Andrea Mayer
@ 2026-07-28 21:49 ` Xiang Mei
2026-07-30 15:16 ` Andrea Mayer
0 siblings, 1 reply; 9+ messages in thread
From: Xiang Mei @ 2026-07-28 21:49 UTC (permalink / raw)
To: Andrea Mayer
Cc: Jakub Kicinski, David S . Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, netdev, linux-kernel, Pablo Neira Ayuso,
Ryoga Saito, AutonomousCodeSecurity, tgopinath, kys,
stefano.salsano
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook
2026-07-28 21:49 ` Xiang Mei
@ 2026-07-30 15:16 ` Andrea Mayer
2026-07-30 15:23 ` Pablo Neira Ayuso
0 siblings, 1 reply; 9+ messages in thread
From: Andrea Mayer @ 2026-07-30 15:16 UTC (permalink / raw)
To: Xiang Mei
Cc: Jakub Kicinski, David S . Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, netdev, linux-kernel, Pablo Neira Ayuso,
Ryoga Saito, AutonomousCodeSecurity, tgopinath, kys,
stefano.salsano, justin.iurman, 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook
2026-07-30 15:16 ` Andrea Mayer
@ 2026-07-30 15:23 ` Pablo Neira Ayuso
0 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2026-07-30 15:23 UTC (permalink / raw)
To: Andrea Mayer
Cc: Xiang Mei, Jakub Kicinski, David S . Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, netdev, linux-kernel, Ryoga Saito,
AutonomousCodeSecurity, tgopinath, kys, stefano.salsano,
justin.iurman
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-07-30 15:23 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20 20:44 [PATCH net] seg6: fix NULL deref in input_action_end_dx{4,6}_finish() after nf hook Xiang Mei (Microsoft)
2026-07-23 17:09 ` Jakub Kicinski
2026-07-24 13:11 ` Andrea Mayer
2026-07-28 21:49 ` Xiang Mei
2026-07-30 15:16 ` Andrea Mayer
2026-07-30 15:23 ` Pablo Neira Ayuso
2026-07-28 21:16 ` Xiang Mei
2026-07-23 17:22 ` Pablo Neira Ayuso
2026-07-28 21:18 ` Xiang Mei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox