Merge pull request #11358 from Snowflake-Labs:xuzhoyin-mmap-fix

PiperOrigin-RevId: 720973349
google · Jan 29, 2025 · ddebbe5 · ddebbe5
2 parents 63c9278 + 191b53d
commit ddebbe5
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 0 deletions.
diff --git a/pkg/sentry/syscalls/linux/sys_mmap.go b/pkg/sentry/syscalls/linux/sys_mmap.go
@@ -98,6 +98,15 @@ func Mmap(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr, *
 			opts.MaxPerms.Write = false
 		}
 
+		// mmap requires volume NO_EXEC be false if request has PROT_EXEC flag.
+		if file.Mount().MountFlags()&linux.ST_NOEXEC != 0 {
+			if opts.Perms.Execute {
+				return 0, nil, linuxerr.EPERM
+			}
+
+			opts.MaxPerms.Execute = false
+		}
+
 		if err := file.ConfigureMMap(t, &opts); err != nil {
 			return 0, nil, err
 		}

diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD
@@ -1432,6 +1432,7 @@ cc_binary(
         "//test/util:file_descriptor",
         "//test/util:fs_util",
         "//test/util:logging",
+        "//test/util:memory_util",
         "//test/util:mount_util",
         "//test/util:multiprocess_util",
         "//test/util:posix_error",

diff --git a/test/syscalls/linux/mount.cc b/test/syscalls/linux/mount.cc
@@ -62,6 +62,7 @@
 #include "test/util/fs_util.h"
 #include "test/util/linux_capability_util.h"
 #include "test/util/logging.h"
+#include "test/util/memory_util.h"
 #include "test/util/mount_util.h"
 #include "test/util/multiprocess_util.h"
 #include "test/util/posix_error.h"
@@ -252,6 +253,83 @@ TEST(MountTest, UmountDetach) {
       OpenAt(mounted_dir.get(), "..", O_DIRECTORY | O_RDONLY));
 }
 
+TEST(MountTest, MMapWithExecProtFailsOnNoExecFile) {
+  // Skips the test if test does not have needed capability to create the volume
+  // mount.
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
+
+  auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  auto ret = ASSERT_NO_ERRNO_AND_VALUE(
+      Mount("", dir.path(), kTmpfs, MS_NOEXEC, "", 0));
+  auto file = ASSERT_NO_ERRNO_AND_VALUE(
+      TempPath::CreateFileWith(dir.path(), "random1", 0777));
+
+  FileDescriptor fd =
+      ASSERT_NO_ERRNO_AND_VALUE(Open(file.path().c_str(), O_RDWR));
+  ASSERT_THAT(reinterpret_cast<uintptr_t>(
+                  mmap(0, kPageSize, PROT_EXEC, MAP_PRIVATE, fd.get(), 0)),
+              SyscallFailsWithErrno(EPERM));
+}
+
+TEST(MountTest, MMapWithExecProtSucceedsOnExecutableVolumeFile) {
+  // Capability is needed to create tmpfs.
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
+
+  auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  auto ret = ASSERT_NO_ERRNO_AND_VALUE(Mount("", dir.path(), kTmpfs, 0, "", 0));
+  auto file = ASSERT_NO_ERRNO_AND_VALUE(
+      TempPath::CreateFileWith(dir.path(), "random1", 0777));
+
+  FileDescriptor fd =
+      ASSERT_NO_ERRNO_AND_VALUE(Open(file.path().c_str(), O_RDWR));
+
+  void* address = mmap(0, kPageSize, PROT_EXEC, MAP_PRIVATE, fd.get(), 0);
+  EXPECT_NE(address, MAP_FAILED);
+
+  MunmapSafe(address, kPageSize);
+}
+
+TEST(MountTest, MMapWithoutNoExecProtSucceedsOnNoExecFile) {
+  // Capability is needed to create tmpfs.
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
+
+  auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  auto ret = ASSERT_NO_ERRNO_AND_VALUE(
+      Mount("", dir.path(), kTmpfs, MS_NOEXEC, "", 0));
+  auto file = ASSERT_NO_ERRNO_AND_VALUE(
+      TempPath::CreateFileWith(dir.path(), "random1", 0777));
+  FileDescriptor fd =
+      ASSERT_NO_ERRNO_AND_VALUE(Open(file.path().c_str(), O_RDWR));
+
+  void* address =
+      mmap(0, kPageSize, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd.get(), 0);
+  EXPECT_NE(address, MAP_FAILED);
+
+  MunmapSafe(address, kPageSize);
+}
+
+TEST(MountTest, MProtectWithNoExecProtFailsOnNoExecFile) {
+  // Capability is needed to create tmpfs.
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
+
+  auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  auto ret = ASSERT_NO_ERRNO_AND_VALUE(
+      Mount("", dir.path(), kTmpfs, MS_NOEXEC, "", 0));
+  auto file = ASSERT_NO_ERRNO_AND_VALUE(
+      TempPath::CreateFileWith(dir.path(), "random1", 0777));
+  FileDescriptor fd =
+      ASSERT_NO_ERRNO_AND_VALUE(Open(file.path().c_str(), O_RDWR));
+
+  void* address =
+      mmap(0, kPageSize, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd.get(), 0);
+  EXPECT_NE(address, MAP_FAILED);
+
+  ASSERT_THAT(mprotect(address, kPageSize, PROT_EXEC),
+              SyscallFailsWithErrno(EACCES));
+
+  MunmapSafe(address, kPageSize);
+}
+
 TEST(MountTest, UmountMountsStackedOnDot) {
   SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
   // Verify that unmounting at "." properly unmounts the mount at the top of