Re: [PATCH net v2 1/2] seg6: check lwtunnel state after nf hooks in seg6local

[Andrea Mayer]

On Tue, 28 Jul 2026 21:54:47 +0000
"Xiang Mei (Microsoft)" <xmei5@xxxxxxx> wrote:

Thanks for the v2.

The overall code looks fine.

> When nf_hooks_lwtunnel is enabled, three seg6local functions run as the
> okfn of a netfilter hook and recover the behavior's parameters by
> re-reading skb_dst(skb)->lwtstate:
>
>   seg6_local_input_core()          LOCAL_IN
>   input_action_end_dx4_finish()    PRE_ROUTING
>   input_action_end_dx6_finish()    PRE_ROUTING
>
> The okfn signature is (net, sk, skb), so the skb's dst is the only place
> the state can be found, but a hook is free to change it.
> nf_nat_ipv{4,6}_in() calls skb_dst_drop(skb) once a NAT rule has
> rewritten the addresses, and the okfn then dereferences NULL. A hook can

Isn't nf_nat_ipv4_in() called nf_nat_ipv4_pre_routing() nowadays? The IPv6
one still has _in. The rest of the sentence is right.

> also leave a metadata dst, a route whose lwtstate is NULL, or a route of
> a foreign encap type, the last of which makes seg6_local_lwtunnel()
> reinterpret another structure so that seg6_local_input_core() calls
> through slwt->desc->input.
>
> An unprivileged user can reach the End.DX6 case from a user and network
> namespace: enable nf_hooks_lwtunnel, install a seg6local End.DX6 route
> and an nftables ip6 nat PRE_ROUTING DNAT rule, then send one matching
> SRv6 packet. It panics in softirq NAPI receive context:
>
>   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
>
> Add a helper that returns the state only when skb_dst() still is a real
> (non-metadata) route whose lwtstate is of the expected type, and drop the
> packet otherwise.
>
> The six other callers of seg6_local_lwtunnel() take the lwtunnel_state as
> an argument from the lwtunnel core, which dispatched them through
> seg6_local_ops, so the pointer and its type are known good.
>

That is true except for seg6_local_build_state(), which allocates the
lwtunnel_state with lwtunnel_state_alloc() and passes it back through the
output parameter. The pointer is still known good there, just for a
different reason.

> Fixes: 7a3f5b0de364 ("netfilter: add netfilter hooks to SRv6 data plane")
> Reported-by: AutonomousCodeSecurity@xxxxxxxxxxxxx
> Signed-off-by: Xiang Mei (Microsoft) <xmei5@xxxxxxx>
> ---
> v2:
>  - Check that the dst is valid and its lwtstate is of the expected type,
>    not just that the dst is non-NULL (Jakub Kicinski, Pablo Neira Ayuso,
>    Sashiko).
>  - Also fix seg6_local_input_core(), same okfn-after-NF_HOOK shape.
>  - seg6_iptunnel.c is handled in patch 2/2 (Andrea Mayer).
>
> v1: https://lore.kernel.org/all/20260720204430.1886091-1-xmei5%40asu.edu/
>
>  net/ipv6/seg6_local.c | 34 ++++++++++++++++++++++++++++------
>  1 file changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
> index 2b41e4c0dddd..b75a4dc3a36d 100644
> --- a/net/ipv6/seg6_local.c
> +++ b/net/ipv6/seg6_local.c
> @@ -24,6 +24,7 @@
>  #include <net/addrconf.h>
>  #include <net/ip6_route.h>
>  #include <net/dst_cache.h>
> +#include <net/dst_metadata.h>
>  #include <net/ip_tunnels.h>
>  #ifdef CONFIG_IPV6_SEG6_HMAC
>  #include <net/seg6_hmac.h>
> @@ -213,6 +214,17 @@ static struct seg6_local_lwt *seg6_local_lwtunnel(struct lwtunnel_state *lwt)
>  	return (struct seg6_local_lwt *)lwt->data;
>  }
>
> +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;

The check is type-level, so it stops the crash without guaranteeing the
same instance, as we discussed on v1. That is reasonable for net.

> +
> +	return seg6_local_lwtunnel(dst->lwtstate);
> +}
> +
>  static struct ipv6_sr_hdr *get_and_validate_srh(struct sk_buff *skb)
>  {
>  	struct ipv6_sr_hdr *srh;
> @@ -905,11 +917,14 @@ static int input_action_end_dx2(struct sk_buff *skb,
>  static int input_action_end_dx6_finish(struct net *net, struct sock *sk,
>  				       struct sk_buff *skb)
>  {
> -	struct dst_entry *orig_dst = skb_dst(skb);
>  	struct in6_addr *nhaddr = NULL;
>  	struct seg6_local_lwt *slwt;
>
> -	slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
> +	slwt = seg6_local_lwt_from_skb(skb);
> +	if (!slwt) {
> +		kfree_skb(skb);
> +		return -EINVAL;
> +	}

One nit: the same check has unlikely() in patch 2/2 but not here. It is
just as rare here, so I would add it, and to the others below as well.

>
>  	/* The inner packet is not associated to any local interface,
>  	 * so we do not call netif_rx().
> @@ -956,13 +971,16 @@ static int input_action_end_dx6(struct sk_buff *skb,
>  static int input_action_end_dx4_finish(struct net *net, struct sock *sk,
>  				       struct sk_buff *skb)
>  {
> -	struct dst_entry *orig_dst = skb_dst(skb);
>  	enum skb_drop_reason reason;
>  	struct seg6_local_lwt *slwt;
>  	struct iphdr *iph;
>  	__be32 nhaddr;
>
> -	slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
> +	slwt = seg6_local_lwt_from_skb(skb);
> +	if (!slwt) {
> +		kfree_skb(skb);
> +		return -EINVAL;
> +	}
>
>  	iph = ip_hdr(skb);
>
> @@ -1609,13 +1627,17 @@ static void seg6_local_update_counters(struct seg6_local_lwt *slwt,
>  static int seg6_local_input_core(struct net *net, struct sock *sk,
>  				 struct sk_buff *skb)
>  {
> -	struct dst_entry *orig_dst = skb_dst(skb);
>  	struct seg6_action_desc *desc;
>  	struct seg6_local_lwt *slwt;
>  	unsigned int len = skb->len;
>  	int rc;
>
> -	slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
> +	slwt = seg6_local_lwt_from_skb(skb);
> +	if (!slwt) {
> +		kfree_skb(skb);
> +		return -EINVAL;
> +	}
> +
>  	desc = slwt->desc;
>
>  	rc = desc->input(skb, slwt);
> --
> 2.43.0
>

I will follow up on patch 2/2.

Ciao,
Andrea