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