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) --- 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 +#include +#include +#include +#include +#include +#include +#include + +#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)