netdev - [PATCH net-next v2] l2tp: avoid using drain_workqueue in l2tp_pre_exit

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

Message-Id: <20240823142257.692667-1-jchapman@katalix.com>
Date: Fri, 23 Aug 2024 15:22:57 +0100
From: James Chapman <jchapman@...alix.com>
To: netdev@...r.kernel.org
Cc: davem@...emloft.net,
	edumazet@...gle.com,
	kuba@...nel.org,
	pabeni@...hat.com,
	dsahern@...nel.org,
	tparkin@...alix.com,
	xiyou.wangcong@...il.com
Subject: [PATCH net-next v2] l2tp: avoid using drain_workqueue in l2tp_pre_exit_net

Recent commit fc7ec7f554d7 ("l2tp: delete sessions using work queue")
incorrectly uses drain_workqueue. The use of drain_workqueue in
l2tp_pre_exit_net is flawed because the workqueue is shared by all
nets and it is therefore possible for new work items to be queued
for other nets while drain_workqueue runs.

Instead of using drain_workqueue, use __flush_workqueue twice. The
first one will run all tunnel delete work items and any work already
queued. When tunnel delete work items are run, they may queue
new session delete work items, which the second __flush_workqueue will
run.

In l2tp_exit_net, warn if any of the net's idr lists are not empty.

Fixes: fc7ec7f554d7 ("l2tp: delete sessions using work queue")
Signed-off-by: James Chapman <jchapman@...alix.com>
---
 v2:
   - remove unneeded per-net net_closing flag (paolo)
   - remove loop waiting for idr lists to be empty (paolo)
   - add correct fixes tag (paolo)
   - in l2tp_exit_net warn if idr lists not empty
 v1: https://lore.kernel.org/netdev/20240819145208.3209296-1-jchapman@katalix.com/
---
 net/l2tp/l2tp_core.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index af87c781d6a6..e5e492284997 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1856,14 +1856,14 @@ static __net_exit void l2tp_pre_exit_net(struct net *net)
 	rcu_read_unlock_bh();
 
 	if (l2tp_wq) {
-		/* ensure that all TUNNEL_DELETE work items are run before
-		 * draining the work queue since TUNNEL_DELETE requests may
-		 * queue SESSION_DELETE work items for each session in the
-		 * tunnel. drain_workqueue may otherwise warn if SESSION_DELETE
-		 * requests are queued while the work queue is being drained.
+		/* Run all TUNNEL_DELETE work items just queued. */
+		__flush_workqueue(l2tp_wq);
+
+		/* Each TUNNEL_DELETE work item will queue a SESSION_DELETE
+		 * work item for each session in the tunnel. Flush the
+		 * workqueue again to process these.
 		 */
 		__flush_workqueue(l2tp_wq);
-		drain_workqueue(l2tp_wq);
 	}
 }
 
@@ -1871,8 +1871,11 @@ static __net_exit void l2tp_exit_net(struct net *net)
 {
 	struct l2tp_net *pn = l2tp_pernet(net);
 
+	WARN_ON_ONCE(!idr_is_empty(&pn->l2tp_v2_session_idr));
 	idr_destroy(&pn->l2tp_v2_session_idr);
+	WARN_ON_ONCE(!idr_is_empty(&pn->l2tp_v3_session_idr));
 	idr_destroy(&pn->l2tp_v3_session_idr);
+	WARN_ON_ONCE(!idr_is_empty(&pn->l2tp_tunnel_idr));
 	idr_destroy(&pn->l2tp_tunnel_idr);
 }
 
-- 
2.34.1