mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ipc/msg: prevent msg_qbytes limit bypass in IPC_SET
@ 2026-10-28  3:35 Shubhang Kaushik (Ampere)
  2026-10-28  4:26 ` Andrew Morton
  0 siblings, 1 reply; 2+ messages in thread
From: Shubhang Kaushik (Ampere) @ 2026-10-28  3:35 UTC (permalink / raw)
  To: Shuah Khan, Lu Shuaibing, Andrew Morton, Shubhang Kaushik,
	Christopher Lameter (Ampere)
  Cc: linux-kernel, linux-kselftest, stable, Shubhang Kaushik (Ampere)

The msg_qbytes field in the msqid64_ds userspace ABI is an unsigned
long. Commit 889b331724c82 ("ipc/msg.c: consolidate all xxxctl_down()
functions") passed that value to msgctl_down() as an int.

An unprivileged owner can set msg_qbytes to UINT_MAX. The narrowing
conversion produces -1 on supported Linux targets, so the check against
the int-valued msg_ctlmnb sysctl does not reject the request:

        if (msg_qbytes > ns->msg_ctlmnb &&
            !capable(CAP_SYS_RESOURCE))

The value is then assigned to the unsigned long q_qbytes field, setting
it to ULONG_MAX. This bypasses the CAP_SYS_RESOURCE requirement for
increasing a queue above MSGMNB and lets an unprivileged user increase
queue capacity without the configured per-queue limit.

Keep msg_qbytes as unsigned long through validation and assignment.
Also preserve its type in ipcctl_obtain_check(), which supplies it to
the unsigned-long audit field.

Add a regression test which drops CAP_SYS_RESOURCE and verifies that
IPC_SET with UINT_MAX fails with EPERM.

Fixes: 889b331724c82 ("ipc/msg.c: consolidate all xxxctl_down() functions")
Cc: stable@vger.kernel.org
Signed-off-by: Shubhang Kaushik (Ampere) <sh@gentwo.org>
---
 ipc/msg.c                                   |  2 +-
 ipc/util.c                                  |  3 +-
 ipc/util.h                                  |  3 +-
 tools/testing/selftests/ipc/Makefile        |  2 +-
 tools/testing/selftests/ipc/msgctl_qbytes.c | 75 +++++++++++++++++++++++++++++
 5 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/ipc/msg.c b/ipc/msg.c
index 62996b97f0ac2137dfc2fa109394d6db1fcb4a0f..116126f1235cd6ce9b819cfd317ef039071b95ca 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -399,7 +399,7 @@ copy_msqid_from_user(struct msqid64_ds *out, void __user *buf, int version)
  * NOTE: no locks must be held, the rwsem is taken inside this function.
  */
 static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd,
-			struct ipc64_perm *perm, int msg_qbytes)
+			struct ipc64_perm *perm, unsigned long msg_qbytes)
 {
 	struct kern_ipc_perm *ipcp;
 	struct msg_queue *msq;
diff --git a/ipc/util.c b/ipc/util.c
index 1737d776bc089e27495e810569bf302069db3712..c435b2d93a12e92e930358a8bc53a06314d065f5 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -718,7 +718,8 @@ int ipc_update_perm(struct ipc64_perm *in, struct kern_ipc_perm *out)
  */
 struct kern_ipc_perm *ipcctl_obtain_check(struct ipc_namespace *ns,
 					struct ipc_ids *ids, int id, int cmd,
-					struct ipc64_perm *perm, int extra_perm)
+					struct ipc64_perm *perm,
+					unsigned long extra_perm)
 {
 	kuid_t euid;
 	int err = -EPERM;
diff --git a/ipc/util.h b/ipc/util.h
index a55d6cebe6d3b0974548a497fcb889248aed0f87..55a76f94667cd4bb248fbcea26323db8d2778a44 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -180,7 +180,8 @@ void ipc64_perm_to_ipc_perm(struct ipc64_perm *in, struct ipc_perm *out);
 int ipc_update_perm(struct ipc64_perm *in, struct kern_ipc_perm *out);
 struct kern_ipc_perm *ipcctl_obtain_check(struct ipc_namespace *ns,
 					     struct ipc_ids *ids, int id, int cmd,
-					     struct ipc64_perm *perm, int extra_perm);
+					     struct ipc64_perm *perm,
+					     unsigned long extra_perm);
 
 static inline void ipc_update_pid(struct pid **pos, struct pid *pid)
 {
diff --git a/tools/testing/selftests/ipc/Makefile b/tools/testing/selftests/ipc/Makefile
index fad10f2bb57b0f06f0c741fa89f0489f6952eba6..930a334747a6353c86bebe1f4a521aeee3fe8228 100644
--- a/tools/testing/selftests/ipc/Makefile
+++ b/tools/testing/selftests/ipc/Makefile
@@ -12,7 +12,7 @@ endif
 
 CFLAGS += $(KHDR_INCLUDES)
 
-TEST_GEN_PROGS := msgque
+TEST_GEN_PROGS := msgctl_qbytes msgque
 
 include ../lib.mk
 
diff --git a/tools/testing/selftests/ipc/msgctl_qbytes.c b/tools/testing/selftests/ipc/msgctl_qbytes.c
new file mode 100644
index 0000000000000000000000000000000000000000..fdf3420173de99ece66c69ff8418a46878d56dbd
--- /dev/null
+++ b/tools/testing/selftests/ipc/msgctl_qbytes.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <errno.h>
+#include <limits.h>
+#include <linux/capability.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/msg.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+#include "kselftest.h"
+
+static int drop_cap_sys_resource(void)
+{
+	struct __user_cap_header_struct hdr = {
+		.version = _LINUX_CAPABILITY_VERSION_3,
+	};
+	struct __user_cap_data_struct data[_LINUX_CAPABILITY_U32S_3];
+	unsigned int index = CAP_SYS_RESOURCE / 32;
+	__u32 mask = 1U << (CAP_SYS_RESOURCE % 32);
+
+	if (syscall(SYS_capget, &hdr, data))
+		return -1;
+
+	data[index].effective &= ~mask;
+	data[index].permitted &= ~mask;
+
+	return syscall(SYS_capset, &hdr, data);
+}
+
+int main(void)
+{
+	struct msqid_ds ds;
+	int msqid;
+	int ret;
+
+	ksft_print_header();
+	ksft_set_plan(1);
+
+	if (drop_cap_sys_resource())
+		ksft_exit_fail_perror("drop CAP_SYS_RESOURCE");
+
+	msqid = msgget(IPC_PRIVATE, 0600);
+	if (msqid == -1)
+		ksft_exit_fail_perror("msgget");
+
+	ret = msgctl(msqid, IPC_STAT, &ds);
+	if (ret == -1) {
+		ksft_perror("msgctl(IPC_STAT)");
+		goto out_fail;
+	}
+
+	/*
+	 * msg_ctlmnb is an int, so UINT_MAX must require
+	 * CAP_SYS_RESOURCE.
+	 */
+	ds.msg_qbytes = UINT_MAX;
+	errno = 0;
+	ret = msgctl(msqid, IPC_SET, &ds);
+	if (ret == -1 && errno == EPERM) {
+		ksft_test_result_pass("IPC_SET rejects oversized msg_qbytes\n");
+		msgctl(msqid, IPC_RMID, NULL);
+		ksft_exit_pass();
+	}
+
+	if (ret == -1)
+		ksft_perror("msgctl(IPC_SET)");
+	else
+		ksft_print_msg("msgctl(IPC_SET) unexpectedly succeeded\n");
+
+out_fail:
+	msgctl(msqid, IPC_RMID, NULL);
+	ksft_test_result_fail("IPC_SET accepted oversized msg_qbytes\n");
+	ksft_exit_fail();
+}

---
base-commit: 8a36f53fd5ec3eff01f1b637d42cbdc3821d0493
change-id: 20261027-ipc-msg-qbytes-49b15931c5b6

Best regards,
-- 
Shubhang Kaushik (Ampere) <sh@gentwo.org>


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

* Re: [PATCH] ipc/msg: prevent msg_qbytes limit bypass in IPC_SET
  2026-10-28  3:35 [PATCH] ipc/msg: prevent msg_qbytes limit bypass in IPC_SET Shubhang Kaushik (Ampere)
@ 2026-10-28  4:26 ` Andrew Morton
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Morton @ 2026-10-28  4:26 UTC (permalink / raw)
  To: Shubhang Kaushik (Ampere)
  Cc: Shuah Khan, Lu Shuaibing, Shubhang Kaushik,
	Christopher Lameter (Ampere),
	linux-kernel, linux-kselftest, stable, Manfred Spraul,
	Davidlohr Bueso, Koester

On Tue, 27 Oct 2026 20:35:00 -0700 "Shubhang Kaushik (Ampere)" <sh@gentwo.org> wrote:

> The msg_qbytes field in the msqid64_ds userspace ABI is an unsigned
> long. Commit 889b331724c82 ("ipc/msg.c: consolidate all xxxctl_down()
> functions") passed that value to msgctl_down() as an int.
> 
> An unprivileged owner can set msg_qbytes to UINT_MAX. The narrowing
> conversion produces -1 on supported Linux targets, so the check against
> the int-valued msg_ctlmnb sysctl does not reject the request:
> 
>         if (msg_qbytes > ns->msg_ctlmnb &&
>             !capable(CAP_SYS_RESOURCE))
> 
> The value is then assigned to the unsigned long q_qbytes field, setting
> it to ULONG_MAX. This bypasses the CAP_SYS_RESOURCE requirement for
> increasing a queue above MSGMNB and lets an unprivileged user increase
> queue capacity without the configured per-queue limit.
> 
> Keep msg_qbytes as unsigned long through validation and assignment.
> Also preserve its type in ipcctl_obtain_check(), which supplies it to
> the unsigned-long audit field.
> 
> Add a regression test which drops CAP_SYS_RESOURCE and verifies that
> IPC_SET with UINT_MAX fails with EPERM.

Thanks.

Manfred and Davidlohr help out with ipc/, although MAINTAINERS keeps
that secret (I'm working on changing this).

My AI sidekick said:

: The kernel change looks right. One small selftest fix: please report
: SKIP for ENOSYS from either capability setup or msgget(). With
: CONFIG_MULTIUSER=n, capget/capset are unavailable and the unprivileged
: case cannot be exercised. CONFIG_SYSVIPC=n makes msgget unavailable.
: Other setup and creation errors should still fail.
: 
: Please mention the practical consequence in the changelog: an
: unprivileged queue owner can bypass both the byte limit and the queued
: message-count limit. Even zero-length messages consume kernel memory.
: Subsequent sends can cause memory pressure or exhaustion beyond the
: configured per-queue allowance, subject to the other resource limits.
: 
: The Fixes tag is correct and this warrants stable. Don't add a v5.6+
: restriction: the introducing change was backported, and the bug is
: present in v5.4.19 and v4.19.103 too.
: 
: I found no second confirmed capacity bypass in the adjacent IPC paths.
: Jan Koester's "[RFC PATCH v2 30/41] ipc: add IPC_STAT_R and IPC_SET_R"
: touches these same signatures and retains int qbytes. Please preserve
: this unsigned-long correction when the two changes are integrated.
: 
: https://lists.openwall.net/linux-kernel/2026/10/21/347

> Fixes: 889b331724c82 ("ipc/msg.c: consolidate all xxxctl_down() functions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shubhang Kaushik (Ampere) <sh@gentwo.org>
> ---
>  ipc/msg.c                                   |  2 +-
>  ipc/util.c                                  |  3 +-
>  ipc/util.h                                  |  3 +-
>  tools/testing/selftests/ipc/Makefile        |  2 +-
>  tools/testing/selftests/ipc/msgctl_qbytes.c | 75 +++++++++++++++++++++++++++++
>  5 files changed, 81 insertions(+), 4 deletions(-)
> 
> diff --git a/ipc/msg.c b/ipc/msg.c
> index 62996b97f0ac2137dfc2fa109394d6db1fcb4a0f..116126f1235cd6ce9b819cfd317ef039071b95ca 100644
> --- a/ipc/msg.c
> +++ b/ipc/msg.c
> @@ -399,7 +399,7 @@ copy_msqid_from_user(struct msqid64_ds *out, void __user *buf, int version)
>   * NOTE: no locks must be held, the rwsem is taken inside this function.
>   */
>  static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd,
> -			struct ipc64_perm *perm, int msg_qbytes)
> +			struct ipc64_perm *perm, unsigned long msg_qbytes)
>  {
>  	struct kern_ipc_perm *ipcp;
>  	struct msg_queue *msq;
> diff --git a/ipc/util.c b/ipc/util.c
> index 1737d776bc089e27495e810569bf302069db3712..c435b2d93a12e92e930358a8bc53a06314d065f5 100644
> --- a/ipc/util.c
> +++ b/ipc/util.c
> @@ -718,7 +718,8 @@ int ipc_update_perm(struct ipc64_perm *in, struct kern_ipc_perm *out)
>   */
>  struct kern_ipc_perm *ipcctl_obtain_check(struct ipc_namespace *ns,
>  					struct ipc_ids *ids, int id, int cmd,
> -					struct ipc64_perm *perm, int extra_perm)
> +					struct ipc64_perm *perm,
> +					unsigned long extra_perm)
>  {
>  	kuid_t euid;
>  	int err = -EPERM;
> diff --git a/ipc/util.h b/ipc/util.h
> index a55d6cebe6d3b0974548a497fcb889248aed0f87..55a76f94667cd4bb248fbcea26323db8d2778a44 100644
> --- a/ipc/util.h
> +++ b/ipc/util.h
> @@ -180,7 +180,8 @@ void ipc64_perm_to_ipc_perm(struct ipc64_perm *in, struct ipc_perm *out);
>  int ipc_update_perm(struct ipc64_perm *in, struct kern_ipc_perm *out);
>  struct kern_ipc_perm *ipcctl_obtain_check(struct ipc_namespace *ns,
>  					     struct ipc_ids *ids, int id, int cmd,
> -					     struct ipc64_perm *perm, int extra_perm);
> +					     struct ipc64_perm *perm,
> +					     unsigned long extra_perm);
>  
>  static inline void ipc_update_pid(struct pid **pos, struct pid *pid)
>  {
> diff --git a/tools/testing/selftests/ipc/Makefile b/tools/testing/selftests/ipc/Makefile
> index fad10f2bb57b0f06f0c741fa89f0489f6952eba6..930a334747a6353c86bebe1f4a521aeee3fe8228 100644
> --- a/tools/testing/selftests/ipc/Makefile
> +++ b/tools/testing/selftests/ipc/Makefile
> @@ -12,7 +12,7 @@ endif
>  
>  CFLAGS += $(KHDR_INCLUDES)
>  
> -TEST_GEN_PROGS := msgque
> +TEST_GEN_PROGS := msgctl_qbytes msgque
>  
>  include ../lib.mk
>  
> diff --git a/tools/testing/selftests/ipc/msgctl_qbytes.c b/tools/testing/selftests/ipc/msgctl_qbytes.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..fdf3420173de99ece66c69ff8418a46878d56dbd
> --- /dev/null
> +++ b/tools/testing/selftests/ipc/msgctl_qbytes.c
> @@ -0,0 +1,75 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <errno.h>
> +#include <limits.h>
> +#include <linux/capability.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/msg.h>
> +#include <sys/syscall.h>
> +#include <unistd.h>
> +
> +#include "kselftest.h"
> +
> +static int drop_cap_sys_resource(void)
> +{
> +	struct __user_cap_header_struct hdr = {
> +		.version = _LINUX_CAPABILITY_VERSION_3,
> +	};
> +	struct __user_cap_data_struct data[_LINUX_CAPABILITY_U32S_3];
> +	unsigned int index = CAP_SYS_RESOURCE / 32;
> +	__u32 mask = 1U << (CAP_SYS_RESOURCE % 32);
> +
> +	if (syscall(SYS_capget, &hdr, data))
> +		return -1;
> +
> +	data[index].effective &= ~mask;
> +	data[index].permitted &= ~mask;
> +
> +	return syscall(SYS_capset, &hdr, data);
> +}
> +
> +int main(void)
> +{
> +	struct msqid_ds ds;
> +	int msqid;
> +	int ret;
> +
> +	ksft_print_header();
> +	ksft_set_plan(1);
> +
> +	if (drop_cap_sys_resource())
> +		ksft_exit_fail_perror("drop CAP_SYS_RESOURCE");
> +
> +	msqid = msgget(IPC_PRIVATE, 0600);
> +	if (msqid == -1)
> +		ksft_exit_fail_perror("msgget");
> +
> +	ret = msgctl(msqid, IPC_STAT, &ds);
> +	if (ret == -1) {
> +		ksft_perror("msgctl(IPC_STAT)");
> +		goto out_fail;
> +	}
> +
> +	/*
> +	 * msg_ctlmnb is an int, so UINT_MAX must require
> +	 * CAP_SYS_RESOURCE.
> +	 */
> +	ds.msg_qbytes = UINT_MAX;
> +	errno = 0;
> +	ret = msgctl(msqid, IPC_SET, &ds);
> +	if (ret == -1 && errno == EPERM) {
> +		ksft_test_result_pass("IPC_SET rejects oversized msg_qbytes\n");
> +		msgctl(msqid, IPC_RMID, NULL);
> +		ksft_exit_pass();
> +	}
> +
> +	if (ret == -1)
> +		ksft_perror("msgctl(IPC_SET)");
> +	else
> +		ksft_print_msg("msgctl(IPC_SET) unexpectedly succeeded\n");
> +
> +out_fail:
> +	msgctl(msqid, IPC_RMID, NULL);
> +	ksft_test_result_fail("IPC_SET accepted oversized msg_qbytes\n");
> +	ksft_exit_fail();
> +}
> 
> ---
> base-commit: 8a36f53fd5ec3eff01f1b637d42cbdc3821d0493
> change-id: 20261027-ipc-msg-qbytes-49b15931c5b6
> 
> Best regards,
> -- 
> Shubhang Kaushik (Ampere) <sh@gentwo.org>

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

end of thread, other threads:[~2026-10-28  4:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28  3:35 [PATCH] ipc/msg: prevent msg_qbytes limit bypass in IPC_SET Shubhang Kaushik (Ampere)
2026-10-28  4:26 ` 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®