netdev - Re: [PATCH net] net: fix __dst_negative

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240612155235.GA2187093@google.com>
Date: Wed, 12 Jun 2024 16:52:35 +0100
From: Lee Jones <lee@...nel.org>
To: Eric Dumazet <edumazet@...gle.com>, stable@...r.kernel.org
Cc: "David S . Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	David Ahern <dsahern@...nel.org>, netdev@...r.kernel.org,
	eric.dumazet@...il.com, Clement Lecigne <clecigne@...gle.com>,
	Tom Herbert <tom@...bertland.com>
Subject: Re: [PATCH net] net: fix __dst_negative_advice() race

On Tue, 28 May 2024, Eric Dumazet wrote:

> __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@...gle.com>
> Diagnosed-by: Clement Lecigne <clecigne@...gle.com>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Cc: Tom Herbert <tom@...bertland.com>
> ---
>  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(-)

Could we have this patch in all Stable branches please?

Upstream commit:

  Fixes: 92f1655aa2b2 ("net: fix __dst_negative_advice() race")

-- 
Lee Jones [李琼斯]