netdev - Re: [PATCH net-next] tcp: use 2-arg optimal variant of kfree

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: <Y4qPJ89SBWACbbTr@google.com>
Date:   Fri, 2 Dec 2022 23:49:59 +0000
From:   Joel Fernandes <joel@...lfernandes.org>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
        eric.dumazet@...il.com, Dmitry Safonov <dima@...sta.com>,
        "Paul E . McKenney" <paulmck@...nel.org>
Subject: Re: [PATCH net-next] tcp: use 2-arg optimal variant of kfree_rcu()

On Fri, Dec 02, 2022 at 05:28:47AM +0000, Eric Dumazet wrote:
> kfree_rcu(1-arg) should be avoided as much as possible,
> since this is only possible from sleepable contexts,
> and incurr extra rcu barriers.
> 
> I wish the 1-arg variant of kfree_rcu() would
> get a distinct name, like kfree_rcu_slow()
> to avoid it being abused.

Hi Eric,
Nice to see your patch.

Paul, all, regarding Eric's concern, would the following work to warn of
users? Credit to Paul/others for discussing the idea on another thread. One
thing to note here is, this debugging will only be in effect on preemptible
kernels, but should still help catch issues hopefully.

The other idea Paul mentioned is to introduce a new dedicated API for 1-arg
sleepable cases. My concern with that was that, that being effective depends
on the user using the right API in the first place.

I did not test it yet, but wanted to discuss a bit first.

Cheers,

- Joel

---8<-----------------------

diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index 9bc025aa79a3..112d230279ea 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -106,6 +106,11 @@ static inline void __kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
 	}
 
 	// kvfree_rcu(one_arg) call.
+	if (IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible() && !head) {
+		WARN_ONCE(1, "%s(): Please provide an rcu_head in preemptible"
+			  " contexts to avoid long waits!\n", __func__);
+	}
+
 	might_sleep();
 	synchronize_rcu();
 	kvfree((void *) func);
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 0ca21ac0f064..b29df1305a2e 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -3324,6 +3324,11 @@ void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
 		 * only. For other places please embed an rcu_head to
 		 * your data.
 		 */
+		if (IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible() && !head) {
+			WARN_ONCE(1, "%s(): Please provide an rcu_head in preemptible"
+				  " contexts to avoid long waits!\n", __func__);
+		}
+
 		might_sleep();
 		ptr = (unsigned long *) func;
 	}