| 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
| ||
|
Message-Id: <20240819145208.3209296-1-jchapman@katalix.com>
Date: Mon, 19 Aug 2024 15:52:08 +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] l2tp: avoid using drain_workqueue in l2tp_pre_exit_net
Recent commit c1b2e36b8776 ("l2tp: flush workqueue before draining
it") 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
while drain_workqueue runs.
Instead of using drain_workqueue, use a loop to delete all tunnels and
__flush_workqueue until all tunnel/session lists of the net are
empty. Add a per-net flag to ensure that no new tunnel can be created
in the net once l2tp_pre_exit_net starts.
Signed-off-by: James Chapman <jchapman@...alix.com>
Signed-off-by: Tom Parkin <tparkin@...alix.com>
---
net/l2tp/l2tp_core.c | 38 +++++++++++++++++++++++++++++---------
net/l2tp/l2tp_core.h | 2 +-
net/l2tp/l2tp_netlink.c | 2 +-
net/l2tp/l2tp_ppp.c | 3 ++-
4 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index af87c781d6a6..246b07342b86 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -107,6 +107,7 @@ static struct workqueue_struct *l2tp_wq;
/* per-net private data for this module */
static unsigned int l2tp_net_id;
struct l2tp_net {
+ bool net_closing;
/* Lock for write access to l2tp_tunnel_idr */
spinlock_t l2tp_tunnel_idr_lock;
struct idr l2tp_tunnel_idr;
@@ -1560,13 +1561,19 @@ static int l2tp_tunnel_sock_create(struct net *net,
return err;
}
-int l2tp_tunnel_create(int fd, int version, u32 tunnel_id, u32 peer_tunnel_id,
+int l2tp_tunnel_create(struct net *net, int fd, int version,
+ u32 tunnel_id, u32 peer_tunnel_id,
struct l2tp_tunnel_cfg *cfg, struct l2tp_tunnel **tunnelp)
{
+ struct l2tp_net *pn = l2tp_pernet(net);
struct l2tp_tunnel *tunnel = NULL;
int err;
enum l2tp_encap_type encap = L2TP_ENCAPTYPE_UDP;
+ /* This pairs with WRITE_ONCE() in l2tp_pre_exit_net(). */
+ if (READ_ONCE(pn->net_closing))
+ return -ENETDOWN;
+
if (cfg)
encap = cfg->encap;
@@ -1832,6 +1839,8 @@ static __net_init int l2tp_init_net(struct net *net)
{
struct l2tp_net *pn = net_generic(net, l2tp_net_id);
+ pn->net_closing = false;
+
idr_init(&pn->l2tp_tunnel_idr);
spin_lock_init(&pn->l2tp_tunnel_idr_lock);
@@ -1848,6 +1857,12 @@ static __net_exit void l2tp_pre_exit_net(struct net *net)
struct l2tp_tunnel *tunnel = NULL;
unsigned long tunnel_id, tmp;
+ /* Prevent new tunnel create API requests in the net.
+ * Pairs with READ_ONCE in l2tp_tunnel_create.
+ */
+ WRITE_ONCE(pn->net_closing, true);
+
+again:
rcu_read_lock_bh();
idr_for_each_entry_ul(&pn->l2tp_tunnel_idr, tunnel, tmp, tunnel_id) {
if (tunnel)
@@ -1855,16 +1870,21 @@ 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.
- */
+ if (l2tp_wq)
__flush_workqueue(l2tp_wq);
- drain_workqueue(l2tp_wq);
+
+ /* repeat until all of the net's IDR lists are empty, in case tunnels
+ * or sessions were being created just before l2tp_pre_exit_net was
+ * called.
+ */
+ rcu_read_lock_bh();
+ if (!idr_is_empty(&pn->l2tp_tunnel_idr) ||
+ !idr_is_empty(&pn->l2tp_v2_session_idr) ||
+ !idr_is_empty(&pn->l2tp_v3_session_idr)) {
+ rcu_read_unlock_bh();
+ goto again;
}
+ rcu_read_unlock_bh();
}
static __net_exit void l2tp_exit_net(struct net *net)
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index ffd8ced3a51f..a765123e213d 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -232,7 +232,7 @@ struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net,
* Creation of a new instance is a two-step process: create, then register.
* Destruction is triggered using the *_delete functions, and completes asynchronously.
*/
-int l2tp_tunnel_create(int fd, int version, u32 tunnel_id,
+int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id,
u32 peer_tunnel_id, struct l2tp_tunnel_cfg *cfg,
struct l2tp_tunnel **tunnelp);
int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net,
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 284f1dec1b56..cd410144b42e 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -234,7 +234,7 @@ static int l2tp_nl_cmd_tunnel_create(struct sk_buff *skb, struct genl_info *info
switch (cfg.encap) {
case L2TP_ENCAPTYPE_UDP:
case L2TP_ENCAPTYPE_IP:
- ret = l2tp_tunnel_create(fd, proto_version, tunnel_id,
+ ret = l2tp_tunnel_create(net, fd, proto_version, tunnel_id,
peer_tunnel_id, &cfg, &tunnel);
break;
}
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 53baf2dd5d5d..2c083ef2e4ee 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -660,7 +660,8 @@ static struct l2tp_tunnel *pppol2tp_tunnel_get(struct net *net,
if (info->fd < 0)
return ERR_PTR(-EBADF);
- error = l2tp_tunnel_create(info->fd,
+ error = l2tp_tunnel_create(net,
+ info->fd,
info->version,
info->tunnel_id,
info->peer_tunnel_id, &tcfg,
--
2.34.1
Powered by blists - more mailing lists