Skip to content

Commit 191b53d

Browse files
committed
Fix EXEC permission of the volume mount when calling mmap with PROT_EXEC
1 parent 1c38b6c commit 191b53d

File tree

3 files changed

+73
-0
lines changed

3 files changed

+73
-0
lines changed

pkg/sentry/syscalls/linux/sys_mmap.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,15 @@ func Mmap(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr, *
9898
opts.MaxPerms.Write = false
9999
}
100100

101+
// mmap requires volume NO_EXEC be false if request has PROT_EXEC flag.
102+
if file.Mount().MountFlags()&linux.ST_NOEXEC != 0 {
103+
if opts.Perms.Execute {
104+
return 0, nil, linuxerr.EPERM
105+
}
106+
107+
opts.MaxPerms.Execute = false
108+
}
109+
101110
if err := file.ConfigureMMap(t, &opts); err != nil {
102111
return 0, nil, err
103112
}

test/syscalls/linux/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1432,6 +1432,7 @@ cc_binary(
14321432
"//test/util:file_descriptor",
14331433
"//test/util:fs_util",
14341434
"//test/util:logging",
1435+
"//test/util:memory_util",
14351436
"//test/util:mount_util",
14361437
"//test/util:multiprocess_util",
14371438
"//test/util:posix_error",

test/syscalls/linux/mount.cc

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
#include "test/util/fs_util.h"
6363
#include "test/util/linux_capability_util.h"
6464
#include "test/util/logging.h"
65+
#include "test/util/memory_util.h"
6566
#include "test/util/mount_util.h"
6667
#include "test/util/multiprocess_util.h"
6768
#include "test/util/posix_error.h"
@@ -252,6 +253,68 @@ TEST(MountTest, UmountDetach) {
252253
OpenAt(mounted_dir.get(), "..", O_DIRECTORY | O_RDONLY));
253254
}
254255

256+
TEST(MountTest, MMapWithExecProtFailsOnNoExecFile) {
257+
// Skips the test if test does not have needed capability to create the volume mount.
258+
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
259+
260+
auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
261+
auto ret = ASSERT_NO_ERRNO_AND_VALUE(Mount("", dir.path(), kTmpfs, MS_NOEXEC, "", 0));
262+
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(dir.path(), "random1", 0777));
263+
264+
FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(file.path().c_str(), O_RDWR));
265+
ASSERT_THAT(
266+
reinterpret_cast<uintptr_t>(mmap(0, kPageSize, PROT_EXEC, MAP_PRIVATE, fd.get(), 0)),
267+
SyscallFailsWithErrno(EPERM));
268+
}
269+
270+
TEST(MountTest, MMapWithExecProtSucceedsOnExecutableVolumeFile) {
271+
// Capability is needed to create tmpfs.
272+
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
273+
274+
auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
275+
auto ret = ASSERT_NO_ERRNO_AND_VALUE(Mount("", dir.path(), kTmpfs, 0, "", 0));
276+
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(dir.path(), "random1", 0777));
277+
278+
FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(file.path().c_str(), O_RDWR));
279+
280+
void* address = mmap(0, kPageSize, PROT_EXEC, MAP_PRIVATE, fd.get(), 0);
281+
EXPECT_NE(address, MAP_FAILED);
282+
283+
MunmapSafe(address, kPageSize);
284+
}
285+
286+
TEST(MountTest, MMapWithoutNoExecProtSucceedsOnNoExecFile) {
287+
// Capability is needed to create tmpfs.
288+
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
289+
290+
auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
291+
auto ret = ASSERT_NO_ERRNO_AND_VALUE(Mount("", dir.path(), kTmpfs, MS_NOEXEC, "", 0));
292+
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(dir.path(), "random1", 0777));
293+
FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(file.path().c_str(), O_RDWR));
294+
295+
void* address = mmap(0, kPageSize, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd.get(), 0);
296+
EXPECT_NE(address, MAP_FAILED);
297+
298+
MunmapSafe(address, kPageSize);
299+
}
300+
301+
TEST(MountTest, MProtectWithNoExecProtFailsOnNoExecFile) {
302+
// Capability is needed to create tmpfs.
303+
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
304+
305+
auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
306+
auto ret = ASSERT_NO_ERRNO_AND_VALUE(Mount("", dir.path(), kTmpfs, MS_NOEXEC, "", 0));
307+
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(dir.path(), "random1", 0777));
308+
FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(file.path().c_str(), O_RDWR));
309+
310+
void* address = mmap(0, kPageSize, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd.get(), 0);
311+
EXPECT_NE(address, MAP_FAILED);
312+
313+
ASSERT_THAT(mprotect(address, kPageSize, PROT_EXEC), SyscallFailsWithErrno(EACCES));
314+
315+
MunmapSafe(address, kPageSize);
316+
}
317+
255318
TEST(MountTest, UmountMountsStackedOnDot) {
256319
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
257320
// Verify that unmounting at "." properly unmounts the mount at the top of

0 commit comments

Comments
 (0)