From 768231db3c0851e808b0a35aa5d212dc27082437 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 30 Sep 2026 16:21:16 -0400 Subject: [PATCH] Match Linux message-queue unlink permissions The mqfs root introduced in 61cbf7787 was read-only (0555), so mq_unlink rejected queue owners without DAC override capabilities. Linux instead creates a world-writable sticky directory [1]. Use the same directory mode and enforce sticky ownership checks on the registry unlink path, which bypasses the normal VFS unlink checks. Drop DAC bypass in the existing queue permission tests and check that another unprivileged user cannot unlink the queue. [1]: https://github.com/torvalds/linux/blob/830b3c68c/ipc/mqueue.c#L404-L422 Assisted-by: OpenAI Codex --- pkg/sentry/fsimpl/mqfs/registry.go | 3 +++ pkg/sentry/fsimpl/mqfs/root.go | 2 +- test/syscalls/linux/BUILD | 2 ++ test/syscalls/linux/mq.cc | 27 +++++++++++++++++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/pkg/sentry/fsimpl/mqfs/registry.go b/pkg/sentry/fsimpl/mqfs/registry.go index a8077863674..0a2729f3b9a 100644 --- a/pkg/sentry/fsimpl/mqfs/registry.go +++ b/pkg/sentry/fsimpl/mqfs/registry.go @@ -129,6 +129,9 @@ func (r *RegistryImpl) Unlink(ctx context.Context, name string) error { return err } defer inode.DecRef(ctx) + if err := vfs.CheckDeleteSticky(creds, root.Mode(), root.UID(), inode.UID(), inode.GID()); err != nil { + return err + } return root.Unlink(ctx, name, inode) } diff --git a/pkg/sentry/fsimpl/mqfs/root.go b/pkg/sentry/fsimpl/mqfs/root.go index 06c6a44942b..a50c023b036 100644 --- a/pkg/sentry/fsimpl/mqfs/root.go +++ b/pkg/sentry/fsimpl/mqfs/root.go @@ -46,7 +46,7 @@ var _ kernfs.Inode = (*rootInode)(nil) // newRootInode returns a new, initialized rootInode. func (fs *filesystem) newRootInode(ctx context.Context, creds *auth.Credentials) kernfs.Inode { inode := &rootInode{} - inode.InodeAttrs.Init(ctx, creds, linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.ModeDirectory|linux.FileMode(0555)) + inode.InodeAttrs.Init(ctx, creds, linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.ModeDirectory|linux.ModeSticky|linux.FileMode(0777)) inode.OrderedChildren.Init(kernfs.OrderedChildrenOptions{Writable: true}) inode.InitRefs() return inode diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index c41138e5d1f..f4603efa0d8 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -5185,6 +5185,8 @@ cc_binary( "//test/util:temp_path", "//test/util:test_main", "//test/util:test_util", + "//test/util:thread_util", + "@com_google_absl//absl/flags:flag", "@com_google_absl//absl/strings:str_format", ], ) diff --git a/test/syscalls/linux/mq.cc b/test/syscalls/linux/mq.cc index f02259a0700..ae18ef297c5 100644 --- a/test/syscalls/linux/mq.cc +++ b/test/syscalls/linux/mq.cc @@ -18,14 +18,17 @@ #include #include #include +#include #include #include #include +#include #include #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "absl/flags/flag.h" #include "absl/strings/str_format.h" #include "test/util/capability_util.h" #include "test/util/cleanup.h" @@ -36,9 +39,12 @@ #include "test/util/posix_error.h" #include "test/util/temp_path.h" #include "test/util/test_util.h" +#include "test/util/thread_util.h" #define NAME_MAX 255 +ABSL_FLAG(int32_t, scratch_uid, 65534, "scratch UID"); + namespace gvisor { namespace testing { namespace { @@ -202,8 +208,25 @@ TEST(MqTest, NoQueueExists) { PosixErrorIs(ENOENT)); } +TEST(MqTest, UnlinkOtherUserQueue) { + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SETUID))); + PosixQueue queue = ASSERT_NO_ERRNO_AND_VALUE( + MqOpen(O_RDWR | O_CREAT | O_EXCL, 0600, nullptr)); + + // Change only this thread's credentials so the owner can clean up afterward. + ScopedThread([&] { + AutoCapability fowner(CAP_FOWNER, false); + ASSERT_THAT( + syscall(SYS_setresuid, -1, absl::GetFlag(FLAGS_scratch_uid), -1), + SyscallSucceeds()); + EXPECT_THAT(MqUnlink(queue.name()), PosixErrorIs(EACCES)); + }); +} + // Test trying to re-open a queue with invalid permissions. TEST(MqTest, OpenNoAccess) { + AutoCapability dacOverride(CAP_DAC_OVERRIDE, false); + AutoCapability dacReadSearch(CAP_DAC_READ_SEARCH, false); PosixQueue queue = ASSERT_NO_ERRNO_AND_VALUE( MqOpen(O_RDWR | O_CREAT | O_EXCL, 0000, nullptr)); @@ -214,6 +237,8 @@ TEST(MqTest, OpenNoAccess) { // Test trying to re-open a read-only queue for write. TEST(MqTest, OpenReadAccess) { + AutoCapability dacOverride(CAP_DAC_OVERRIDE, false); + AutoCapability dacReadSearch(CAP_DAC_READ_SEARCH, false); PosixQueue queue = ASSERT_NO_ERRNO_AND_VALUE( MqOpen(O_RDWR | O_CREAT | O_EXCL, 0400, nullptr)); @@ -224,6 +249,8 @@ TEST(MqTest, OpenReadAccess) { // Test trying to re-open a write-only queue for read. TEST(MqTest, OpenWriteAccess) { + AutoCapability dacOverride(CAP_DAC_OVERRIDE, false); + AutoCapability dacReadSearch(CAP_DAC_READ_SEARCH, false); PosixQueue queue = ASSERT_NO_ERRNO_AND_VALUE( MqOpen(O_RDWR | O_CREAT | O_EXCL, 0200, nullptr));