On Thu, Jun 13, 2024 at 09:12:44AM +0100, Lee Jones wrote:
> From: Eric Dumazet <edumazet@xxxxxxxxxx>
>
> __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 <clecigne@xxxxxxxxxx>
> Diagnosed-by: Clement Lecigne <clecigne@xxxxxxxxxx>
> Signed-off-by: Eric Dumazet <edumazet@xxxxxxxxxx>
> Cc: Tom Herbert <tom@xxxxxxxxxxxxxxx>
> Reviewed-by: David Ahern <dsahern@xxxxxxxxxx>
> Link: https://lore.kernel.org/r/20240528114353.1794151-1-edumazet@xxxxxxxxxx
> Signed-off-by: Jakub Kicinski <kuba@xxxxxxxxxx>
> (cherry picked from commit 92f1655aa2b2294d0b49925f3b875a634bd3b59e)
> [Lee: Stable backport]
> Signed-off-by: Lee Jones <lee@xxxxxxxxxx>
> ---
> 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(-)
All now queued up, thanks!
greg k-h
