netdev - Re: [RFC PATCH] net: flush 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: <CANn89iKg4p+ZgW36mKf-843QGydw0g_jxvti86QJOoxCyB0A8A@mail.gmail.com>
Date: Mon, 20 May 2024 15:54:55 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>, 
	Jakub Kicinski <kuba@...nel.org>, David Ahern <dsahern@...nel.org>, 
	Sabrina Dubroca <sd@...asysnail.net>
Subject: Re: [RFC PATCH] net: flush dst_cache on device removal

On Mon, May 20, 2024 at 1:00 PM Paolo Abeni <pabeni@...hat.com> wrote:
>
> Eric reported that dst_cache don't cope correctly with device removal,
> keeping the cached dst unmodified even when the underlining device is
> deleted and the dst itself is not uncached.
>
> The above causes the infamous 'unregistering netdevice' hangup.
>
> Address the issue implementing explicit book-keeping of all the
> initialized dst_caches. At network device unregistration time, traverse
> them, looking for relevant dst and eventually replace the dst reference
> with a blackhole one.
>
> Use an xarray to store the dst_cache references, to avoid blocking the
> BH during the traversal for a possibly unbounded time.
>
> Reported-by: Eric Dumazet <edumazet@...gle.com>
> Fixes: 911362c70df5 ("net: add dst_cache support")
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> ---
> I can't reproduce the issue locally, I hope somebody able to observe it
> could step-in and give this patch a shot.
> ---

H Paolo, thanks for your patch.

It seems dst_cache_netdev_event() could spend an awful amount of cpu
in complex setups.

I wonder if we could instead reuse the existing uncached_list
mechanism we have already ?

BTW it seems we could get rid of the ul->quarantine lists.

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index bbc2a0dd931429e7f8c68df0df48bce6d604fb56..964dd7cc37ca8c53ebbd1306adb47772c62a7d36
100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -130,7 +130,6 @@ static struct fib6_info *rt6_get_route_info(struct net *net,
 struct uncached_list {
        spinlock_t              lock;
        struct list_head        head;
-       struct list_head        quarantine;
 };

 static DEFINE_PER_CPU_ALIGNED(struct uncached_list, rt6_uncached_list);
@@ -188,8 +187,7 @@ static void rt6_uncached_list_flush_dev(struct
net_device *dev)
                                handled = true;
                        }
                        if (handled)
-                               list_move(&rt->dst.rt_uncached,
-                                         &ul->quarantine);
+                               list_del_init(&rt->dst.rt_uncached);
                }
                spin_unlock_bh(&ul->lock);
        }
@@ -6754,7 +6752,6 @@ int __init ip6_route_init(void)
                struct uncached_list *ul = per_cpu_ptr(&rt6_uncached_list, cpu);

                INIT_LIST_HEAD(&ul->head);
-               INIT_LIST_HEAD(&ul->quarantine);
                spin_lock_init(&ul->lock);
        }