From 52e339d515be755fbdf9e49d68c214857385569e Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Thu, 13 Jun 2024 10:21:54 +0200 Subject: [PATCH] 6.6-stable patches added patches: net-fix-__dst_negative_advice-race.patch --- .../net-fix-__dst_negative_advice-race.patch | 195 ++++++++++++++++++ queue-6.6/series | 1 + 2 files changed, 196 insertions(+) create mode 100644 queue-6.6/net-fix-__dst_negative_advice-race.patch diff --git a/queue-6.6/net-fix-__dst_negative_advice-race.patch b/queue-6.6/net-fix-__dst_negative_advice-race.patch new file mode 100644 index 00000000000..295dc26492d --- /dev/null +++ b/queue-6.6/net-fix-__dst_negative_advice-race.patch @@ -0,0 +1,195 @@ +From 92f1655aa2b2294d0b49925f3b875a634bd3b59e Mon Sep 17 00:00:00 2001 +From: Eric Dumazet +Date: Tue, 28 May 2024 11:43:53 +0000 +Subject: net: fix __dst_negative_advice() race + +From: Eric Dumazet + +commit 92f1655aa2b2294d0b49925f3b875a634bd3b59e upstream. + +__dst_negative_advice() does not enforce proper RCU rules when +sk->dst_cache must be cleared, leading to possible UAF. + +RCU rules are that we must first clear sk->sk_dst_cache, +then call dst_release(old_dst). + +Note that sk_dst_reset(sk) is implementing this protocol correctly, +while __dst_negative_advice() uses the wrong order. + +Given that ip6_negative_advice() has special logic +against RTF_CACHE, this means each of the three ->negative_advice() +existing methods must perform the sk_dst_reset() themselves. + +Note the check against NULL dst is centralized in +__dst_negative_advice(), there is no need to duplicate +it in various callbacks. + +Many thanks to Clement Lecigne for tracking this issue. + +This old bug became visible after the blamed commit, using UDP sockets. + +Fixes: a87cb3e48ee8 ("net: Facility to report route quality of connected sockets") +Reported-by: Clement Lecigne +Diagnosed-by: Clement Lecigne +Signed-off-by: Eric Dumazet +Cc: Tom Herbert +Reviewed-by: David Ahern +Link: https://lore.kernel.org/r/20240528114353.1794151-1-edumazet@google.com +Signed-off-by: Jakub Kicinski +[Lee: Stable backport] +Signed-off-by: Lee Jones +Signed-off-by: Greg Kroah-Hartman +--- + include/net/dst_ops.h | 2 +- + include/net/sock.h | 13 +++---------- + net/ipv4/route.c | 22 ++++++++-------------- + net/ipv6/route.c | 29 +++++++++++++++-------------- + net/xfrm/xfrm_policy.c | 11 +++-------- + 5 files changed, 30 insertions(+), 47 deletions(-) + +--- a/include/net/dst_ops.h ++++ b/include/net/dst_ops.h +@@ -24,7 +24,7 @@ struct dst_ops { + void (*destroy)(struct dst_entry *); + void (*ifdown)(struct dst_entry *, + struct net_device *dev); +- struct dst_entry * (*negative_advice)(struct dst_entry *); ++ void (*negative_advice)(struct sock *sk, struct dst_entry *); + void (*link_failure)(struct sk_buff *); + void (*update_pmtu)(struct dst_entry *dst, struct sock *sk, + struct sk_buff *skb, u32 mtu, +--- a/include/net/sock.h ++++ b/include/net/sock.h +@@ -2183,17 +2183,10 @@ sk_dst_get(const struct sock *sk) + + static inline void __dst_negative_advice(struct sock *sk) + { +- struct dst_entry *ndst, *dst = __sk_dst_get(sk); ++ struct dst_entry *dst = __sk_dst_get(sk); + +- if (dst && dst->ops->negative_advice) { +- ndst = dst->ops->negative_advice(dst); +- +- if (ndst != dst) { +- rcu_assign_pointer(sk->sk_dst_cache, ndst); +- sk_tx_queue_clear(sk); +- WRITE_ONCE(sk->sk_dst_pending_confirm, 0); +- } +- } ++ if (dst && dst->ops->negative_advice) ++ dst->ops->negative_advice(sk, dst); + } + + static inline void dst_negative_advice(struct sock *sk) +--- a/net/ipv4/route.c ++++ b/net/ipv4/route.c +@@ -132,7 +132,8 @@ struct dst_entry *ipv4_dst_check(struct + static unsigned int ipv4_default_advmss(const struct dst_entry *dst); + INDIRECT_CALLABLE_SCOPE + unsigned int ipv4_mtu(const struct dst_entry *dst); +-static struct dst_entry *ipv4_negative_advice(struct dst_entry *dst); ++static void ipv4_negative_advice(struct sock *sk, ++ struct dst_entry *dst); + static void ipv4_link_failure(struct sk_buff *skb); + static void ip_rt_update_pmtu(struct dst_entry *dst, struct sock *sk, + struct sk_buff *skb, u32 mtu, +@@ -837,22 +838,15 @@ static void ip_do_redirect(struct dst_en + __ip_do_redirect(rt, skb, &fl4, true); + } + +-static struct dst_entry *ipv4_negative_advice(struct dst_entry *dst) ++static void ipv4_negative_advice(struct sock *sk, ++ struct dst_entry *dst) + { + struct rtable *rt = (struct rtable *)dst; +- struct dst_entry *ret = dst; + +- if (rt) { +- if (dst->obsolete > 0) { +- ip_rt_put(rt); +- ret = NULL; +- } else if ((rt->rt_flags & RTCF_REDIRECTED) || +- rt->dst.expires) { +- ip_rt_put(rt); +- ret = NULL; +- } +- } +- return ret; ++ if ((dst->obsolete > 0) || ++ (rt->rt_flags & RTCF_REDIRECTED) || ++ rt->dst.expires) ++ sk_dst_reset(sk); + } + + /* +--- a/net/ipv6/route.c ++++ b/net/ipv6/route.c +@@ -87,7 +87,8 @@ struct dst_entry *ip6_dst_check(struct d + static unsigned int ip6_default_advmss(const struct dst_entry *dst); + INDIRECT_CALLABLE_SCOPE + unsigned int ip6_mtu(const struct dst_entry *dst); +-static struct dst_entry *ip6_negative_advice(struct dst_entry *); ++static void ip6_negative_advice(struct sock *sk, ++ struct dst_entry *dst); + static void ip6_dst_destroy(struct dst_entry *); + static void ip6_dst_ifdown(struct dst_entry *, + struct net_device *dev); +@@ -2760,24 +2761,24 @@ INDIRECT_CALLABLE_SCOPE struct dst_entry + } + EXPORT_INDIRECT_CALLABLE(ip6_dst_check); + +-static struct dst_entry *ip6_negative_advice(struct dst_entry *dst) ++static void ip6_negative_advice(struct sock *sk, ++ struct dst_entry *dst) + { + struct rt6_info *rt = (struct rt6_info *) dst; + +- if (rt) { +- if (rt->rt6i_flags & RTF_CACHE) { +- rcu_read_lock(); +- if (rt6_check_expired(rt)) { +- rt6_remove_exception_rt(rt); +- dst = NULL; +- } +- rcu_read_unlock(); +- } else { +- dst_release(dst); +- dst = NULL; ++ if (rt->rt6i_flags & RTF_CACHE) { ++ rcu_read_lock(); ++ if (rt6_check_expired(rt)) { ++ /* counteract the dst_release() in sk_dst_reset() */ ++ dst_hold(dst); ++ sk_dst_reset(sk); ++ ++ rt6_remove_exception_rt(rt); + } ++ rcu_read_unlock(); ++ return; + } +- return dst; ++ sk_dst_reset(sk); + } + + static void ip6_link_failure(struct sk_buff *skb) +--- a/net/xfrm/xfrm_policy.c ++++ b/net/xfrm/xfrm_policy.c +@@ -3853,15 +3853,10 @@ static void xfrm_link_failure(struct sk_ + /* Impossible. Such dst must be popped before reaches point of failure. */ + } + +-static struct dst_entry *xfrm_negative_advice(struct dst_entry *dst) ++static void xfrm_negative_advice(struct sock *sk, struct dst_entry *dst) + { +- if (dst) { +- if (dst->obsolete) { +- dst_release(dst); +- dst = NULL; +- } +- } +- return dst; ++ if (dst->obsolete) ++ sk_dst_reset(sk); + } + + static void xfrm_init_pmtu(struct xfrm_dst **bundle, int nr) diff --git a/queue-6.6/series b/queue-6.6/series index 586d8cda96a..48bb09cd6e9 100644 --- a/queue-6.6/series +++ b/queue-6.6/series @@ -100,3 +100,4 @@ kdb-use-format-strings-rather-than-0-injection-in-kdb_read.patch kdb-fix-console-handling-when-editing-and-tab-completing-commands.patch kdb-merge-identical-case-statements-in-kdb_read.patch kdb-use-format-specifiers-rather-than-memset-for-padding-in-kdb_read.patch +net-fix-__dst_negative_advice-race.patch -- 2.47.3