Re: [PATCH] ipc/sem: account enlarged sops arrays to memcg

[Manfred Spraul <manfred@colorfullife.com>]

Hi,

On 10/17/26 12:59 AM, Andrew Morton wrote:
> (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.

Just for understanding:

Each sleeping thread (within do_semtimedop()) can use 60 MB, assuming it 
triggers a 10 mio semop() call.

It could be a bogus 5 mio times "increase by 1, decrease by 1" semop(), 
or a valid semop() on a suitably large array.

>> 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 previous assumption was the other way around:

ipc is full of legacy users.

Thus any hard limit risks to break existing setups.

If I remember it right, either SHM or MSG had code that tried to split 
the permitted allocation, i.e. if someone creates a new namespace, then 
each name space inherits half of the limit.

This was removed, since memcg would be the better substitute.

Old discussion: https://marc.info/?l=linux-mm&m=139639755705736&w=2

But I can't find right now the code that tried to split the limits.

>> 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.

This would mean workloads with higher SEMOPM values cannot run in 
containers, correct?

According to commit e4243b8062c1 "ipc/sem: play nicer with large nsops 
allocations", values up to 1000 are recommended.

And 
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/5/html/Tuning_and_Optimizing_Red_Hat_Enterprise_Linux_for_Oracle_9i_and_10g_Databases/sect-Oracle_9i_and_10g_Tuning_Guide-Setting_Semaphores-An_Example_of_Semaphore_Settings.html 
recommends to set it to SEMMSL, i.e. 5000 in the given example.

> - 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.