[PATCH v2 0/3] ipc/sem: account enlarged sops arrays and validate SEMOPM

[PATCH v2 0/3] ipc/sem: account enlarged sops arrays and validate SEMOPM

[Yingjie Wang]

Andrew, Manfred, thanks for the review.

This revision keeps the memcg accounting fix independently applicable,
makes the SEMOPM validation safe under concurrent sysctl writers, and moves
the UAPI comment cleanup to a separate follow-up patch.

Patch 1 charges operation arrays to the caller's memory cgroup only when
the actual nsops exceeds the compiled-in SEMOPM default. Requests within
the default limit retain the existing allocation behavior.

Patch 2 rejects negative SEMOPM values. Writes are parsed and validated in
temporary storage while a per-IPC-namespace mutex serializes writers from
the initial snapshot through publication. Shared settings are accessed
with READ_ONCE() and WRITE_ONCE(). Failed writes leave the live settings
unchanged, and a partial write cannot restore fields from a stale snapshot.
Readers retain the existing per-field visibility.

Patch 3 removes the obsolete UAPI warning about physical fragmentation for
SEMOPM values above 1000. The operation array has used kvmalloc() since
commit e4243b8062c1 ("ipc/sem: play nicer with large nsops allocations").

The series is based on 7be4bc52fd9e ("Merge tag 'driver-core-7.4-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core").

Validation completed:

- W=1 builds of the changed objects and full x86-64 bzImage builds for
  patch 1 alone and the final series;
- a W=1 i386 build of ipc/sem.o, ipc/ipc_sysctl.o, and ipc/namespace.o;
- initial and user-owned non-initial IPC namespaces rejected negative
  SEMOPM, invalid SEMMNI, and malformed writes without changing live
  settings, accepted zero and positive SEMOPM values, and enforced the
  configured operation-count boundary;
- both namespaces completed 10,000 rounds of concurrent partial and full
  writes and 10,000 rounds of concurrent rejected and successful writes;
  the same test reproduced the stale-snapshot overwrite on the superseded
  implementation in round 1;
- SEMOPM limits of 500 and 501 accepted requests at their exact boundary
  and rejected the immediately larger request;
- allocation profiling returned to its baseline after successful semop(),
  EAGAIN, EFAULT, signal interruption, and IPC_RMID wakeup paths;
- with patch 1 alone, a test using sixteen 1,000,000-operation calls in a
  32 MiB memory cgroup observed memory.peak reach 33,554,432 bytes, one
  memcg OOM kill, and allocation profiling return to its baseline after
  the caller exited.

Strict checkpatch reported only the CONST_STRUCT warning for the local
ctl_table copy in patch 2. That copy must remain writable because its data
pointer is redirected to the temporary array.

Changes since v1:

- account only requests whose actual nsops exceeds the compiled-in SEMOPM
  default and place this independently applicable fix first;
- reject negative SEMOPM values before changing live settings;
- serialize kernel.sem writers across snapshot, parsing, validation, and
  publication, while preserving per-field READ_ONCE()/WRITE_ONCE() access;
- attribute the long-standing signed/unsigned limit bug to the initial Git
  commit and retain commit 50ec499b9a43 as the reachability explanation;
- move the UAPI comment cleanup into a separate follow-up patch;
- add concurrent-writer, accounting-boundary, cleanup-path, and i386 build
  validation.

An AI coding assistant assisted with implementation review, commit-message
drafting, and test planning.

V1:
https://lore.kernel.org/r/20261016152609.2528805-1-x-professor@outlook.com

Andrew's review:
https://lore.kernel.org/r/20261016155905.2a659b7bb7d61f33467a3263@linux-foundation.org

Manfred's review:
https://lore.kernel.org/r/51c93c19-f1dd-455f-8698-6b1ee30eb630@colorfullife.com

Yingjie Wang (3):
  ipc/sem: account enlarged sops arrays to memcg
  ipc/sem: reject negative SEMOPM values
  ipc/sem: remove obsolete SEMOPM fragmentation warning

 include/linux/ipc_namespace.h |  3 +++
 include/uapi/linux/sem.h      |  7 +++----
 ipc/ipc_sysctl.c              | 38 ++++++++++++++++++++++++++---------
 ipc/sem.c                     |  5 ++++-
 ipc/util.h                    |  7 +++----
 5 files changed, 42 insertions(+), 18 deletions(-)


base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614
-- 
2.43.0

[PATCH v2 1/3] ipc/sem: account enlarged sops arrays to memcg

[Yingjie Wang]

Commit 6a4746ba0619 ("ipc: remove memcg accounting for sops objects in
do_semtimedop()") removed accounting based on the assumption that sops
arrays would remain small, describing a maximum cost of two pages.

SEMOPM was already tunable. Commit 50ec499b9a43 ("sysctl: allow change
system v ipc sysctls inside ipc namespace") later made that tuning
available to IPC namespace owners without capabilities in the initial
user namespace. A blocking semop() retains its operation array, so
raising SEMOPM permits enlarged arrays to consume user-controlled kernel
memory outside the calling task's memory cgroup.

Use GFP_KERNEL_ACCOUNT when the actual nsops exceeds the compiled-in
SEMOPM default. This preserves the existing allocation behavior for
requests within the default limit while accounting the enlarged arrays.

Link: https://lore.kernel.org/r/20261016152609.2528805-1-x-professor@outlook.com
Fixes: 6a4746ba0619 ("ipc: remove memcg accounting for sops objects in do_semtimedop()")
Cc: stable@vger.kernel.org
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 5ec41de7e85b2..8c9edecc16076 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;
 	}
-- 
2.43.0

[PATCH v2 2/3] ipc/sem: reject negative SEMOPM values

[Yingjie Wang]

The SEMOPM field has been stored as an int while semop() receives an
unsigned operation count since the beginning of Git history. A negative
SEMOPM accepted by proc_dointvec() is converted to a large unsigned value
in the comparison, defeating the intended operation-count limit.

Commit 50ec499b9a43 ("sysctl: allow change system v ipc sysctls inside ipc
namespace") made kernel.sem writable by IPC namespace owners without
capabilities in the initial user namespace.

Parse writes against a temporary copy and validate SEMOPM and SEMMNI
before changing the live settings. Hold a per-namespace mutex from the
snapshot through publication so concurrent sysctl writers cannot publish
stale values for fields they did not supply. Access the live fields with
READ_ONCE() and WRITE_ONCE(), preserving the per-field access semantics of
proc_dointvec(). Read requests continue to use proc_dointvec() directly.

Failed writes leave the live settings unchanged. Successful partial writes
change only the supplied fields relative to the latest locked snapshot.
The writer mutex retains the existing per-field visibility for readers.

Keep zero and positive SEMOPM values valid. Zero continues to reject every
non-empty semop() request.

Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Link: https://lore.kernel.org/r/20261016155905.2a659b7bb7d61f33467a3263@linux-foundation.org
Link: https://lore.kernel.org/r/51c93c19-f1dd-455f-8698-6b1ee30eb630@colorfullife.com
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 include/linux/ipc_namespace.h |  3 +++
 ipc/ipc_sysctl.c              | 38 ++++++++++++++++++++++++++---------
 ipc/sem.c                     |  1 +
 ipc/util.h                    |  7 +++----
 4 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index 12faca29bbb9c..c06568ccfca88 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -4,6 +4,7 @@
 
 #include <linux/err.h>
 #include <linux/idr.h>
+#include <linux/mutex.h>
 #include <linux/rwsem.h>
 #include <linux/notifier.h>
 #include <linux/nsproxy.h>
@@ -31,6 +32,8 @@ struct ipc_ids {
 struct ipc_namespace {
 	struct ipc_ids	ids[3];
 
+	/* Serialize kernel.sem writers. */
+	struct mutex	sem_ctls_mutex;
 	int		sem_ctls[4];
 	int		used_sems;
 
diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
index d038d944257f6..2a542f838a530 100644
--- a/ipc/ipc_sysctl.c
+++ b/ipc/ipc_sysctl.c
@@ -51,21 +51,41 @@ static int proc_ipc_auto_msgmni(const struct ctl_table *table, int write,
 static int proc_ipc_sem_dointvec(const struct ctl_table *table, int write,
 	void *buffer, size_t *lenp, loff_t *ppos)
 {
+	struct ctl_table ipc_table;
 	struct ipc_namespace *ns =
 		container_of(table->data, struct ipc_namespace, sem_ctls);
-	int ret, semmni;
+	int sem_ctls[ARRAY_SIZE(ns->sem_ctls)];
+	size_t i;
+	int ret;
+
+	if (!write)
+		return proc_dointvec(table, write, buffer, lenp, ppos);
 
-	semmni = ns->sem_ctls[3];
-	ret = proc_dointvec(table, write, buffer, lenp, ppos);
+	mutex_lock(&ns->sem_ctls_mutex);
+	for (i = 0; i < ARRAY_SIZE(sem_ctls); i++)
+		sem_ctls[i] = READ_ONCE(ns->sem_ctls[i]);
 
-	if (!ret)
-		ret = sem_check_semmni(ns);
+	memcpy(&ipc_table, table, sizeof(ipc_table));
+	ipc_table.data = sem_ctls;
 
-	/*
-	 * Reset the semmni value if an error happens.
-	 */
+	ret = proc_dointvec(&ipc_table, write, buffer, lenp, ppos);
 	if (ret)
-		ns->sem_ctls[3] = semmni;
+		goto out_unlock;
+
+	if (sem_ctls[2] < 0) {
+		ret = -ERANGE;
+		goto out_unlock;
+	}
+
+	ret = sem_check_semmni(sem_ctls[3]);
+	if (ret)
+		goto out_unlock;
+
+	for (i = 0; i < ARRAY_SIZE(sem_ctls); i++)
+		WRITE_ONCE(ns->sem_ctls[i], sem_ctls[i]);
+
+out_unlock:
+	mutex_unlock(&ns->sem_ctls_mutex);
 	return ret;
 }
 
diff --git a/ipc/sem.c b/ipc/sem.c
index 8c9edecc16076..019a736b79316 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -248,6 +248,7 @@ static int sysvipc_sem_proc_show(struct seq_file *s, void *it);
 
 void sem_init_ns(struct ipc_namespace *ns)
 {
+	mutex_init(&ns->sem_ctls_mutex);
 	ns->sc_semmsl = SEMMSL;
 	ns->sc_semmns = SEMMNS;
 	ns->sc_semopm = SEMOPM;
diff --git a/ipc/util.h b/ipc/util.h
index a55d6cebe6d3b..b148b5e15fc6e 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -245,13 +245,12 @@ int ipcget(struct ipc_namespace *ns, struct ipc_ids *ids,
 void free_ipcs(struct ipc_namespace *ns, struct ipc_ids *ids,
 		void (*free)(struct ipc_namespace *, struct kern_ipc_perm *));
 
-static inline int sem_check_semmni(struct ipc_namespace *ns) {
+static inline int sem_check_semmni(int semmni)
+{
 	/*
 	 * Check semmni range [0, ipc_mni]
-	 * semmni is the last element of sem_ctls[4] array
 	 */
-	return ((ns->sem_ctls[3] < 0) || (ns->sem_ctls[3] > ipc_mni))
-		? -ERANGE : 0;
+	return semmni < 0 || semmni > ipc_mni ? -ERANGE : 0;
 }
 
 #ifdef CONFIG_COMPAT
-- 
2.43.0

[PATCH v2 3/3] ipc/sem: remove obsolete SEMOPM fragmentation warning

[Yingjie Wang]

The UAPI comment advises keeping SEMOPM at or below 1000 because a larger
operation array may fail due to physical memory fragmentation.

Commit e4243b8062c1 ("ipc/sem: play nicer with large nsops allocations")
changed the operation array to kvmalloc() specifically to support larger
configured limits, with vmalloc fallback when a physically contiguous
allocation is unavailable.

Replace the obsolete warning with a description of the compiled-in
default and the runtime sysctl.

Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 include/uapi/linux/sem.h | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/sem.h b/include/uapi/linux/sem.h
index 75aa3b273cd99..cfd772f4c3396 100644
--- a/include/uapi/linux/sem.h
+++ b/include/uapi/linux/sem.h
@@ -71,16 +71,15 @@ struct  seminfo {
  * The values has been chosen to be larger than necessary for any
  * known configuration.
  *
- * SEMOPM should not be increased beyond 1000, otherwise there is the
- * risk that semop()/semtimedop() fails due to kernel memory fragmentation when
- * allocating the sop array.
+ * SEMOPM is the default maximum number of operations per semop() call. The
+ * runtime limit can be changed through /proc/sys/kernel/sem.
  */
 
 
 #define SEMMNI  32000           /* <= IPCMNI  max # of semaphore identifiers */
 #define SEMMSL  32000           /* <= INT_MAX max num of semaphores per id */
 #define SEMMNS  (SEMMNI*SEMMSL) /* <= INT_MAX max # of semaphores in system */
-#define SEMOPM  500	        /* <= 1 000 max num of ops per semop call */
+#define SEMOPM  500	        /* default max num of ops per semop call */
 #define SEMVMX  32767           /* <= 32767 semaphore maximum value */
 #define SEMAEM  SEMVMX          /* adjust on exit max value */
 
-- 
2.43.0