[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@xxxxxxxxxxxxxxxxxxxx>
Link: https://lore.kernel.org/r/20261016155905.2a659b7bb7d61f33467a3263@xxxxxxxxxxxxxxxxxxxx
Link: https://lore.kernel.org/r/51c93c19-f1dd-455f-8698-6b1ee30eb630@xxxxxxxxxxxxxxxx
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Yingjie Wang <x-professor@xxxxxxxxxxx>
---
 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