mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ipc/sem: account enlarged sops arrays to memcg
@ 2026-10-16 15:26 Wang Yingjie
  2026-10-16 22:59 ` Andrew Morton
  0 siblings, 1 reply; 2+ messages in thread
From: Wang Yingjie @ 2026-10-16 15:26 UTC (permalink / raw)
  To: Alexey Gladkov, Andrew Morton, Eric W. Biederman
  Cc: Manfred Spraul, Vasily Averin, Johannes Weiner, linux-kernel,
	linux-mm, cgroups

Commit 50ec499b9a43 ("sysctl: allow change system v ipc sysctls
inside ipc namespace") lets an IPC namespace owner change kernel.sem.
The SEMOPM field defaults to 500 but has no semantic range check.

do_semtimedop() allocates nsops * sizeof(struct sembuf) with
GFP_KERNEL. A blocking operation retains that buffer, so a user can
raise SEMOPM and hold large attacker-selected amounts of host kernel
memory without charging the calling task memcg.

On the current mainline base, one 10,000,000-operation call retained
60,936,448 bytes while a 32 MiB cgroup peaked at 909,312 bytes and
reported no limit event. Sixteen 1,000,000-operation calls retained
100,860,416 bytes while the cgroup peaked at 1,310,720 bytes.

Commit 6a4746ba0619 ("ipc: remove memcg accounting for sops objects in
do_semtimedop()") removed accounting on the premise that the buffer was
at most two pages. Preserve that default-sized path, but account arrays
larger than the default SEMOPM value.

With this change, both test configurations reach the 32 MiB cgroup
limit and the caller is terminated by the memcg OOM killer before it
can retain the enlarged arrays.

The sysctl also accepts negative SEMOPM values. Since nsops is unsigned,
SEMOPM=-1 bypasses the intended operation-count check. This change still
accounts any admitted array larger than the default, while leaving
SEMOPM range policy for separate review.

Fixes: 50ec499b9a43 ("sysctl: allow change system v ipc sysctls inside ipc namespace")
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 ipc/sem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ipc/sem.c b/ipc/sem.c
index 5ec41de7e85b2b..8c9edecc160763 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -2234,7 +2234,9 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
 		return -EINVAL;
 
 	if (nsops > SEMOPM_FAST) {
-		sops = kvmalloc_objs(*sops, nsops);
+		gfp_t gfp = nsops > SEMOPM ? GFP_KERNEL_ACCOUNT : GFP_KERNEL;
+
+		sops = kvmalloc_objs(*sops, nsops, gfp);
 		if (sops == NULL)
 			return -ENOMEM;
 	}

base-commit: 2f2f7eeef8b0a1a9b9ca998231419873ccae8d4e
-- 
2.43.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] ipc/sem: account enlarged sops arrays to memcg
  2026-10-16 15:26 [PATCH] ipc/sem: account enlarged sops arrays to memcg Wang Yingjie
@ 2026-10-16 22:59 ` Andrew Morton
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Morton @ 2026-10-16 22:59 UTC (permalink / raw)
  To: Wang Yingjie
  Cc: Alexey Gladkov, Eric W. Biederman, Manfred Spraul, Vasily Averin,
	Johannes Weiner, linux-kernel, linux-mm, cgroups

(Alexey, Eric, others: please weigh in)

On Fri, 16 Oct 2026 15:26:11 +0000 Wang Yingjie <x-professor@outlook.com> wrote:

> Commit 50ec499b9a43 ("sysctl: allow change system v ipc sysctls
> inside ipc namespace") lets an IPC namespace owner change kernel.sem.
> The SEMOPM field defaults to 500 but has no semantic range check.
> 
> do_semtimedop() allocates nsops * sizeof(struct sembuf) with
> GFP_KERNEL. A blocking operation retains that buffer, so a user can
> raise SEMOPM and hold large attacker-selected amounts of host kernel
> memory without charging the calling task memcg.
> 
> On the current mainline base, one 10,000,000-operation call retained
> 60,936,448 bytes while a 32 MiB cgroup peaked at 909,312 bytes and
> reported no limit event. Sixteen 1,000,000-operation calls retained
> 100,860,416 bytes while the cgroup peaked at 1,310,720 bytes.
> 
> Commit 6a4746ba0619 ("ipc: remove memcg accounting for sops objects in
> do_semtimedop()") removed accounting on the premise that the buffer was
> at most two pages. Preserve that default-sized path, but account arrays
> larger than the default SEMOPM value.

OK, I think. But...

> With this change, both test configurations reach the 32 MiB cgroup
> limit and the caller is terminated by the memcg OOM killer before it
> can retain the enlarged arrays.

Only if memcg is enabled and the task's memory cgroup has an effective
limit.  Otherwise we can end up handing out idiotic amounts of kernel
memory.  Memcg accounting isn't a substitute for limiting the
allocation size.

> The sysctl also accepts negative SEMOPM values. Since nsops is unsigned,
> SEMOPM=-1 bypasses the intended operation-count check.

Let's fix this first. nsops is unsigned int, whereas ns->sc_semopm is
int. In the comparison nsops > ns->sc_semopm, a negative SEMOPM is
converted to unsigned, so SEMOPM=-1 effectively sets the limit to
UINT_MAX.

I'm thinking:

- Permit init_ipc_ns.sc_semopm to be set to any value from 1 to INT_MAX.
  This preserves the existing ability of host root to configure larger
  limits.

- For all other IPC namespaces, permit values from 1 to 1000. The
  documentation has long recommended 1000 as an upper limit, although
  it hasn't been enforced. This would be a new restriction, so the
  changelog should explain why we're introducing it.

- Validate the new values before installing any of them.  The current
  proc_ipc_sem_dointvec() updates the live array directly, and
  rejecting an invalid value afterward would leave a window in which
  other tasks could observe it.

- Update the comment in include/uapi/linux/sem.h. The warning about
  physical memory fragmentation is outdated now that the allocation
  uses kvmalloc(). Document the new 1000 limit for non-initial IPC
  namespaces instead.

Ideally, a new IPC namespace would inherit its parent's configured
limit, but currently every new namespace starts at SEMOPM=500. I don't
think we need to change that as part of this fix.

Then please redo your memcg-accounting patch as a separate followup,
with suitably adjusted changelogging. That would provide additional
protection for legitimately enlarged allocations in the initial IPC
namespace and for allocations between 501 and 1000 operations elsewhere.

Thanks.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-16 22:59 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-16 15:26 [PATCH] ipc/sem: account enlarged sops arrays to memcg Wang Yingjie
2026-10-16 22:59 ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®