diff --git a/pkg/sentry/kernel/shm/shm.go b/pkg/sentry/kernel/shm/shm.go index bdd81873bd1..aa38b349d4f 100644 --- a/pkg/sentry/kernel/shm/shm.go +++ b/pkg/sentry/kernel/shm/shm.go @@ -647,25 +647,32 @@ func (s *Shm) Set(ctx context.Context, ds *linux.ShmidDS) error { return nil } -// MarkDestroyed marks a segment for destruction. The segment is actually -// destroyed once it has no references. MarkDestroyed may be called multiple -// times, and is safe to call after a segment has already been destroyed. See -// shmctl(IPC_RMID). -func (s *Shm) MarkDestroyed(ctx context.Context) { - s.registry.dissociateKey(s) +// MarkDestroyed marks a segment for destruction. The segment is +// actually destroyed once it has no references. MarkDestroyed may be called +// multiple times, and is safe to call after a segment has already been +// destroyed. See shmctl(IPC_RMID). +func (s *Shm) MarkDestroyed(ctx context.Context) error { + creds := auth.CredentialsFromContext(ctx) s.mu.Lock() + if !s.obj.CheckOwnership(creds) { + s.mu.Unlock() + return linuxerr.EPERM + } if s.pendingDestruction { s.mu.Unlock() - return + return nil } s.pendingDestruction = true s.mu.Unlock() + s.registry.dissociateKey(s) + // Drop the self-reference so destruction occurs when all // external references are gone. // // N.B. This cannot be the final DecRef, as the caller also // holds a reference. s.DecRef(ctx) + return nil } diff --git a/pkg/sentry/syscalls/linux/sys_shm.go b/pkg/sentry/syscalls/linux/sys_shm.go index 801c50a827a..2a7b888e9c4 100644 --- a/pkg/sentry/syscalls/linux/sys_shm.go +++ b/pkg/sentry/syscalls/linux/sys_shm.go @@ -145,8 +145,7 @@ func Shmctl(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr, return 0, nil, err case linux.IPC_RMID: - segment.MarkDestroyed(t) - return 0, nil, nil + return 0, nil, segment.MarkDestroyed(t) case linux.SHM_LOCK, linux.SHM_UNLOCK: // We currently do not support memory locking anywhere. diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 5532f54770f..b70a30a49b9 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -4928,11 +4928,14 @@ cc_binary( linkstatic = 1, malloc = "//test/util:errno_safe_allocator", deps = select_gtest() + [ + "//test/util:capability_util", "//test/util:multiprocess_util", "//test/util:posix_error", "//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/time", ], ) diff --git a/test/syscalls/linux/shm.cc b/test/syscalls/linux/shm.cc index 44f75a4e362..2e3921c037b 100644 --- a/test/syscalls/linux/shm.cc +++ b/test/syscalls/linux/shm.cc @@ -17,14 +17,22 @@ #include #include #include +#include #include +#include #include "gmock/gmock.h" +#include "absl/flags/flag.h" #include "absl/time/clock.h" +#include "test/util/capability_util.h" #include "test/util/multiprocess_util.h" #include "test/util/posix_error.h" #include "test/util/temp_path.h" #include "test/util/test_util.h" +#include "test/util/thread_util.h" + +ABSL_FLAG(int32_t, scratch_uid, 65534, "scratch UID"); +ABSL_FLAG(int32_t, scratch_gid, 65534, "scratch GID"); namespace gvisor { namespace testing { @@ -539,6 +547,49 @@ TEST(ShmTest, MprotectWriteOnWritableSegmentSucceeds) { ASSERT_NO_ERRNO(Shmdt(addr)); } +TEST(ShmTest, RmidOwnershipPermissionDenied) { + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SETUID))); + + const uid_t scratch_uid = absl::GetFlag(FLAGS_scratch_uid); + const gid_t scratch_gid = absl::GetFlag(FLAGS_scratch_gid); + + ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE( + Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0600)); + + // Drop privileges and change IDs in a separate thread so the test runner's + // main thread credentials remain unaffected. AutoCapability must stay in + // scope for the shmctl body. + auto drop_privs = [&](auto&& body) { + AutoCapability cap(CAP_SYS_ADMIN, false); + EXPECT_THAT(syscall(SYS_setresgid, -1, scratch_gid, -1), SyscallSucceeds()); + EXPECT_THAT(syscall(SYS_setresuid, -1, scratch_uid, -1), SyscallSucceeds()); + body(); + }; + + ScopedThread([&] { + drop_privs([&] { + EXPECT_THAT(Shmctl(shm.id(), IPC_RMID, nullptr), + PosixErrorIs(EPERM, _)); + }); + }); + + // Verify that the segment was not destroyed and remains valid. + struct shmid_ds attr; + ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_STAT, &attr)); + EXPECT_EQ(attr.shm_perm.mode & 0777, 0600); + EXPECT_FALSE(attr.shm_perm.mode & SHM_DEST); + + // Transfer ownership to the scratch user via IPC_SET. + attr.shm_perm.uid = scratch_uid; + ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_SET, &attr)); + + // In the same dropped-privilege thread (now the owner), IPC_RMID + // should succeed. + ScopedThread([&] { + drop_privs([&] { EXPECT_NO_ERRNO(shm.Rmid()); }); + }); +} + } // namespace } // namespace testing } // namespace gvisor