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

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

[Yingjie Wang]

Andrew, Manfred, thanks for the review.

This version separates the accounting threshold from the UAPI default.
SEMOPM can be changed when rebuilding the kernel, so it should not also
determine which operation arrays are accounted. The new internal
SEMOPM_NOACCOUNT threshold is fixed at 1000 operations. Allocations above
it use GFP_KERNEL_ACCOUNT; requests at or below it keep their existing
behavior.

The sysctl handler now parses writes into a temporary array while holding a
per-IPC-namespace mutex. The mutex covers the snapshot, validation, and
publication, so a failed write leaves the live settings unchanged and a
partial write cannot restore fields from an older snapshot. The snapshot and
publication use READ_ONCE() and WRITE_ONCE(), and reads continue through
proc_dointvec(). The unused sem_check_semmni() helper has also been removed.

The UAPI warning about physical fragmentation is obsolete now that the
operation array uses kvmalloc(). The header now describes SEMOPM as the
compiled-in default and retains the uniform <= INT_MAX range comment.

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 a full x86-64 bzImage build;
- 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;
- zero and positive SEMOPM values remained valid, with the configured
  operation-count boundary enforced;
- the operation-count boundary was checked at 1000 and 1001 operations;
- concurrent partial, full, rejected, and successful sysctl writes completed
  without stale-snapshot overwrites;
- successful, EAGAIN, EFAULT, signal-interruption, and IPC_RMID paths released
  operation-array allocations;
- a constrained memcg run with enlarged operation arrays entered the
  GFP_KERNEL_ACCOUNT limit path and returned allocation profiling to baseline;
- strict checkpatch (with the intentional CONST_STRUCT diagnostic ignored)
  and patch application checks passed.

Changes since v2:

- use a fixed internal SEMOPM_NOACCOUNT=1000 threshold instead of the
  rebuildable UAPI SEMOPM value for the accounting decision;
- remove sem_check_semmni() and inline the SEMMNI and SEMOPM range checks;
- restore the uniform <= INT_MAX SEMOPM comment form in the UAPI header;
- retain Manfred Spraul's Reviewed-by tags on the two patches revised
  according to his comments.

V2:
https://www.spinics.net/lists/kernel/msg6527052.html

Manfred's v2 review of 2/3:
https://www.spinics.net/lists/kernel/msg6528247.html

Manfred's v2 review of 3/3:
https://www.spinics.net/lists/kernel/msg6528679.html

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

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              | 35 ++++++++++++++++++++++++++---------
 ipc/sem.c                     |  7 ++++++-
 ipc/util.h                    |  9 ---------
 5 files changed, 38 insertions(+), 23 deletions(-)


base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614
-- 
2.43.0

[PATCH v3 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 is a UAPI default that can be changed when rebuilding the kernel.
It must not also define the accounting policy, since changing that default
would enlarge the unaccounted range.

Keep the historical small-array range unaccounted with a fixed internal
threshold of 1000 operations, and use GFP_KERNEL_ACCOUNT when the actual
nsops exceeds it. Requests within that range retain the existing allocation
behavior, while larger user-controlled arrays are charged to the caller's
memory cgroup.

Commit 50ec499b9a43 ("sysctl: allow change system v ipc sysctls inside ipc
namespace") later made the per-namespace limit tunable by IPC namespace
owners without capabilities in the initial user namespace.

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 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ipc/sem.c b/ipc/sem.c
index 5ec41de7e85b2..a8c9807175520 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -176,6 +176,7 @@ static int sysvipc_sem_proc_show(struct seq_file *s, void *it);
 
 #define SEMMSL_FAST	256 /* 512 bytes on stack */
 #define SEMOPM_FAST	64  /* ~ 372 bytes on stack */
+#define SEMOPM_NOACCOUNT	1000 /* ~ 2 pages for struct sembuf */
 
 /*
  * Switching from the mode suitable for simple ops
@@ -2234,7 +2235,10 @@ 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_NOACCOUNT ?
+			GFP_KERNEL_ACCOUNT : GFP_KERNEL;
+
+		sops = kvmalloc_objs(*sops, nsops, gfp);
 		if (sops == NULL)
 			return -ENOMEM;
 	}
-- 
2.43.0

[PATCH v3 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>
Reviewed-by: Manfred Spraul <manfred@colorfullife.com>
---
 include/linux/ipc_namespace.h |  3 +++
 ipc/ipc_sysctl.c              | 35 ++++++++++++++++++++++++++---------
 ipc/sem.c                     |  1 +
 ipc/util.h                    |  9 ---------
 4 files changed, 30 insertions(+), 18 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..a05e28e67c305 100644
--- a/ipc/ipc_sysctl.c
+++ b/ipc/ipc_sysctl.c
@@ -51,21 +51,38 @@ 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 ||
+	    sem_ctls[3] < 0 || sem_ctls[3] > ipc_mni) {
+		ret = -ERANGE;
+		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 a8c9807175520..0f7d32c12993f 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -249,6 +249,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..51b4efa3d6db0 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -245,15 +245,6 @@ 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) {
-	/*
-	 * 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;
-}
-
 #ifdef CONFIG_COMPAT
 #include <linux/compat.h>
 struct compat_ipc_perm {
-- 
2.43.0

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

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 6187 bytes --]

Hi Yingjie,

> Date: 2026-10-24 12:21:42+0000
> From: Yingjie Wang <x-professor@outlook.com>
>
> 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>
> Reviewed-by: Manfred Spraul <manfred@colorfullife.com>
> ---
>  include/linux/ipc_namespace.h |  3 +++
>  ipc/ipc_sysctl.c              | 35 ++++++++++++++++++++++++++---------
>  ipc/sem.c                     |  1 +
>  ipc/util.h                    |  9 ---------
>  4 files changed, 30 insertions(+), 18 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..a05e28e67c305 100644
> --- a/ipc/ipc_sysctl.c
> +++ b/ipc/ipc_sysctl.c
> @@ -51,21 +51,38 @@ 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));

Why memcpy(3)?  Being a structure, we could use simple assignment, which
is type safe.

	ipc_table = *table;

memcpy(3) is way too powerful for this, and that can be dangerous.
A few ways where it is easy to screw a call to memcpy(3) are:

-  Size of destination doesn't match size of input.
-  Size argument doesn't match size of destination.
-  Type of input doesn't match type of destination.
-  You may forget to copy a flexible array member.

In some cases, you really want to defeat those, and then memcpy(3) is
useful (or you're copying an array, and then you have no choice), but if
not, memccpy(3) is not the best tool.

[CC += linux-hardening]


Have a lovely day!
Alex

> +	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 ||
> +	    sem_ctls[3] < 0 || sem_ctls[3] > ipc_mni) {
> +		ret = -ERANGE;
> +		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 a8c9807175520..0f7d32c12993f 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -249,6 +249,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..51b4efa3d6db0 100644
> --- a/ipc/util.h
> +++ b/ipc/util.h
> @@ -245,15 +245,6 @@ 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) {
> -	/*
> -	 * 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;
> -}
> -
>  #ifdef CONFIG_COMPAT
>  #include <linux/compat.h>
>  struct compat_ipc_perm {
> -- 
> 2.43.0
> 

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 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>
Reviewed-by: Manfred Spraul <manfred@colorfullife.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..d9f961a1592c7 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	        /* <= INT_MAX 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