* [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg [not found] <20261105140815.92841-1-alex@badcode.im> @ 2026-11-05 14:08 ` Alexander Ildiyarov 2026-11-10 16:52 ` Jason A. Donenfeld 2026-11-05 14:08 ` [PATCH net 2/2] wireguard: noise: recheck replay and flood under write lock on initiation Alexander Ildiyarov 1 sibling, 1 reply; 10+ messages in thread From: Alexander Ildiyarov @ 2026-11-05 14:08 UTC (permalink / raw) To: alexvancasper; +Cc: Alexander Ildiyarov, stable Peers, allowed IP trie nodes, the per-peer dst_cache and the per-device hashtables are all allocated in the context of the process that configures the device over netlink, but none of these allocations are charged to that process's memory cgroup: the wg_peer and allowedips_node caches are created without SLAB_ACCOUNT, and the hashtables and dst_cache use plain GFP_KERNEL. The amount of memory is bounded only by MAX_PEERS_PER_DEVICE (2^20) per device, with no limit on the number of devices or on the number of allowed IPs per peer. Since WG_CMD_SET_DEVICE uses GENL_UNS_ADMIN_PERM, any user who can create a user and network namespace can do this, and thereby escape the memory limits of the cgroup it runs in. For example, a process in a cgroup with memory.max=64M that adds 100000 peers with 20 /128 allowed IPs each grows Slab by ~790 MiB, yet the cgroup's memory.current stays below 1 MiB and its memory limit is never hit. Fix this by creating the two caches with SLAB_ACCOUNT and using GFP_KERNEL_ACCOUNT for the hashtables and the dst_cache. The keypair allocations are left alone, as they happen from the handshake worker and are bounded to three per peer. Fixes: e7096c131e51 ("net: WireGuard secure network tunnel") Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Alexander Ildiyarov <alex@badcode.im> --- drivers/net/wireguard/allowedips.c | 2 +- drivers/net/wireguard/peer.c | 4 ++-- drivers/net/wireguard/peerlookup.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/wireguard/allowedips.c b/drivers/net/wireguard/allowedips.c index 5ece9acad64d..2f2b8a94c8a7 100644 --- a/drivers/net/wireguard/allowedips.c +++ b/drivers/net/wireguard/allowedips.c @@ -411,7 +411,7 @@ struct wg_peer *wg_allowedips_lookup_src(struct allowedips *table, int __init wg_allowedips_slab_init(void) { - node_cache = KMEM_CACHE(allowedips_node, 0); + node_cache = KMEM_CACHE(allowedips_node, SLAB_ACCOUNT); return node_cache ? 0 : -ENOMEM; } diff --git a/drivers/net/wireguard/peer.c b/drivers/net/wireguard/peer.c index 1cb502a932e0..05b80521c706 100644 --- a/drivers/net/wireguard/peer.c +++ b/drivers/net/wireguard/peer.c @@ -33,7 +33,7 @@ struct wg_peer *wg_peer_create(struct wg_device *wg, peer = kmem_cache_zalloc(peer_cache, GFP_KERNEL); if (unlikely(!peer)) return ERR_PTR(ret); - if (unlikely(dst_cache_init(&peer->endpoint_cache, GFP_KERNEL))) + if (unlikely(dst_cache_init(&peer->endpoint_cache, GFP_KERNEL_ACCOUNT))) goto err; peer->device = wg; @@ -229,7 +229,7 @@ void wg_peer_put(struct wg_peer *peer) int __init wg_peer_init(void) { - peer_cache = KMEM_CACHE(wg_peer, 0); + peer_cache = KMEM_CACHE(wg_peer, SLAB_ACCOUNT); return peer_cache ? 0 : -ENOMEM; } diff --git a/drivers/net/wireguard/peerlookup.c b/drivers/net/wireguard/peerlookup.c index 99651a86ac95..f333e906014a 100644 --- a/drivers/net/wireguard/peerlookup.c +++ b/drivers/net/wireguard/peerlookup.c @@ -21,7 +21,7 @@ static struct hlist_head *pubkey_bucket(struct pubkey_hashtable *table, struct pubkey_hashtable *wg_pubkey_hashtable_alloc(void) { - struct pubkey_hashtable *table = kvmalloc_obj(*table); + struct pubkey_hashtable *table = kvmalloc_obj(*table, GFP_KERNEL_ACCOUNT); if (!table) return NULL; @@ -82,7 +82,7 @@ static struct hlist_head *index_bucket(struct index_hashtable *table, struct index_hashtable *wg_index_hashtable_alloc(void) { - struct index_hashtable *table = kvmalloc_obj(*table); + struct index_hashtable *table = kvmalloc_obj(*table, GFP_KERNEL_ACCOUNT); if (!table) return NULL; -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg 2026-11-05 14:08 ` [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg Alexander Ildiyarov @ 2026-11-10 16:52 ` Jason A. Donenfeld 0 siblings, 0 replies; 10+ messages in thread From: Jason A. Donenfeld @ 2026-11-10 16:52 UTC (permalink / raw) To: Alexander Ildiyarov; +Cc: alexvancasper, stable On Thu, Nov 05, 2026 at 05:08:14PM +0300, Alexander Ildiyarov wrote: > Peers, allowed IP trie nodes, the per-peer dst_cache and the per-device > hashtables are all allocated in the context of the process that > configures the device over netlink, but none of these allocations are > charged to that process's memory cgroup: the wg_peer and allowedips_node > caches are created without SLAB_ACCOUNT, and the hashtables and > dst_cache use plain GFP_KERNEL. > > The amount of memory is bounded only by MAX_PEERS_PER_DEVICE (2^20) per > device, with no limit on the number of devices or on the number of > allowed IPs per peer. Since WG_CMD_SET_DEVICE uses GENL_UNS_ADMIN_PERM, > any user who can create a user and network namespace can do this, and > thereby escape the memory limits of the cgroup it runs in. > > For example, a process in a cgroup with memory.max=64M that adds 100000 > peers with 20 /128 allowed IPs each grows Slab by ~790 MiB, yet the > cgroup's memory.current stays below 1 MiB and its memory limit is never > hit. > > Fix this by creating the two caches with SLAB_ACCOUNT and using > GFP_KERNEL_ACCOUNT for the hashtables and the dst_cache. The keypair > allocations are left alone, as they happen from the handshake worker and > are bounded to three per peer. I wasn't aware of this trick, I think mostly because most drivers don't use this. Does any other part of the networking stack do this? Is this a normal thing? ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net 2/2] wireguard: noise: recheck replay and flood under write lock on initiation [not found] <20261105140815.92841-1-alex@badcode.im> 2026-11-05 14:08 ` [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg Alexander Ildiyarov @ 2026-11-05 14:08 ` Alexander Ildiyarov 2026-11-10 16:51 ` Jason A. Donenfeld 1 sibling, 1 reply; 10+ messages in thread From: Alexander Ildiyarov @ 2026-11-05 14:08 UTC (permalink / raw) To: alexvancasper; +Cc: Alexander Ildiyarov, stable wg_noise_handshake_consume_initiation() checks the initiation's timestamp against latest_timestamp, and last_initiation_consumption against the current time, while holding handshake->lock for reading. It then drops the lock and takes it again for writing to store the new state, without repeating the checks. Incoming handshake packets are spread round-robin over all online CPUs, so two copies of the same initiation, received back to back, are processed in parallel. Both copies can pass the checks before either of them updates latest_timestamp and last_initiation_consumption, so the replay and flood protection is bypassed. The responder then creates two responses, each with a new ephemeral key, and installs the keypair of the last one, while the initiator accepts whichever response arrives first. When those differ, the session is unusable until the initiator gives up on it and starts a new handshake, and an attacker who can see and duplicate initiations can repeat this for every handshake. The window is narrow, since the expensive DH computations happen before the check, so it is between up_read() and down_write() only. With netem duplicating every initiation between two namespaces, no double response was observed in 200 handshakes on a 4-CPU VM. With a usleep_range(1000, 2000) inserted before down_write() to widen the window, 30 of 50 duplicated initiations produced two responses, and in 12 of 50 cases the session was unusable after the handshake. With this patch applied, and the same delay, every duplicate was rejected and all 50 sessions worked. Fix this by doing the checks again once the write lock is held, keeping the read-locked check as a fast path for rejecting replays. This is the initiation-side counterpart of the response-side race fixed by: commit 2a73c9b2c0ca ("wireguard: noise: reject response consumption after intermediate initiation") Fixes: e7096c131e51 ("net: WireGuard secure network tunnel") Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Alexander Ildiyarov <alex@badcode.im> --- drivers/net/wireguard/noise.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireguard/noise.c b/drivers/net/wireguard/noise.c index 428d3afe3ce4..209127654e65 100644 --- a/drivers/net/wireguard/noise.c +++ b/drivers/net/wireguard/noise.c @@ -580,13 +580,23 @@ wg_noise_handshake_create_initiation(struct message_handshake_initiation *dst, return ret; } +/* Must hold handshake->lock, for reading or writing */ +static bool initiation_is_replay_or_flood(const struct noise_handshake *handshake, + const u8 timestamp[NOISE_TIMESTAMP_LEN]) +{ + return memcmp(timestamp, handshake->latest_timestamp, + NOISE_TIMESTAMP_LEN) <= 0 || + (s64)handshake->last_initiation_consumption + + NSEC_PER_SEC / INITIATIONS_PER_SECOND > + (s64)ktime_get_coarse_boottime_ns(); +} + struct wg_peer * wg_noise_handshake_consume_initiation(struct message_handshake_initiation *src, struct wg_device *wg) { struct wg_peer *peer = NULL, *ret_peer = NULL; struct noise_handshake *handshake; - bool replay_attack, flood_attack; u8 key[NOISE_SYMMETRIC_KEY_LEN]; u8 chaining_key[NOISE_HASH_LEN]; u8 hash[NOISE_HASH_LEN]; @@ -594,6 +604,7 @@ wg_noise_handshake_consume_initiation(struct message_handshake_initiation *src, u8 e[NOISE_PUBLIC_KEY_LEN]; u8 t[NOISE_TIMESTAMP_LEN]; u64 initiation_consumption; + bool reject; down_read(&wg->static_identity.lock); if (unlikely(!wg->static_identity.has_identity)) @@ -630,17 +641,21 @@ wg_noise_handshake_consume_initiation(struct message_handshake_initiation *src, goto out; down_read(&handshake->lock); - replay_attack = memcmp(t, handshake->latest_timestamp, - NOISE_TIMESTAMP_LEN) <= 0; - flood_attack = (s64)handshake->last_initiation_consumption + - NSEC_PER_SEC / INITIATIONS_PER_SECOND > - (s64)ktime_get_coarse_boottime_ns(); + reject = initiation_is_replay_or_flood(handshake, t); up_read(&handshake->lock); - if (replay_attack || flood_attack) + if (reject) goto out; /* Success! Copy everything to peer */ down_write(&handshake->lock); + /* Check again, now that we have an exclusive lock, so that two threads + * processing the same initiation in parallel can't both pass the check + * above before either of them updates the timestamp. + */ + if (initiation_is_replay_or_flood(handshake, t)) { + up_write(&handshake->lock); + goto out; + } memcpy(handshake->remote_ephemeral, e, NOISE_PUBLIC_KEY_LEN); if (memcmp(t, handshake->latest_timestamp, NOISE_TIMESTAMP_LEN) > 0) memcpy(handshake->latest_timestamp, t, NOISE_TIMESTAMP_LEN); -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net 2/2] wireguard: noise: recheck replay and flood under write lock on initiation 2026-11-05 14:08 ` [PATCH net 2/2] wireguard: noise: recheck replay and flood under write lock on initiation Alexander Ildiyarov @ 2026-11-10 16:51 ` Jason A. Donenfeld 2026-11-10 18:08 ` alex 0 siblings, 1 reply; 10+ messages in thread From: Jason A. Donenfeld @ 2026-11-10 16:51 UTC (permalink / raw) To: Alexander Ildiyarov; +Cc: alexvancasper, stable On Thu, Nov 05, 2026 at 05:08:15PM +0300, Alexander Ildiyarov wrote: > wg_noise_handshake_consume_initiation() checks the initiation's > timestamp against latest_timestamp, and last_initiation_consumption > against the current time, while holding handshake->lock for reading. > It then drops the lock and takes it again for writing to store the new > state, without repeating the checks. > > Incoming handshake packets are spread round-robin over all online CPUs, > so two copies of the same initiation, received back to back, are > processed in parallel. Both copies can pass the checks before either of > them updates latest_timestamp and last_initiation_consumption, so the > replay and flood protection is bypassed. The responder then creates two > responses, each with a new ephemeral key, and installs the keypair of > the last one, while the initiator accepts whichever response arrives > first. When those differ, the session is unusable until the initiator > gives up on it and starts a new handshake, and an attacker who can see > and duplicate initiations can repeat this for every handshake. > > The window is narrow, since the expensive DH computations happen before > the check, so it is between up_read() and down_write() only. With netem > duplicating every initiation between two namespaces, no double response > was observed in 200 handshakes on a 4-CPU VM. With a usleep_range(1000, > 2000) inserted before down_write() to widen the window, 30 of 50 > duplicated initiations produced two responses, and in 12 of 50 cases > the session was unusable after the handshake. With this patch applied, > and the same delay, every duplicate was rejected and all 50 sessions > worked. > > Fix this by doing the checks again once the write lock is held, keeping > the read-locked check as a fast path for rejecting replays. The downside, though, is that now it means there's potential to do flood attacks that take the exclusive lock, which is what was trying to be avoided, right? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 2/2] wireguard: noise: recheck replay and flood under write lock on initiation 2026-11-10 16:51 ` Jason A. Donenfeld @ 2026-11-10 18:08 ` alex 0 siblings, 0 replies; 10+ messages in thread From: alex @ 2026-11-10 18:08 UTC (permalink / raw) To: Jason A. Donenfeld; +Cc: alexvancasper, stable I don't think it changes which packets take the exclusive lock. Before this patch, every initiation that passed the read-locked check already went on to take down_write() unconditionally to copy the new state into the handshake. With the patch, exactly the same set of packets reaches down_write(). The only difference is that a packet that lost the race now sees the updated latest_timestamp/last_initiation_consumption and bails out right away, instead of overwriting the handshake state and generating a second response. So the time spent under the write lock goes down for those packets, not up. Everything else is still filtered under the read lock as before. Once the first copy has stored its timestamp and consumption time, later replays, and any initiation within the next 1/INITIATIONS_PER_SECOND, are rejected at the down_read() fast path and never touch the write lock. Only copies of the same initiation that are processed truly in parallel, so at most one per CPU, can get to down_write(), and that was already the case before. This is the same pattern the response side uses after 2a73c9b2c0ca: check under the read lock as a fast path, then check again after down_write() before committing. Thanks, Alexander -------- Исходное сообщение -------- Тема: Re: [PATCH net 2/2] wireguard: noise: recheck replay and flood under write lock on initiation Дата: 10.11.2026 16:51 От: "Jason A. Donenfeld" <Jason@zx2c4.com> Кому: Alexander Ildiyarov <alex@badcode.im> On Thu, Nov 05, 2026 at 05:08:15PM +0300, Alexander Ildiyarov wrote: > wg_noise_handshake_consume_initiation() checks the initiation's > timestamp against latest_timestamp, and last_initiation_consumption > against the current time, while holding handshake->lock for reading. > It then drops the lock and takes it again for writing to store the new > state, without repeating the checks. > > Incoming handshake packets are spread round-robin over all online CPUs, > so two copies of the same initiation, received back to back, are > processed in parallel. Both copies can pass the checks before either of > them updates latest_timestamp and last_initiation_consumption, so the > replay and flood protection is bypassed. The responder then creates two > responses, each with a new ephemeral key, and installs the keypair of > the last one, while the initiator accepts whichever response arrives > first. When those differ, the session is unusable until the initiator > gives up on it and starts a new handshake, and an attacker who can see > and duplicate initiations can repeat this for every handshake. > > The window is narrow, since the expensive DH computations happen before > the check, so it is between up_read() and down_write() only. With netem > duplicating every initiation between two namespaces, no double response > was observed in 200 handshakes on a 4-CPU VM. With a usleep_range(1000, > 2000) inserted before down_write() to widen the window, 30 of 50 > duplicated initiations produced two responses, and in 12 of 50 cases > the session was unusable after the handshake. With this patch applied, > and the same delay, every duplicate was rejected and all 50 sessions > worked. > > Fix this by doing the checks again once the write lock is held, keeping > the read-locked check as a fast path for rejecting replays. The downside, though, is that now it means there's potential to do flood attacks that take the exclusive lock, which is what was trying to be avoided, right? ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20261105141006.95083-1-alex@badcode.im>]
* [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg [not found] <20261105141006.95083-1-alex@badcode.im> @ 2026-11-05 14:10 ` Alexander Ildiyarov 2026-11-09 15:20 ` netdev-bot+sashiko 2026-11-10 16:57 ` Jason A. Donenfeld 0 siblings, 2 replies; 10+ messages in thread From: Alexander Ildiyarov @ 2026-11-05 14:10 UTC (permalink / raw) To: Jason A . Donenfeld, wireguard, netdev Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel, Alexander Ildiyarov, stable Peers, allowed IP trie nodes, the per-peer dst_cache and the per-device hashtables are all allocated in the context of the process that configures the device over netlink, but none of these allocations are charged to that process's memory cgroup: the wg_peer and allowedips_node caches are created without SLAB_ACCOUNT, and the hashtables and dst_cache use plain GFP_KERNEL. The amount of memory is bounded only by MAX_PEERS_PER_DEVICE (2^20) per device, with no limit on the number of devices or on the number of allowed IPs per peer. Since WG_CMD_SET_DEVICE uses GENL_UNS_ADMIN_PERM, any user who can create a user and network namespace can do this, and thereby escape the memory limits of the cgroup it runs in. For example, a process in a cgroup with memory.max=64M that adds 100000 peers with 20 /128 allowed IPs each grows Slab by ~790 MiB, yet the cgroup's memory.current stays below 1 MiB and its memory limit is never hit. Fix this by creating the two caches with SLAB_ACCOUNT and using GFP_KERNEL_ACCOUNT for the hashtables and the dst_cache. The keypair allocations are left alone, as they happen from the handshake worker and are bounded to three per peer. Fixes: e7096c131e51 ("net: WireGuard secure network tunnel") Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Alexander Ildiyarov <alex@badcode.im> --- drivers/net/wireguard/allowedips.c | 2 +- drivers/net/wireguard/peer.c | 4 ++-- drivers/net/wireguard/peerlookup.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/wireguard/allowedips.c b/drivers/net/wireguard/allowedips.c index 5ece9acad64d..2f2b8a94c8a7 100644 --- a/drivers/net/wireguard/allowedips.c +++ b/drivers/net/wireguard/allowedips.c @@ -411,7 +411,7 @@ struct wg_peer *wg_allowedips_lookup_src(struct allowedips *table, int __init wg_allowedips_slab_init(void) { - node_cache = KMEM_CACHE(allowedips_node, 0); + node_cache = KMEM_CACHE(allowedips_node, SLAB_ACCOUNT); return node_cache ? 0 : -ENOMEM; } diff --git a/drivers/net/wireguard/peer.c b/drivers/net/wireguard/peer.c index 1cb502a932e0..05b80521c706 100644 --- a/drivers/net/wireguard/peer.c +++ b/drivers/net/wireguard/peer.c @@ -33,7 +33,7 @@ struct wg_peer *wg_peer_create(struct wg_device *wg, peer = kmem_cache_zalloc(peer_cache, GFP_KERNEL); if (unlikely(!peer)) return ERR_PTR(ret); - if (unlikely(dst_cache_init(&peer->endpoint_cache, GFP_KERNEL))) + if (unlikely(dst_cache_init(&peer->endpoint_cache, GFP_KERNEL_ACCOUNT))) goto err; peer->device = wg; @@ -229,7 +229,7 @@ void wg_peer_put(struct wg_peer *peer) int __init wg_peer_init(void) { - peer_cache = KMEM_CACHE(wg_peer, 0); + peer_cache = KMEM_CACHE(wg_peer, SLAB_ACCOUNT); return peer_cache ? 0 : -ENOMEM; } diff --git a/drivers/net/wireguard/peerlookup.c b/drivers/net/wireguard/peerlookup.c index 99651a86ac95..f333e906014a 100644 --- a/drivers/net/wireguard/peerlookup.c +++ b/drivers/net/wireguard/peerlookup.c @@ -21,7 +21,7 @@ static struct hlist_head *pubkey_bucket(struct pubkey_hashtable *table, struct pubkey_hashtable *wg_pubkey_hashtable_alloc(void) { - struct pubkey_hashtable *table = kvmalloc_obj(*table); + struct pubkey_hashtable *table = kvmalloc_obj(*table, GFP_KERNEL_ACCOUNT); if (!table) return NULL; @@ -82,7 +82,7 @@ static struct hlist_head *index_bucket(struct index_hashtable *table, struct index_hashtable *wg_index_hashtable_alloc(void) { - struct index_hashtable *table = kvmalloc_obj(*table); + struct index_hashtable *table = kvmalloc_obj(*table, GFP_KERNEL_ACCOUNT); if (!table) return NULL; -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg 2026-11-05 14:10 ` [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg Alexander Ildiyarov @ 2026-11-09 15:20 ` netdev-bot+sashiko 2026-11-10 16:57 ` Jason A. Donenfeld 1 sibling, 0 replies; 10+ messages in thread From: netdev-bot+sashiko @ 2026-11-09 15:20 UTC (permalink / raw) To: alex Cc: Jason, wireguard, netdev, andrew+netdev, davem, edumazet, kuba, pabeni, linux-kernel, stable Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider. Critical: 0 · High: 1 · Medium: 2 · Low: 0 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [High] Incomplete fix: every peer still creates uncharged memory and a PID. - [Medium] Incomplete fix: per-device allocations in wg_newlink() are still not charged. - [Medium] The commit leaves noise_keypair allocations uncharged, saying they 'happen from the handshake worker and are bounded to three per… -- wireguard: account peer and allowedips allocations to memcg This patch creates the wg_peer and allowedips_node slab caches with SLAB_ACCOUNT. It also switches the per-peer dst_cache and the per-device pubkey and index hashtables to GFP_KERNEL_ACCOUNT. The goal is to stop a user with a user and network namespace from escaping its memory cgroup limit by adding peers over WG_CMD_SET_DEVICE. > The keypair allocations are left alone, as they happen from the handshake > worker and are bounded to three per peer. [Severity: Medium] Does the per-peer bound make the keypair memory small enough to ignore? struct noise_keypair embeds a noise_replay_counter with: unsigned long backtrack[COUNTER_BITS_TOTAL / BITS_PER_LONG]; COUNTER_BITS_TOTAL is 8192, so that is a 1KB bitmap. The struct ends up around 1.2KB, which means keypair_create() is served from kmalloc-2k: drivers/net/wireguard/noise.c:keypair_create() { struct noise_keypair *keypair = kzalloc_obj(*keypair); ... } add_new_keypair() keeps two keypairs in slots per peer, plus any that are in flight or waiting on RCU. That is about 4KB or more of uncharged memory per peer. The now-charged wg_peer object plus the per-CPU dst_cache come to about 2KB. The configuring user picks every peer public key, so it knows the matching private keys. A UDP program in the same netns can send valid handshake initiations for every peer to the listen port. The responder path then allocates keypairs from the kworker: wg_packet_handshake_receive_worker() wg_packet_send_handshake_response() wg_noise_handshake_begin_session() keypair_create() Once the peer count is limited only by charged memory, could the uncharged keypair memory still reach about 1-2x memory.max? This runs in a worker, so GFP_KERNEL_ACCOUNT alone would charge root. Would this need the configuring memcg to be recorded, with set_active_memcg() used in the handshake path? > diff --git a/drivers/net/wireguard/peer.c b/drivers/net/wireguard/peer.c > index 1cb502a932e07..05b80521c7061 100644 > --- a/drivers/net/wireguard/peer.c > +++ b/drivers/net/wireguard/peer.c > @@ -33,7 +33,7 @@ struct wg_peer *wg_peer_create(struct wg_device *wg, > peer = kmem_cache_zalloc(peer_cache, GFP_KERNEL); > if (unlikely(!peer)) > return ERR_PTR(ret); > - if (unlikely(dst_cache_init(&peer->endpoint_cache, GFP_KERNEL))) > + if (unlikely(dst_cache_init(&peer->endpoint_cache, GFP_KERNEL_ACCOUNT))) > goto err; > > peer->device = wg; [Severity: High] The peer slab and dst_cache are now charged. Does each peer still pin a larger amount of uncharged memory through its NAPI kthread? wg_newlink() turns on threaded NAPI for every wireguard device: netif_threaded_enable(dev); ret = register_netdevice(dev); Later in wg_peer_create(), each peer does: netif_napi_add(wg->dev, &peer->napi, wg_packet_rx_poll); Peers have no napi->config, so napi_get_threaded_config() returns dev->threaded. That spawns one kthread per peer: wg_peer_create() netif_napi_add() netif_napi_add_weight_locked() napi_kthread_create() kthread_run(napi_threaded_poll, n, "napi/%s-%d", ...) kthreadd forks the thread. Its task_struct and kernel stack (about 20-30KB) are therefore charged to the root memcg. The PID is also taken outside the caller's pids cgroup and RLIMIT_NPROC. Each peer charges only about 2KB for wg_peer plus dst_cache. Could a cgroup with memory.max=64M still create tens of thousands of peers and pin hundreds of MiB of uncharged memory? With a larger limit, could this also use up pid_max for the whole system? More GFP_KERNEL_ACCOUNT flags would not cover this, because kthreadd does the allocation. Would this need a per-device or per-user cap on peers or NAPI threads, or a design that does not spawn a kthread per peer? The next patch in the series ("wireguard: noise: recheck replay and flood under write lock on initiation") only changes the replay checks in noise.c and does not affect this. [ ... ] > diff --git a/drivers/net/wireguard/peerlookup.c b/drivers/net/wireguard/peerlookup.c > index 99651a86ac953..f333e906014ab 100644 > --- a/drivers/net/wireguard/peerlookup.c > +++ b/drivers/net/wireguard/peerlookup.c [ ... ] > @@ -82,7 +82,7 @@ static struct hlist_head *index_bucket(struct index_hashtable *table, > > struct index_hashtable *wg_index_hashtable_alloc(void) > { > - struct index_hashtable *table = kvmalloc_obj(*table); > + struct index_hashtable *table = kvmalloc_obj(*table, GFP_KERNEL_ACCOUNT); > > if (!table) > return NULL; [Severity: Medium] The two per-device hashtables are now charged, and the commit message says the number of devices has no limit. Are the other per-device allocations in wg_newlink() still uncharged? wg_newlink() calls wg_packet_queue_init() three times, and each call does: drivers/net/wireguard/queueing.c:wg_packet_queue_init() { ... ret = ptr_ring_init(&queue->ring, len, GFP_KERNEL); ... queue->worker = wg_packet_percpu_multicore_worker_alloc(function, queue); ... } ptr_ring_init() ends up in kvmalloc_array() with GFP_KERNEL. On 64-bit that is 8KB + 8KB + 32KB per device (MAX_QUEUED_PACKETS and MAX_QUEUED_INCOMING_HANDSHAKES). The per-CPU worker arrays come from alloc_percpu() without __GFP_ACCOUNT: struct multicore_worker __percpu *worker = alloc_percpu(struct multicore_worker); wg_newlink() also creates three workqueues. packet_crypt_wq uses WQ_MEM_RECLAIM, which adds a rescuer kthread forked by kthreadd: wg->packet_crypt_wq = alloc_workqueue("wg-crypt-%s", WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM | WQ_PERCPU, 0, dev->name); Could a user with user and net namespaces call RTM_NEWLINK in a loop to create wireguard devices? That would still allocate per-device memory that never shows up in memory.current. Should the ptr_ring and per-CPU worker allocations be charged as well? -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105141006.95083-1-alex%40badcode.im ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg 2026-11-05 14:10 ` [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg Alexander Ildiyarov 2026-11-09 15:20 ` netdev-bot+sashiko @ 2026-11-10 16:57 ` Jason A. Donenfeld 2026-11-10 18:04 ` alex 1 sibling, 1 reply; 10+ messages in thread From: Jason A. Donenfeld @ 2026-11-10 16:57 UTC (permalink / raw) To: Alexander Ildiyarov Cc: wireguard, netdev, Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel, stable On Thu, Nov 05, 2026 at 05:10:05PM +0300, Alexander Ildiyarov wrote: > Peers, allowed IP trie nodes, the per-peer dst_cache and the per-device > hashtables are all allocated in the context of the process that > configures the device over netlink, but none of these allocations are > charged to that process's memory cgroup: the wg_peer and allowedips_node > caches are created without SLAB_ACCOUNT, and the hashtables and > dst_cache use plain GFP_KERNEL. > > The amount of memory is bounded only by MAX_PEERS_PER_DEVICE (2^20) per > device, with no limit on the number of devices or on the number of > allowed IPs per peer. Since WG_CMD_SET_DEVICE uses GENL_UNS_ADMIN_PERM, > any user who can create a user and network namespace can do this, and > thereby escape the memory limits of the cgroup it runs in. > > For example, a process in a cgroup with memory.max=64M that adds 100000 > peers with 20 /128 allowed IPs each grows Slab by ~790 MiB, yet the > cgroup's memory.current stays below 1 MiB and its memory limit is never > hit. > > Fix this by creating the two caches with SLAB_ACCOUNT and using > GFP_KERNEL_ACCOUNT for the hashtables and the dst_cache. The keypair > allocations are left alone, as they happen from the handshake worker and > are bounded to three per peer. I wasn't aware of this trick, I think mostly because most drivers don't use this. Does any other part of the networking stack do this? Is this a normal thing? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg 2026-11-10 16:57 ` Jason A. Donenfeld @ 2026-11-10 18:04 ` alex 2026-11-10 18:06 ` Jason A. Donenfeld 0 siblings, 1 reply; 10+ messages in thread From: alex @ 2026-11-10 18:04 UTC (permalink / raw) To: Jason A. Donenfeld Cc: wireguard, netdev, Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel, stable Yes, it is the documented expectation for long-lived objects whose allocation userspace can trigger. Documentation/core-api/memory-allocation.rst says: Untrusted allocations triggered from userspace should be a subject of kmem accounting and must have __GFP_ACCOUNT bit set. Most hardware drivers don't need it because their allocations are sized by the hardware, not by userspace. Virtual devices configured over netlink from inside a user namespace are a different case, and the core stack has been converted for that reason. For example: 6126891c6d4f ("memcg: enable accounting for IP address and routing-related objects") c948f51c1654 ("memcg: enable accounting for net_device and Tx/Rx queues") a89893dd7b08 ("memcg: enable accounting for VLAN group array") 961c6136359e ("net: enable memcg accounting for veth queues") 33758c891479 ("memcg: enable accounting for nft objects") and more recently: 1beb81947eb4 ("net/sched: account classifier filter allocations to memcg") b0057c68df71 ("xsk: account ring allocations to memcg") The fib_trie, ip6_fib and ip6 dst caches are created with SLAB_ACCOUNT, and the netdev queues and VLAN array use GFP_KERNEL_ACCOUNT. The first commit describes the same situation as here: a netadmin inside a container adds many addresses/routes and makes the kernel allocate unaccounted long-lived memory. WireGuard peers and allowed IPs are the same kind of object, so they should be charged to the same memcg. There is no functional change unless a memcg limit is set. Then the configuring process gets -ENOMEM (or the memcg OOM killer runs) instead of the allocations silently going past the limit. Thanks, Alexander -------- Исходное сообщение -------- Тема: Re: [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg Дата: 10.11.2026 16:57 От: "Jason A. Donenfeld" <Jason@zx2c4.com> Кому: Alexander Ildiyarov <alex@badcode.im> On Thu, Nov 05, 2026 at 05:10:05PM +0300, Alexander Ildiyarov wrote: > Peers, allowed IP trie nodes, the per-peer dst_cache and the per-device > hashtables are all allocated in the context of the process that > configures the device over netlink, but none of these allocations are > charged to that process's memory cgroup: the wg_peer and > allowedips_node > caches are created without SLAB_ACCOUNT, and the hashtables and > dst_cache use plain GFP_KERNEL. > > The amount of memory is bounded only by MAX_PEERS_PER_DEVICE (2^20) per > device, with no limit on the number of devices or on the number of > allowed IPs per peer. Since WG_CMD_SET_DEVICE uses GENL_UNS_ADMIN_PERM, > any user who can create a user and network namespace can do this, and > thereby escape the memory limits of the cgroup it runs in. > > For example, a process in a cgroup with memory.max=64M that adds 100000 > peers with 20 /128 allowed IPs each grows Slab by ~790 MiB, yet the > cgroup's memory.current stays below 1 MiB and its memory limit is never > hit. > > Fix this by creating the two caches with SLAB_ACCOUNT and using > GFP_KERNEL_ACCOUNT for the hashtables and the dst_cache. The keypair > allocations are left alone, as they happen from the handshake worker > and > are bounded to three per peer. I wasn't aware of this trick, I think mostly because most drivers don't use this. Does any other part of the networking stack do this? Is this a normal thing? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg 2026-11-10 18:04 ` alex @ 2026-11-10 18:06 ` Jason A. Donenfeld 0 siblings, 0 replies; 10+ messages in thread From: Jason A. Donenfeld @ 2026-11-10 18:06 UTC (permalink / raw) To: alex Cc: wireguard, netdev, Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel, stable Okay, I guess I was just looking for some judgement discussion, not more output from the LLM. I'll look into this myself on the slow track for a little bit. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-11-10 18:08 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20261105140815.92841-1-alex@badcode.im>
2026-11-05 14:08 ` [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg Alexander Ildiyarov
2026-11-10 16:52 ` Jason A. Donenfeld
2026-11-05 14:08 ` [PATCH net 2/2] wireguard: noise: recheck replay and flood under write lock on initiation Alexander Ildiyarov
2026-11-10 16:51 ` Jason A. Donenfeld
2026-11-10 18:08 ` alex
[not found] <20261105141006.95083-1-alex@badcode.im>
2026-11-05 14:10 ` [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg Alexander Ildiyarov
2026-11-09 15:20 ` netdev-bot+sashiko
2026-11-10 16:57 ` Jason A. Donenfeld
2026-11-10 18:04 ` alex
2026-11-10 18:06 ` Jason A. Donenfeld
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox