netdev - Re: [PATCH net] net: decrease cached dst counters in dst

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <89dcde93-8e5a-4193-aa01-fde5dd5ee1fd@redhat.com>
Date: Tue, 1 Apr 2025 10:00:56 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Antoine Tenart <atenart@...nel.org>, davem@...emloft.net,
 kuba@...nel.org, edumazet@...gle.com
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net] net: decrease cached dst counters in dst_release



On 3/26/25 6:36 PM, Antoine Tenart wrote:
> Upstream fix ac888d58869b ("net: do not delay dst_entries_add() in
> dst_release()") moved decrementing the dst count from dst_destroy to
> dst_release to avoid accessing already freed data in case of netns
> dismantle. However in case CONFIG_DST_CACHE is enabled and OvS+tunnels
> are used, this fix is incomplete as the same issue will be seen for
> cached dsts:
> 
>   Unable to handle kernel paging request at virtual address ffff5aabf6b5c000
>   Call trace:
>    percpu_counter_add_batch+0x3c/0x160 (P)
>    dst_release+0xec/0x108
>    dst_cache_destroy+0x68/0xd8
>    dst_destroy+0x13c/0x168
>    dst_destroy_rcu+0x1c/0xb0
>    rcu_do_batch+0x18c/0x7d0
>    rcu_core+0x174/0x378
>    rcu_core_si+0x18/0x30
> 
> Fix this by invalidating the cache, and thus decrementing cached dst
> counters, in dst_release too.
> 
> Fixes: d71785ffc7e7 ("net: add dst_cache to ovs vxlan lwtunnel")
> Signed-off-by: Antoine Tenart <atenart@...nel.org>


> ---
>  net/core/dst.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/net/core/dst.c b/net/core/dst.c
> index 9552a90d4772..6d76b799ce64 100644
> --- a/net/core/dst.c
> +++ b/net/core/dst.c
> @@ -165,6 +165,14 @@ static void dst_count_dec(struct dst_entry *dst)
>  void dst_release(struct dst_entry *dst)
>  {
>  	if (dst && rcuref_put(&dst->__rcuref)) {
> +#ifdef CONFIG_DST_CACHE
> +		if (dst->flags & DST_METADATA) {
> +			struct metadata_dst *md_dst = (struct metadata_dst *)dst;
> +
> +			if (md_dst->type == METADATA_IP_TUNNEL)
> +				dst_cache_reset_now(&md_dst->u.tun_info.dst_cache);

I think the fix is correct, but I'm wondering if we have a similar issue
for the METADATA_XFRM meta-dst. Isn't:

	dst_release(md_dst->u.xfrm_info.dst_orig);

in metadata_dst_free() going to cause the same UaF? Why don't we need to
clean such dst here, too?

Thanks!

Paolo