From ee7a713757314366023e855a76ff17943b6b9296 Mon Sep 17 00:00:00 2001 From: "Isaac J. Manjarres" Date: Tue, 3 Dec 2024 09:42:56 -0800 Subject: [PATCH] ashmem: Ensure all memfds have non-executable permissions by default Currently, memfds are created with executable permissions, meaning that one can load a binary into a memfd buffer and use fexecve() to run said binary. This is not desirable for security reasons, and also does not match with the behavior that the ashmem driver currently supports. When the ashmem driver is in use, /dev/ashmem* does not have executable permissions, so fexecve() cannot be used on those buffers. Linux kernels 6.3+ offer MFD_NOEXEC_SEAL as part of the memfd interface, which allows one to create memfds with non-executable permissions. Furthermore, the executable permissions cannot be changed on these memfds. This matches the expected behavior that ashmem provided, so allow memfd usage only if MFD_NOEXEC_SEAL is supported, and create memfds with non-executable permissions by default. Bug: 111903542 Change-Id: Ibb2c2be3c118ead44fc12bcd2b63dcf6f83c9b03 Signed-off-by: Isaac J. Manjarres --- libcutils/ashmem-dev.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/libcutils/ashmem-dev.cpp b/libcutils/ashmem-dev.cpp index 46b8ef263..cebfa5d12 100644 --- a/libcutils/ashmem-dev.cpp +++ b/libcutils/ashmem-dev.cpp @@ -114,8 +114,14 @@ static bool __has_memfd_support() { // Check if kernel support exists, otherwise fall back to ashmem. // This code needs to build on old API levels, so we can't use the libc // wrapper. + // + // MFD_NOEXEC_SEAL is used to match the semantics of the ashmem device, + // which did not have executable permissions. This also seals the executable + // permissions of the buffer (i.e. they cannot be changed by fchmod()). + // + // MFD_NOEXEC_SEAL implies MFD_ALLOW_SEALING. android::base::unique_fd fd( - syscall(__NR_memfd_create, "test_android_memfd", MFD_CLOEXEC | MFD_ALLOW_SEALING)); + syscall(__NR_memfd_create, "test_android_memfd", MFD_CLOEXEC | MFD_NOEXEC_SEAL)); if (fd == -1) { ALOGE("memfd_create failed: %s, no memfd support.\n", strerror(errno)); return false; @@ -289,7 +295,13 @@ int ashmem_valid(int fd) static int memfd_create_region(const char* name, size_t size) { // This code needs to build on old API levels, so we can't use the libc // wrapper. - android::base::unique_fd fd(syscall(__NR_memfd_create, name, MFD_CLOEXEC | MFD_ALLOW_SEALING)); + // + // MFD_NOEXEC_SEAL to match the semantics of the ashmem device, which did + // not have executable permissions. This also seals the executable + // permissions of the buffer (i.e. they cannot be changed by fchmod()). + // + // MFD_NOEXEC_SEAL implies MFD_ALLOW_SEALING. + android::base::unique_fd fd(syscall(__NR_memfd_create, name, MFD_CLOEXEC | MFD_NOEXEC_SEAL)); if (fd == -1) { ALOGE("memfd_create(%s, %zd) failed: %s\n", name, size, strerror(errno));