[PATCH net v4] net/packet: account user-triggered allocations to memcg

[PATCH net v4] net/packet: account user-triggered allocations to memcg

[Ahmad AG]

A packet socket makes allocations whose size or count its user chooses
without charging them to the user's memory cgroup: the blocks and block
array of a PACKET_RX_RING or PACKET_TX_RING ring and the rx_owner_map
of a TPACKET_V1/V2 RX ring, the PACKET_FANOUT group (about 1 MB with
max_num_members = 65536), and one packet_mclist per
PACKET_ADD_MEMBERSHIP entry. CAP_NET_RAW in a user namespace is enough
to pin memory far beyond memory.max this way.

Allocate them with GFP_KERNEL_ACCOUNT, as the AF_XDP rings do since
commit b0057c68df71 ("xsk: account ring allocations to memcg"), and with
__GFP_NOWARN where the size comes from user space, so that a ring or
group that does not fit prints no allocation warning. Fixed-size
per-socket state is left unaccounted.

Reproducer, as uid 1000 in a memory cgroup with memory.max 16M, after
unshare(CLONE_NEWUSER | CLONE_NEWNET): open 64 AF_PACKET sockets, give
each a TPACKET_V2 RX ring of 8 blocks of 1 MB, mmap the rings and touch
every block. MemFree drops by 526052 KB while memory.current grows by
1280 KB. 256 fanout groups with max_num_members = 65536 (265144 KB used,
512 KB charged) and 100 sockets with 2000 PACKET_MR_PROMISC entries each
on lo (45220 KB used, 124 KB charged) behave the same. With this change
all three are stopped by the cgroup's OOM killer. psock_fanout,
psock_tpacket and psock_snd.sh pass before and after. Found by code
inspection with the help of an LLM and reproduced on an unmodified
kernel without error injection; tested in a QEMU guest with KASAN on
net.

The ring pages and rx_owner_map were never charged, the rest not since
commit a9bb7e620efd ("memcg: only account kmem allocations marked as
__GFP_ACCOUNT"). The Fixes commit let the user size the fanout array.

Fixes: 9c661b0b8544 ("net/packet: make packet_fanout.arr size configurable up to 64K")
Cc: <stable+noautosel@kernel.org> # changes memcg charging of existing packet rings; not stable material
Signed-off-by: Ahmad AG <ahmadag1001@gmail.com>
---
Tool use: an LLM coding assistant (Claude Code) found the missing
charges by code inspection, drafted the patch, the test program and this
message, and ran the tests in the QEMU guest; I reviewed every hunk, the
results and this message before sending.

v4:
  - trim the commit message
  - add __GFP_NOWARN to the fanout group allocation
  - use __vcalloc() for the vmalloc fallback and bitmap_zalloc() for
    rx_owner_map
v3: https://lore.kernel.org/netdev/20261031084316.387533-1-ahmadag1001@gmail.com/
  - Fixes 9c661b0b8544; stable+noautosel; drop Assisted-by; leave fixed-size
    allocations unaccounted; __GFP_NOWARN on the vmalloc fallback
v2: https://lore.kernel.org/netdev/20261030100504.23498-1-ahmadag1001@gmail.com/
  - cover all user-triggered allocations in one patch
v1: https://lore.kernel.org/netdev/CAASqGyZ-9hqZGWNJShxnrSiXDO==51SVsQW_x_mk993y1pNO8A@mail.gmail.com/

 net/packet/af_packet.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index cf6429f0947..ec6ab96c4f8 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1778,7 +1778,8 @@ static int fanout_add(struct sock *sk, struct fanout_args *args)
 			/* legacy PACKET_FANOUT_MAX */
 			args->max_num_members = 256;
 		err = -ENOMEM;
-		match = kvzalloc_flex(*match, arr, args->max_num_members);
+		match = kvzalloc_flex(*match, arr, args->max_num_members,
+				      GFP_KERNEL_ACCOUNT | __GFP_NOWARN);
 		if (!match)
 			goto out;
 		write_pnet(&match->net, sock_net(sk));
@@ -3773,7 +3774,7 @@ static int packet_mc_add(struct sock *sk, struct packet_mreq_max *mreq)
 		goto done;
 
 	err = -ENOBUFS;
-	i = kmalloc_obj(*i);
+	i = kmalloc_obj(*i, GFP_KERNEL_ACCOUNT);
 	if (i == NULL)
 		goto done;
 
@@ -4494,7 +4495,7 @@ static void packet_free_tx_ring(struct sock *sk, struct pgv *pg_vec,
 static char *alloc_one_pg_vec_page(unsigned long order)
 {
 	char *buffer;
-	gfp_t gfp_flags = GFP_KERNEL | __GFP_COMP |
+	gfp_t gfp_flags = GFP_KERNEL_ACCOUNT | __GFP_COMP |
 			  __GFP_ZERO | __GFP_NOWARN | __GFP_NORETRY;
 
 	buffer = (char *) __get_free_pages(gfp_flags, order);
@@ -4502,7 +4503,8 @@ static char *alloc_one_pg_vec_page(unsigned long order)
 		return buffer;
 
 	/* __get_free_pages failed, fall back to vmalloc */
-	buffer = vzalloc(array_size((1 << order), PAGE_SIZE));
+	buffer = __vcalloc(1 << order, PAGE_SIZE,
+			   GFP_KERNEL_ACCOUNT | __GFP_NOWARN);
 	if (buffer)
 		return buffer;
 
@@ -4523,7 +4525,8 @@ static struct pgv *alloc_pg_vec(struct tpacket_req *req, int order, bool tx_ring
 	struct pgv *pg_vec;
 	int i;
 
-	vec = kzalloc_flex(*vec, pg_vec, block_nr, GFP_KERNEL | __GFP_NOWARN);
+	vec = kzalloc_flex(*vec, pg_vec, block_nr,
+			   GFP_KERNEL_ACCOUNT | __GFP_NOWARN);
 	if (unlikely(!vec))
 		return NULL;
 	vec->order = order;
@@ -4647,8 +4650,10 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
 			break;
 		default:
 			if (!tx_ring) {
-				rx_owner_map = bitmap_alloc(req->tp_frame_nr,
-					GFP_KERNEL | __GFP_NOWARN | __GFP_ZERO);
+				rx_owner_map =
+					bitmap_zalloc(req->tp_frame_nr,
+						      GFP_KERNEL_ACCOUNT |
+						      __GFP_NOWARN);
 				if (!rx_owner_map)
 					goto out_free_pg_vec;
 			}

base-commit: cb284914e749388eef402938bdb3115fe1f6d337
-- 
2.43.0

Re: [PATCH net v4] net/packet: account user-triggered allocations to memcg

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · 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).

- [Medium] The patch only partly closes the PACKET_ADD_MEMBERSHIP memcg
  bypass.

--

net/packet: account user-triggered allocations to memcg

This commit changes several AF_PACKET allocations whose size or count is
chosen by user space to use GFP_KERNEL_ACCOUNT, so they are charged to the
caller's memory cgroup. It covers the ring blocks and block array, the
rx_owner_map, the fanout group, and the packet_mclist entries created by
PACKET_ADD_MEMBERSHIP.

> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index cf6429f094772..ec6ab96c4f87c 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c

[ ... ]

> @@ -3773,7 +3774,7 @@ static int packet_mc_add(struct sock *sk, struct packet_mreq_max *mreq)
>  		goto done;
>  
>  	err = -ENOBUFS;
> -	i = kmalloc_obj(*i);
> +	i = kmalloc_obj(*i, GFP_KERNEL_ACCOUNT);
>  	if (i == NULL)
>  		goto done;
>  

[Severity: Medium]
Does this fully close the PACKET_ADD_MEMBERSHIP bypass described in the
commit message?

For PACKET_MR_MULTICAST and PACKET_MR_UNICAST, packet_mc_add() then calls
packet_dev_mc(), which adds the address to the device list:

packet_mc_add()
  packet_dev_mc()
    dev_mc_add() / dev_uc_add()
      __hw_addr_add_ex()
        __hw_addr_create()

Each distinct address gets a struct netdev_hw_addr, and that allocation
is not charged:

net/core/dev_addr_lists.c:__hw_addr_create() {
    ...
	ha = kmalloc(alloc_size, GFP_ATOMIC);
    ...
}

A struct netdev_hw_addr is about 104 bytes (kmalloc-128). That is larger
than the packet_mclist that is now charged (about 72 bytes,
kmalloc-cg-96). Neither packet_mc_add() nor dev_addr_lists.c limits how
many entries can be added.

On devices with ndo_set_rx_mode_async, netif_addr_lists_snapshot() ->
__hw_addr_list_snapshot() also makes two more GFP_ATOMIC copies of each
uc/mc entry. These copies stay in dev->rx_mode_addr_cache, which is only
flushed in free_netdev().

So a CAP_NET_RAW user in a user namespace can add many distinct
MR_MULTICAST or MR_UNICAST addresses on lo. Each one still pins more
uncharged memory than gets charged.

Would it make sense to also add __GFP_ACCOUNT to the allocation in
__hw_addr_add_ex(), which runs in process context?

The commit message says:

    100 sockets with 2000 PACKET_MR_PROMISC entries each
    on lo (45220 KB used, 124 KB charged) behave the same. With this change
    all three are stopped by the cgroup's OOM killer.

PACKET_MR_PROMISC only calls dev_set_promiscuity() and never reaches
__hw_addr_create(), so the reproducer does not exercise this path.
Should the commit message be narrowed, or should the device address
entries be covered as well?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261101094847.664612-1-ahmadag1001%40gmail.com

Re: [PATCH net v4] net/packet: account user-triggered allocations to memcg

[Ahmad AG]

> Does this fully close the PACKET_ADD_MEMBERSHIP bypass described in the
> commit message?

It covers the packet_mclist entries, which is what the commit message
claims, and the PACKET_MR_PROMISC numbers measure exactly that path.

The struct netdev_hw_addr entries are a separate case. They are device
state in net/core, shared and refcounted across every dev_uc_add() and
dev_mc_add() caller, and allocated with GFP_ATOMIC under
netif_addr_lock_bh(). Accounting them changes behaviour for all of
those callers, not just AF_PACKET, so I would rather keep it out of
this fix and look at it as a separate net/core patch if that is wanted.