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 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. Fixes: 7a3f5b0de364 ("netfilter: add netfilter hooks to SRv6 data plane") Reported-by: AutonomousCodeSecurity@microsoft.com Signed-off-by: Xiang Mei (Microsoft) --- 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 #include #include +#include #include #ifdef CONFIG_IPV6_SEG6_HMAC #include @@ -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; + + 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; + } /* 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 seg6_input_core() and seg6_output_core() are the okfns of the POST_ROUTING hook that seg6_input_nf() and seg6_output_nf() dispatch through when nf_hooks_lwtunnel is enabled, and they read skb_dst(skb)->lwtstate on the assumption fixed for seg6local in patch 1. A hook may drop the dst, replace it with a metadata dst, or leave a route whose lwtstate is NULL; the last is reachable with SNAT plus an XFRM policy. Validate the dst and the encap type before use. seg6_do_srh() reads skb_dst(skb)->lwtstate too, but these two are its only callers and neither touches the dst in between. In seg6_output_core() the validated lwtstate also takes over the dst-loop comparison, so "orig_dst" is gone, matching seg6_input_core(). Fixes: 7a3f5b0de364 ("netfilter: add netfilter hooks to SRv6 data plane") Reported-by: Andrea Mayer Signed-off-by: Xiang Mei (Microsoft) --- v2: new patch, split out from the seg6local fix (Andrea Mayer). net/ipv6/seg6_iptunnel.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c index 4c45c0a77d75..60883dae5ae9 100644 --- a/net/ipv6/seg6_iptunnel.c +++ b/net/ipv6/seg6_iptunnel.c @@ -23,6 +23,7 @@ #include #include #include +#include #ifdef CONFIG_IPV6_SEG6_HMAC #include #endif @@ -65,6 +66,17 @@ seg6_encap_lwtunnel(struct lwtunnel_state *lwt) return seg6_lwt_lwtunnel(lwt)->tuninfo; } +static struct lwtunnel_state *seg6_lwtst_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) + return NULL; + + return dst->lwtstate; +} + static const struct nla_policy seg6_iptunnel_policy[SEG6_IPTUNNEL_MAX + 1] = { [SEG6_IPTUNNEL_SRH] = { .type = NLA_BINARY }, [SEG6_IPTUNNEL_SRC] = NLA_POLICY_EXACT_LEN(sizeof(struct in6_addr)), @@ -488,18 +500,21 @@ static int seg6_input_finish(struct net *net, struct sock *sk, static int seg6_input_core(struct net *net, struct sock *sk, struct sk_buff *skb) { - struct dst_entry *orig_dst = skb_dst(skb); struct dst_entry *dst = NULL; struct lwtunnel_state *lwtst; struct seg6_lwt *slwt; int err; - /* We cannot dereference "orig_dst" once ip6_route_input() or + /* We cannot dereference the incoming dst once ip6_route_input() or * skb_dst_drop() is called. However, in order to detect a dst loop, we * need the address of its lwtstate. So, save the address of lwtstate * now and use it later as a comparison. */ - lwtst = orig_dst->lwtstate; + lwtst = seg6_lwtst_from_skb(skb); + if (unlikely(!lwtst)) { + err = -EINVAL; + goto drop; + } slwt = seg6_lwt_lwtunnel(lwtst); @@ -581,12 +596,18 @@ static int seg6_input(struct sk_buff *skb) static int seg6_output_core(struct net *net, struct sock *sk, struct sk_buff *skb) { - struct dst_entry *orig_dst = skb_dst(skb); struct dst_entry *dst = NULL; + struct lwtunnel_state *lwtst; struct seg6_lwt *slwt; int err; - slwt = seg6_lwt_lwtunnel(orig_dst->lwtstate); + lwtst = seg6_lwtst_from_skb(skb); + if (unlikely(!lwtst)) { + err = -EINVAL; + goto drop; + } + + slwt = seg6_lwt_lwtunnel(lwtst); local_bh_disable(); dst = dst_cache_get(&slwt->cache_output); @@ -614,7 +635,7 @@ static int seg6_output_core(struct net *net, struct sock *sk, } /* cache only if we don't create a dst reference loop */ - if (orig_dst->lwtstate != dst->lwtstate) { + if (lwtst != dst->lwtstate) { local_bh_disable(); dst_cache_set_ip6(&slwt->cache_output, dst, &fl6.saddr); local_bh_enable(); -- 2.43.0