[PATCH 2/4] ovpn: cap number of peers per multi-peer interface

Antonio Quartulli <[email protected]>
Newsgroups gmane.network.openvpn.devel
Message-ID <[email protected]>
From: Antonio Quartulli <[email protected]>

Memcg accounting bounds user-driven peer creation only when the
calling cgroup has memory.max set; on unconstrained hosts it isn't
a hard limit.

Track the live peer count in ovpn_peer_collection::n_peers under the
existing ovpn->lock and reject OVPN_CMD_PEER_NEW with -ENOSPC once
OVPN_MAX_PEERS (65535) is reached. 65535 matches what userspace
OpenVPN servers configure via --max-clients. P2P mode is unaffected.

The same cap also bounds the unaccounted RX-float bind allocations,
since binds are owned by peers.

Signed-off-by: Antonio Quartulli <[email protected]>
---
 drivers/net/ovpn/ovpnpriv.h | 3 +++
 drivers/net/ovpn/peer.c     | 9 +++++++++
 drivers/net/ovpn/peer.h     | 8 ++++++++
 3 files changed, 20 insertions(+)

diff --git a/drivers/net/ovpn/ovpnpriv.h b/drivers/net/ovpn/ovpnpriv.h
index 5898f6adada7..113f5f493575 100644
--- a/drivers/net/ovpn/ovpnpriv.h
+++ b/drivers/net/ovpn/ovpnpriv.h
@@ -24,12 +24,15 @@
  *		  rehashed on the fly due to peer IP change)
  * @by_transp_addr: table of peers indexed by transport address (items can be
  *		    rehashed on the fly due to peer IP change)
+ * @n_peers: number of peers currently in the collection, protected by
+ *	     ovpn_priv->lock
  */
 struct ovpn_peer_collection {
 	DECLARE_HASHTABLE(by_id, 12);
 	struct hlist_nulls_head by_vpn_addr4[1 << 12];
 	struct hlist_nulls_head by_vpn_addr6[1 << 12];
 	struct hlist_nulls_head by_transp_addr[1 << 12];
+	u32 n_peers;
 };
 
 /**
diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c
index 511a7ce9b32b..1d564888479a 100644
--- a/drivers/net/ovpn/peer.c
+++ b/drivers/net/ovpn/peer.c
@@ -286,6 +286,8 @@ void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb)
 	/* RX float path runs in softirq context: __GFP_ACCOUNT would charge
 	 * whatever cgroup is on-CPU when the packet arrives, not the userns
 	 * owner, so pass plain GFP_ATOMIC and skip accounting on this path.
+	 * The number of bind objects that can accumulate via float events is
+	 * bounded by the per-MP peer cap, since binds are owned by peers.
 	 */
 	if (unlikely(ovpn_peer_reset_sockaddr(peer,
 					      (struct sockaddr_storage *)&ss,
@@ -703,6 +705,7 @@ static void ovpn_peer_remove(struct ovpn_peer *peer,
 		hlist_nulls_del_init_rcu(&peer->hash_entry_addr4);
 		hlist_nulls_del_init_rcu(&peer->hash_entry_addr6);
 		hlist_nulls_del_init_rcu(&peer->hash_entry_transp_addr);
+		peer->ovpn->peers->n_peers--;
 		break;
 	case OVPN_MODE_P2P:
 		/* prevent double remove */
@@ -959,6 +962,11 @@ static int ovpn_peer_add_mp(struct ovpn_priv *ovpn, struct ovpn_peer *peer)
 		goto out;
 	}
 
+	if (ovpn->peers->n_peers >= OVPN_MAX_PEERS) {
+		ret = -ENOSPC;
+		goto out;
+	}
+
 	bind = rcu_dereference_protected(peer->bind, true);
 	/* peers connected via TCP have bind == NULL */
 	if (bind) {
@@ -994,6 +1002,7 @@ static int ovpn_peer_add_mp(struct ovpn_priv *ovpn, struct ovpn_peer *peer)
 					      sizeof(peer->id)));
 
 	ovpn_peer_hash_vpn_ip(peer);
+	ovpn->peers->n_peers++;
 out:
 	spin_unlock_bh(&ovpn->lock);
 	return ret;
diff --git a/drivers/net/ovpn/peer.h b/drivers/net/ovpn/peer.h
index 1bfc66821739..2efc782130f3 100644
--- a/drivers/net/ovpn/peer.h
+++ b/drivers/net/ovpn/peer.h
@@ -17,6 +17,14 @@
 #include "socket.h"
 #include "stats.h"
 
+/* Hard cap on number of peers per MP-mode interface. Caps the worst-case
+ * kernel memory an unprivileged userns owner driving OVPN_CMD_PEER_NEW can
+ * pin even when memcg accounting is unconstrained. 65535 matches what
+ * mainstream userspace OpenVPN servers configure via --max-clients, so
+ * legitimate deployments never hit this.
+ */
+#define OVPN_MAX_PEERS 65535
+
 /**
  * struct ovpn_peer - the main remote peer object
  * @ovpn: main openvpn instance this peer belongs to
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.