Re: [PATCH net v2 0/1] xsk: fix unaccounted ring allocations causing memory exhaustion

[Stanislav Fomichev <sdf.kernel@gmail.com>]

On 07/30, Zihan Xi wrote:
> Hi Linux kernel maintainers,
> 
> We found and validated a issue in net/xdp/xsk_queue.c. The bug is reachable by a
> non-root user via user and net namespace.
> We've tested it, and it should not affect any other functionality.
> 
> We will provide detailed information about the bug
> in this email, along with a PoC to trigger it.
> 
> ---- details below ----
> 
> Bug details:
> 
> AF_XDP lets user space allocate RX, TX, UMEM fill and UMEM completion
> rings with setsockopt() and mmap them into the process. The shared
> xskq_create() helper allocates the ring backing memory, but unlike UMEM
> registration it does not account those pages against RLIMIT_MEMLOCK /
> user->locked_vm.
> 
> As a result, a process with CAP_NET_RAW in a user and network namespace
> can request very large rings and pin a large amount of kernel memory
> before bind or any packet I/O. In our original report the resulting OOM
> ended in a panic because that guest had panic_on_oom enabled. The panic
> was only the environment-specific end result; the bug itself is the
> missing resource boundary that lets ring allocations consume excessive
> memory in the first place.
> 
> The first version tried to bound these allocations with sysctl_optmem_max,
> but that was the wrong resource model. We also evaluated a memcg-accounted
> vmalloc path as suggested,

[..]

> but memcg attribution by itself did not impose
> a default enforcement boundary in our validation setup.

Can you expand on this? Maybe we don't pass some GFP_ACCOUNT flag or
something?

> AF_XDP already
> uses RLIMIT_MEMLOCK / user->locked_vm to bound pinned UMEM pages, and the
> ring pages are likewise user-controlled, mmapable and long-lived. This
> version therefore accounts AF_XDP ring allocations with the existing
> mm_account_pinned_pages() helper and unaccounts them when the queue is
> destroyed, so ring memory is constrained by the same default limit model
> already used for AF_XDP UMEM pages.

We get RLIMIT_MEMLOCK just because the users do mmap(), nothing af_xdp
specific. Idk, manually doing accounting seems a bit wrong, memcg/kmem seems
like the way to go. (also, panic on oom also still seems wrong)