netdev - Re: [PATCH net-next 0/3] dst_cache: cope with device removal

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]

Date: Thu, 30 May 2024 19:38:39 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>, 
	David Ahern <dsahern@...nel.org>, Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH net-next 0/3] dst_cache: cope with device removal

On Thu, May 30, 2024 at 7:21 PM Paolo Abeni <pabeni@...hat.com> wrote:
>
> Eric reported a net device refcount leak and diagnosed the root cause
> as the dst_cache not coping well with the underlying device removal.
>
> To address such issue, this series introduces the infrastructure to let
> the existing uncached list handle the relevant cleanup.
>
> Patch 1 and 2 are preparation changes to make the uncached list infra
> more flexible for the new use-case, and patch 3 addresses the issue.
>
> ---
> Targeting net-next as the addressed problem is quite ancient and I fear
> some unexpected side effects for patch 2.

Thanks Paolo, I am going to test this ASAP.

BTW I found suspect dst_cache uses from lwtunnel in the output path.
AFAIK lwtunnel_output() does not block BH.
Either we change lwtunnel_output() or replace some of the ->output
methods to use local_bh_disable() ?

If BH is already held, I do not think
preempt_disable()/preempt_enable(); pairs are necessary.

diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c
index 7563f8c6aa87cf9f7841ee78dcea2a16f60ac344..bf7120ecea1ebe834e70073710be0c1692d7ad1d
100644
--- a/net/ipv6/ioam6_iptunnel.c
+++ b/net/ipv6/ioam6_iptunnel.c
@@ -351,9 +351,9 @@ static int ioam6_output(struct net *net, struct
sock *sk, struct sk_buff *skb)
                goto drop;

        if (!ipv6_addr_equal(&orig_daddr, &ipv6_hdr(skb)->daddr)) {
-               preempt_disable();
+               local_bh_disable();
                dst = dst_cache_get(&ilwt->cache);
-               preempt_enable();
+               local_bh_enable();

                if (unlikely(!dst)) {
                        struct ipv6hdr *hdr = ipv6_hdr(skb);
@@ -373,9 +373,9 @@ static int ioam6_output(struct net *net, struct
sock *sk, struct sk_buff *skb)
                                goto drop;
                        }

-                       preempt_disable();
+                       local_bh_disable();
                        dst_cache_set_ip6(&ilwt->cache, dst, &fl6.saddr);
-                       preempt_enable();
+                       local_bh_enable();
                }

                skb_dst_drop(skb);