mm/memfd: MFD_NOEXEC for memfd_create
Subsystems (4)
cover mm/memfd: MFD_NOEXEC for memfd_create
The default file permissions on a memfd include execute bits, which means that such a memfd can be filled with a executable and passed to the exec() family of functions. This is undesirable on systems where all code is verified and all filesystems are intended to be mounted noexec, since an attacker may be able to use a memfd to load unverified code and execute it.
Additionally, execution via memfd is a common way to avoid scrutiny for malicious code, since it allows execution of a program without a file ever appearing on disk. This attack vector is not totally mitigated with this new flag, since the default memfd file permissions must remain executable to avoid breaking existing legitimate uses, but it should be possible to use other security mechanisms to prevent memfd_create calls without MFD_NOEXEC on systems where it is known that executable memfds are not necessary.
This patch series adds a new MFD_NOEXEC flag for memfd_create(), which allows creation of non-executable memfds, and as part of the implementation of this new flag, it also adds a new F_SEAL_EXEC seal, which will prevent modification of any of the execute bits of a sealed memfd.
I am not sure if this is the best way to implement the desired behavior (for example, the F_SEAL_EXEC seal is really more of an implementation detail and feels a bit clunky to expose), so suggestions are welcome for alternate approaches.
Daniel Verkamp (4):
mm/memfd: add F_SEAL_EXEC
mm/memfd: add MFD_NOEXEC flag to memfd_create
selftests/memfd: add tests for F_SEAL_EXEC
selftests/memfd: add tests for MFD_NOEXEC
include/uapi/linux/fcntl.h | 1 + include/uapi/linux/memfd.h | 1 + mm/memfd.c | 12 ++- mm/shmem.c | 6 ++ tools/testing/selftests/memfd/memfd_test.c | 114 +++++++++++++++++++++ 5 files changed, 133 insertions(+), 1 deletion(-)
Signature · 1 line
2.35.1.1094.g7c7d902a7c-goog
PATCH 1/4 mm/memfd: add F_SEAL_EXEC
The new F_SEAL_EXEC flag will prevent modification of the exec bits:
written as traditional octal mask, 0111, or as named flags, S_IXUSR | S_IXGRP | S_IXOTH. Any chmod(2) or similar call that attempts to modify any of these bits after the seal is applied will fail with errno EPERM.
This will preserve the execute bits as they are at the time of sealing, so the memfd will become either permanently executable or permanently un-executable.
--- include/uapi/linux/fcntl.h | 1 + mm/memfd.c | 2 ++ mm/shmem.c | 6 ++++++ 3 files changed, 9 insertions(+)
include/uapi/linux/fcntl.h +1 -0
@@ -43,6 +43,7 @@#define F_SEAL_GROW 0x0004/* prevent file from growing */ #define F_SEAL_WRITE 0x0008/* prevent writes */ #define F_SEAL_FUTURE_WRITE 0x0010/* prevent future writes while mapped */ +#define F_SEAL_EXEC 0x0020/* prevent chmod modifying exec bits */ /* (1U << 31) is reserved for signed error codes */ /*
mm/memfd.c +2 -0
@@ -147,6 +147,7 @@ static unsigned int *memfd_file_seals_ptr(struct file *file) }#define F_ALL_SEALS (F_SEAL_SEAL | \ + F_SEAL_EXEC | \ F_SEAL_SHRINK | \ F_SEAL_GROW | \ F_SEAL_WRITE | \
@@ -175,6 +176,7 @@ static int memfd_add_seals(struct file *file, unsigned int seals)* SEAL_SHRINK: Prevent the file from shrinking * SEAL_GROW: Prevent the file from growing * SEAL_WRITE: Prevent write access to the file +* SEAL_EXEC: Prevent modification of the exec bits in the file mode * * As we don't require any trust relationship between two parties, we * must prevent seals from being removed. Therefore, sealing a file
mm/shmem.c +6 -0
@@ -1083,6 +1083,12 @@ static int shmem_setattr(struct user_namespace *mnt_userns,if (error)return error; +if ((info->seals &F_SEAL_EXEC ) && (attr->ia_valid &ATTR_MODE )) { +if ((inode->i_mode ^ attr->ia_mode) &0111 ) { +return -EPERM ; + } + } +if (S_ISREG (inode->i_mode) && (attr->ia_valid &ATTR_SIZE )) {loff_t oldsize = inode->i_size;loff_t newsize = attr->ia_size;
Signature · 1 line
2.35.1.1094.g7c7d902a7c-goog
PATCH 2/4 mm/memfd: add MFD_NOEXEC flag to memfd_create
In reply to Daniel VerkampThe new MFD_NOEXEC flag allows the creation of a permanently non-executable memfd. This is accomplished by creating it with a different set of file mode bits (0666) than the default (0777) and applying the F_SEAL_EXEC seal at creation time, so there is no window between memfd creation and seal application.
Unfortunately, the default for memfd must remain executable, since changing this would be an API break, and some programs depend on being able to exec code from a memfd directly. However, this new flag will allow programs to create non-executable memfds, and a distribution may choose to enforce use of this flag in memfd_create calls via other security mechanisms.
--- include/uapi/linux/memfd.h | 1 + mm/memfd.c | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-)
include/uapi/linux/memfd.h +1 -0
@@ -8,6 +8,7 @@#define MFD_CLOEXEC 0x0001U#define MFD_ALLOW_SEALING 0x0002U#define MFD_HUGETLB 0x0004U +#define MFD_NOEXEC 0x0008U/* * Huge page size encoding when MFD_HUGETLB is specified, and a huge page
mm/memfd.c +9 -1
@@ -263,7 +263,7 @@ long memfd_fcntl(struct file *file, unsigned int cmd, unsigned long arg)#define MFD_NAME_PREFIX_LEN (sizeof(MFD_NAME_PREFIX) - 1)#define MFD_NAME_MAX_LEN (NAME_MAX - MFD_NAME_PREFIX_LEN) -#define MFD_ALL_FLAGS (MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_HUGETLB) +#define MFD_ALL_FLAGS (MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_HUGETLB | MFD_NOEXEC) SYSCALL_DEFINE2(memfd_create, const char __user *, uname,
@@ -333,6 +333,14 @@ SYSCALL_DEFINE2(memfd_create, *file_seals &= ~F_SEAL_SEAL ; } +if (flags &MFD_NOEXEC ) { +struct inode *inode =file_inode (file); + + inode->i_mode &= ~0111 ; + file_seals =memfd_file_seals_ptr (file); + *file_seals |=F_SEAL_EXEC ; + } +fd_install (fd, file);kfree (name);return fd;
Signature · 1 line
2.35.1.1094.g7c7d902a7c-goog
PATCH 3/4 selftests/memfd: add tests for F_SEAL_EXEC
In reply to Daniel VerkampBasic tests to ensure that user/group/other execute bits cannot be changed after applying F_SEAL_EXEC to a memfd.
--- tools/testing/selftests/memfd/memfd_test.c | 80 ++++++++++++++++++++++ 1 file changed, 80 insertions(+)
tools/testing/selftests/memfd/memfd_test.c +80 -0
@@ -28,6 +28,10 @@#define MFD_DEF_SIZE 8192#define STACK_SIZE 65536 +#ifndef F_SEAL_EXEC +#define F_SEAL_EXEC 0x0020 +#endif +/* * Default is not to test hugetlbfs */
@@ -594,6 +598,48 @@ static void mfd_fail_grow_write(int fd) } } +static void mfd_assert_mode (int fd,int mode) +{ +struct stat st; + +if (fstat (fd, &st) <0 ) { +printf ("fstat(%d) failed: %m\n" , fd); +abort (); + }else if ((st.st_mode &07777 ) != mode) { +printf ("wrong file mode 0%04o, but expected 0%04o\n" , + (int )st.st_mode &07777 , mode); +abort (); + } +} + +static void mfd_assert_chmod (int fd,int mode) +{ +if (fchmod (fd, mode) <0 ) { +printf ("fchmod(0%04o) failed: %m\n" , mode); +abort (); + } + +mfd_assert_mode (fd, mode); +} + +static void mfd_fail_chmod (int fd,int mode) +{ +struct stat st; + +if (fstat (fd, &st) <0 ) { +printf ("fstat(%d) failed: %m\n" , fd); +abort (); + } + +if (fchmod (fd, mode) ==0 ) { +printf ("fchmod(0%04o) didn't fail as expected\n" ); +abort (); + } + +/* verify that file mode bits did not change */ +mfd_assert_mode (fd, st.st_mode &07777 ); +} +static int idle_thread_fn (void *arg) {sigset_t set;
@@ -880,6 +926,39 @@ static void test_seal_resize(void)close (fd); } +/* +* Test SEAL_EXEC +* Test that chmod() cannot change x bits after sealing +*/ +static void test_seal_exec (void ) +{ +int fd; + +printf ("%s SEAL-EXEC\n" , memfd_str); + + fd =mfd_assert_new ("kern_memfd_seal_exec" , + mfd_def_size, +MFD_CLOEXEC |MFD_ALLOW_SEALING ); + +mfd_assert_mode (fd,0777 ); + +mfd_assert_chmod (fd,0644 ); + +mfd_assert_has_seals (fd,0 ); +mfd_assert_add_seals (fd,F_SEAL_EXEC ); +mfd_assert_has_seals (fd,F_SEAL_EXEC ); + +mfd_assert_chmod (fd,0600 ); +mfd_fail_chmod (fd,0777 ); +mfd_fail_chmod (fd,0670 ); +mfd_fail_chmod (fd,0605 ); +mfd_fail_chmod (fd,0700 ); +mfd_fail_chmod (fd,0100 ); +mfd_assert_chmod (fd,0666 ); + +close (fd); +} +/* * Test sharing via dup() * Test that seals are shared between dupped FDs and they're all equal.
@@ -1059,6 +1138,7 @@ int main(int argc, char **argv)test_seal_shrink ();test_seal_grow ();test_seal_resize (); +test_seal_exec ();test_share_dup ("SHARE-DUP" ,"" );test_share_mmap ("SHARE-MMAP" ,"" );
Signature · 1 line
2.35.1.1094.g7c7d902a7c-goog
Shuah Khan
maintainerRe: [PATCH 3/4] selftests/memfd: add tests for F_SEAL_EXEC
Quoted 24 lines · Daniel Verkamp
Basic tests to ensure that user/group/other execute bits cannot be changed after applying
F_SEAL_EXECto a memfd.Signed-off-by Daniel Verkamp <[email protected]>--- tools/testing/selftests/memfd/memfd_test.c | 80 ++++++++++++++++++++++ 1 file changed, 80 insertions(+)tools/testing/selftests/memfd/memfd_test.c +4
-0@@ -28,6 +28,10 @@#define MFD_DEF_SIZE 8192#define STACK_SIZE 65536 +#ifndef F_SEAL_EXEC +#define F_SEAL_EXEC 0x0020 +#endif +/* * Default is not to test hugetlbfs */
tools/testing/selftests/memfd/memfd_test.c static void mfd_fail_grow_write(int fd) · line 606
Quoted 9 lines
tools/testing/selftests/memfd/memfd_test.c +6
-0@@ -594,6 +598,48 @@ static void mfd_fail_grow_write(int fd) } } +static void mfd_assert_mode (int fd,int mode) +{ +struct stat st; + +if (fstat (fd, &st) <0 ) { +printf ("fstat(%d) failed: %m\n" , fd);
Let's print the filename here - just printing fd isn't useful.
tools/testing/selftests/memfd/memfd_test.c static void mfd_fail_grow_write(int fd) · line 610
Quoted 4 lines
tools/testing/selftests/memfd/memfd_test.c +4
-0+abort (); + }else if ((st.st_mode &07777 ) != mode) { +printf ("wrong file mode 0%04o, but expected 0%04o\n" , + (int )st.st_mode &07777 , mode);
This one doesn't even print fd - same comment here about filename.
tools/testing/selftests/memfd/memfd_test.c static void mfd_fail_grow_write(int fd) · line 618
Quoted 8 lines
tools/testing/selftests/memfd/memfd_test.c +8
-0+abort (); + } +} + +static void mfd_assert_chmod (int fd,int mode) +{ +if (fchmod (fd, mode) <0 ) { +printf ("fchmod(0%04o) failed: %m\n" , mode);
Same here.
tools/testing/selftests/memfd/memfd_test.c static void mfd_fail_grow_write(int fd) · line 630
Quoted 12 lines
tools/testing/selftests/memfd/memfd_test.c +12
-0+abort (); + } + +mfd_assert_mode (fd, mode); +} + +static void mfd_fail_chmod (int fd,int mode) +{ +struct stat st; + +if (fstat (fd, &st) <0 ) { +printf ("fstat(%d) failed: %m\n" , fd);
Same comment about filename
tools/testing/selftests/memfd/memfd_test.c static void mfd_fail_grow_write(int fd) · line 635
Quoted 5 lines
tools/testing/selftests/memfd/memfd_test.c +5
-0+abort (); + } + +if (fchmod (fd, mode) ==0 ) { +printf ("fchmod(0%04o) didn't fail as expected\n" );
Same comment about filename
Quoted 52 lines
tools/testing/selftests/memfd/memfd_test.c +40
-0+abort (); + } + +/* verify that file mode bits did not change */ +mfd_assert_mode (fd, st.st_mode &07777 ); +} +static int idle_thread_fn (void *arg) {sigset_t set;@@ -880,6 +926,39 @@ static void test_seal_resize(void)close (fd); } +/* +* Test SEAL_EXEC +* Test that chmod() cannot change x bits after sealing +*/ +static void test_seal_exec (void ) +{ +int fd; + +printf ("%s SEAL-EXEC\n" , memfd_str); + + fd =mfd_assert_new ("kern_memfd_seal_exec" , + mfd_def_size, +MFD_CLOEXEC |MFD_ALLOW_SEALING ); + +mfd_assert_mode (fd,0777 ); + +mfd_assert_chmod (fd,0644 ); + +mfd_assert_has_seals (fd,0 ); +mfd_assert_add_seals (fd,F_SEAL_EXEC ); +mfd_assert_has_seals (fd,F_SEAL_EXEC ); + +mfd_assert_chmod (fd,0600 ); +mfd_fail_chmod (fd,0777 ); +mfd_fail_chmod (fd,0670 ); +mfd_fail_chmod (fd,0605 ); +mfd_fail_chmod (fd,0700 ); +mfd_fail_chmod (fd,0100 ); +mfd_assert_chmod (fd,0666 ); + +close (fd); +} +/* * Test sharing via dup() * Test that seals are shared between dupped FDs and they're all equal.
tools/testing/selftests/memfd/memfd_test.c int main(int argc, char **argv) · line 1144
Quoted 7 lines
tools/testing/selftests/memfd/memfd_test.c +1
-0@@ -1059,6 +1138,7 @@ int main(int argc, char **argv)test_seal_shrink ();test_seal_grow ();test_seal_resize (); +test_seal_exec ();test_share_dup ("SHARE-DUP" ,"" );test_share_mmap ("SHARE-MMAP" ,"" );
The rest looks good.
thanks,
-- Shuah
PATCH 3/4 selftests/memfd: add tests for F_SEAL_EXEC
From: Daniel Verkamp <[email protected]>
Basic tests to ensure that user/group/other execute bits cannot be changed after applying F_SEAL_EXEC to a memfd.
--- tools/testing/selftests/memfd/memfd_test.c | 129 ++++++++++++++++++++- 1 file changed, 128 insertions(+), 1 deletion(-)
tools/testing/selftests/memfd/memfd_test.c +128 -1
@@ -28,12 +28,44 @@#define MFD_DEF_SIZE 8192#define STACK_SIZE 65536 +#ifndef F_SEAL_EXEC +#define F_SEAL_EXEC 0x0020 +#endif + +#ifndef MAX_PATH +#define MAX_PATH 256 +#endif +/* * Default is not to test hugetlbfs */ static size_t mfd_def_size =MFD_DEF_SIZE ;static const char *memfd_str =MEMFD_STR ; +static ssize_t fd2name (int fd,char *buf,size_t bufsize) +{ +char buf1[MAX_PATH ]; +int size; +ssize_t nbytes; + + size =snprintf (buf1,MAX_PATH ,"/proc/self/fd/%d" , fd); +if (size <0 ) { +printf ("snprintf(%d) failed on %m\n" , fd); +abort (); + } + +/* +* reserver one byte for string termination. +*/ + nbytes =readlink (buf1, buf, bufsize-1 ); +if (nbytes ==-1 ) { +printf ("readlink(%s) failed %m\n" , buf1); +abort (); + } + buf[nbytes] ='\0' ; +return nbytes; +} +static int mfd_assert_new (const char *name,loff_t sz,unsigned int flags) {int r, fd;
@@ -98,11 +130,14 @@ static unsigned int mfd_assert_get_seals(int fd)static void mfd_assert_has_seals (int fd,unsigned int seals) { +char buf[MAX_PATH ]; +int nbytes;unsigned int s; +fd2name (fd, buf,MAX_PATH ); s =mfd_assert_get_seals (fd);if (s != seals) { -printf ("%u != %u = GET_SEALS(%d)\n" , seals, s, fd); +printf ("%u != %u = GET_SEALS(%s)\n" , seals, s, buf);abort (); } }
@@ -594,6 +629,64 @@ static void mfd_fail_grow_write(int fd) } } +static void mfd_assert_mode (int fd,int mode) +{ +struct stat st; +char buf[MAX_PATH ]; +int nbytes; + +fd2name (fd, buf,MAX_PATH ); + +if (fstat (fd, &st) <0 ) { +printf ("fstat(%s) failed: %m\n" , buf); +abort (); + } + +if ((st.st_mode &07777 ) != mode) { +printf ("fstat(%s) wrong file mode 0%04o, but expected 0%04o\n" , + buf, (int )st.st_mode &07777 , mode); +abort (); + } +} + +static void mfd_assert_chmod (int fd,int mode) +{ +char buf[MAX_PATH ]; +int nbytes; + +fd2name (fd, buf,MAX_PATH ); + +if (fchmod (fd, mode) <0 ) { +printf ("fchmod(%s, 0%04o) failed: %m\n" , buf, mode); +abort (); + } + +mfd_assert_mode (fd, mode); +} + +static void mfd_fail_chmod (int fd,int mode) +{ +struct stat st; +char buf[MAX_PATH ]; +int nbytes; + +fd2name (fd, buf,MAX_PATH ); + +if (fstat (fd, &st) <0 ) { +printf ("fstat(%s) failed: %m\n" , buf); +abort (); + } + +if (fchmod (fd, mode) ==0 ) { +printf ("fchmod(%s, 0%04o) didn't fail as expected\n" , + buf, mode); +abort (); + } + +/* verify that file mode bits did not change */ +mfd_assert_mode (fd, st.st_mode &07777 ); +} +static int idle_thread_fn (void *arg) {sigset_t set;
@@ -880,6 +973,39 @@ static void test_seal_resize(void)close (fd); } +/* +* Test SEAL_EXEC +* Test that chmod() cannot change x bits after sealing +*/ +static void test_seal_exec (void ) +{ +int fd; + +printf ("%s SEAL-EXEC\n" , memfd_str); + + fd =mfd_assert_new ("kern_memfd_seal_exec" , + mfd_def_size, +MFD_CLOEXEC |MFD_ALLOW_SEALING ); + +mfd_assert_mode (fd,0777 ); + +mfd_assert_chmod (fd,0644 ); + +mfd_assert_has_seals (fd,0 ); +mfd_assert_add_seals (fd,F_SEAL_EXEC ); +mfd_assert_has_seals (fd,F_SEAL_EXEC ); + +mfd_assert_chmod (fd,0600 ); +mfd_fail_chmod (fd,0777 ); +mfd_fail_chmod (fd,0670 ); +mfd_fail_chmod (fd,0605 ); +mfd_fail_chmod (fd,0700 ); +mfd_fail_chmod (fd,0100 ); +mfd_assert_chmod (fd,0666 ); + +close (fd); +} +/* * Test sharing via dup() * Test that seals are shared between dupped FDs and they're all equal.
@@ -1059,6 +1185,7 @@ int main(int argc, char **argv)test_seal_shrink ();test_seal_grow ();test_seal_resize (); +test_seal_exec ();test_share_dup ("SHARE-DUP" ,"" );test_share_mmap ("SHARE-MMAP" ,"" );
Signature · 1 line
2.37.1.455.g008518b4e5-goog
PATCH 4/4 selftests/memfd: add tests for MFD_NOEXEC
From: Daniel Verkamp <[email protected]>
Tests that ensure MFD_NOEXEC memfds have the appropriate mode bits and cannot be chmod-ed into being executable.
--- tools/testing/selftests/memfd/memfd_test.c | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+)
tools/testing/selftests/memfd/memfd_test.c +34 -0
@@ -36,6 +36,10 @@#define MAX_PATH 256#endif +#ifndef MFD_NOEXEC +#define MFD_NOEXEC 0x0008U +#endif +/* * Default is not to test hugetlbfs */
@@ -1006,6 +1010,35 @@ static void test_seal_exec(void)close (fd); } +/* +* Test memfd_create with MFD_NOEXEC flag +* Test that MFD_NOEXEC applies F_SEAL_EXEC and prevents change of exec bits +*/ +static void test_noexec (void ) +{ +int fd; + +printf ("%s NOEXEC\n" , memfd_str); + +/* Create with NOEXEC and ALLOW_SEALING */ + fd =mfd_assert_new ("kern_memfd_noexec" , + mfd_def_size, +MFD_CLOEXEC |MFD_ALLOW_SEALING |MFD_NOEXEC ); +mfd_assert_mode (fd,0666 ); +mfd_assert_has_seals (fd,F_SEAL_EXEC ); +mfd_fail_chmod (fd,0777 ); +close (fd); + +/* Create with NOEXEC but without ALLOW_SEALING */ + fd =mfd_assert_new ("kern_memfd_noexec" , + mfd_def_size, +MFD_CLOEXEC |MFD_NOEXEC ); +mfd_assert_mode (fd,0666 ); +mfd_assert_has_seals (fd,F_SEAL_EXEC |F_SEAL_SEAL ); +mfd_fail_chmod (fd,0777 ); +close (fd); +} +/* * Test sharing via dup() * Test that seals are shared between dupped FDs and they're all equal.
@@ -1179,6 +1212,7 @@ int main(int argc, char **argv)test_create ();test_basic (); +test_noexec ();test_seal_write ();test_seal_future_write ();
Signature · 1 line
2.37.1.455.g008518b4e5-goog
Re: [PATCH 4/4] selftests/memfd: add tests for MFD_NOEXEC
Quoted 7 lines
tools/testing/selftests/memfd/memfd_test.c static void test_seal_exec(void) · line 1026
Quoted 4 lines
+ /* Create with NOEXEC and
ALLOW_SEALING*/
+ fd = mfd_assert_new("kern_memfd_noexec",tools/testing/selftests/memfd/memfd_test.c +2
-0+ mfd_def_size, + MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_NOEXEC);Don't we need to check fd here?
mfd_assert_new will abort() if fd is not valid, so we don't check fd here.
Jeff
Quoted 75 lines · Jeff Xu
From: Daniel Verkamp <[email protected]>
Tests that ensure
MFD_NOEXECmemfds have the appropriate mode bits and cannot be chmod-ed into being executable.Co-developed-by Jeff Xu <[email protected]>Signed-off-by Jeff Xu <[email protected]>Signed-off-by Daniel Verkamp <[email protected]>--- tools/testing/selftests/memfd/memfd_test.c | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+)tools/testing/selftests/memfd/memfd_test.c +34
-1@@ -36,6 +36,10 @@#define MAX_PATH 256#endif +#ifndef MFD_NOEXEC +#define MFD_NOEXEC 0x0008U +#endif +/* * Default is not to test hugetlbfs */ @@ -1006,6 +1010,35 @@ static void test_seal_exec(void)close (fd); } +/* +* Test memfd_create with MFD_NOEXEC flag +* Test that MFD_NOEXEC applies F_SEAL_EXEC and prevents change of exec bits +*/ +static void test_noexec (void ) +{ +int fd; + +printf ("%s NOEXEC\n" , memfd_str); + +/* Create with NOEXEC and ALLOW_SEALING */ + fd =mfd_assert_new ("kern_memfd_noexec" , + mfd_def_size, +MFD_CLOEXEC |MFD_ALLOW_SEALING |MFD_NOEXEC ); +mfd_assert_mode (fd,0666 ); +mfd_assert_has_seals (fd,F_SEAL_EXEC ); +mfd_fail_chmod (fd,0777 ); +close (fd); + +/* Create with NOEXEC but without ALLOW_SEALING */ + fd =mfd_assert_new ("kern_memfd_noexec" , + mfd_def_size, +MFD_CLOEXEC |MFD_NOEXEC ); +mfd_assert_mode (fd,0666 ); +mfd_assert_has_seals (fd,F_SEAL_EXEC |F_SEAL_SEAL ); +mfd_fail_chmod (fd,0777 ); +close (fd); +} +/* * Test sharing via dup() * Test that seals are shared between dupped FDs and they're all equal. @@ -1179,6 +1212,7 @@ int main(int argc, char **argv)test_create ();test_basic (); +test_noexec ();test_seal_write ();test_seal_future_write (); --2.37.1.455.g008518b4e5-goog
Re: [PATCH 3/4] selftests/memfd: add tests for F_SEAL_EXEC
In reply to Jeff XuHi Shuah Khan
I will continue Daniel Verkamp's work on this patch.
Could you please take a look at the new patch to see if all your comments are addressed ?
Much appreciated.
Best Regards,
Jeff.
Quoted 196 lines · Jeff Xu
From: Daniel Verkamp <[email protected]>
Basic tests to ensure that user/group/other execute bits cannot be changed after applying
F_SEAL_EXECto a memfd.Co-developed-by Jeff Xu <[email protected]>Signed-off-by Jeff Xu <[email protected]>Signed-off-by Daniel Verkamp <[email protected]>--- tools/testing/selftests/memfd/memfd_test.c | 129 ++++++++++++++++++++- 1 file changed, 128 insertions(+), 1 deletion(-)tools/testing/selftests/memfd/memfd_test.c +94
-1@@ -28,12 +28,44 @@#define MFD_DEF_SIZE 8192#define STACK_SIZE 65536 +#ifndef F_SEAL_EXEC +#define F_SEAL_EXEC 0x0020 +#endif + +#ifndef MAX_PATH +#define MAX_PATH 256 +#endif +/* * Default is not to test hugetlbfs */ static size_t mfd_def_size =MFD_DEF_SIZE ;static const char *memfd_str =MEMFD_STR ; +static ssize_t fd2name (int fd,char *buf,size_t bufsize) +{ +char buf1[MAX_PATH ]; +int size; +ssize_t nbytes; + + size =snprintf (buf1,MAX_PATH ,"/proc/self/fd/%d" , fd); +if (size <0 ) { +printf ("snprintf(%d) failed on %m\n" , fd); +abort (); + } + +/* +* reserver one byte for string termination. +*/ + nbytes =readlink (buf1, buf, bufsize-1 ); +if (nbytes ==-1 ) { +printf ("readlink(%s) failed %m\n" , buf1); +abort (); + } + buf[nbytes] ='\0' ; +return nbytes; +} +static int mfd_assert_new (const char *name,loff_t sz,unsigned int flags) {int r, fd;@@ -98,11 +130,14 @@ static unsigned int mfd_assert_get_seals(int fd)static void mfd_assert_has_seals (int fd,unsigned int seals) { +char buf[MAX_PATH ]; +int nbytes;unsigned int s; +fd2name (fd, buf,MAX_PATH ); s =mfd_assert_get_seals (fd);if (s != seals) { -printf ("%u != %u = GET_SEALS(%d)\n" , seals, s, fd); +printf ("%u != %u = GET_SEALS(%s)\n" , seals, s, buf);abort (); } }@@ -594,6 +629,64 @@ static void mfd_fail_grow_write(int fd) } } +static void mfd_assert_mode (int fd,int mode) +{ +struct stat st; +char buf[MAX_PATH ]; +int nbytes; + +fd2name (fd, buf,MAX_PATH ); + +if (fstat (fd, &st) <0 ) { +printf ("fstat(%s) failed: %m\n" , buf); +abort (); + } + +if ((st.st_mode &07777 ) != mode) { + printf("fstat(%s) wrong file mode 0%04o, but expected 0%04o\n", + buf, (int )st.st_mode &07777 , mode); +abort (); + } +} + +static void mfd_assert_chmod (int fd,int mode) +{ +char buf[MAX_PATH ]; +int nbytes; + +fd2name (fd, buf,MAX_PATH ); + +if (fchmod (fd, mode) <0 ) { +printf ("fchmod(%s, 0%04o) failed: %m\n" , buf, mode); +abort (); + } + +mfd_assert_mode (fd, mode); +} + +static void mfd_fail_chmod (int fd,int mode) +{ +struct stat st; +char buf[MAX_PATH ]; +int nbytes; + +fd2name (fd, buf,MAX_PATH ); + +if (fstat (fd, &st) <0 ) { +printf ("fstat(%s) failed: %m\n" , buf); +abort (); + } + +if (fchmod (fd, mode) ==0 ) { +printf ("fchmod(%s, 0%04o) didn't fail as expected\n" , + buf, mode); +abort (); + } + +/* verify that file mode bits did not change */ +mfd_assert_mode (fd, st.st_mode &07777 ); +} +static int idle_thread_fn (void *arg) {sigset_t set;tools/testing/selftests/memfd/memfd_test.c +34
-1@@ -880,6 +973,39 @@ static void test_seal_resize(void)close (fd); } +/* +* Test SEAL_EXEC +* Test that chmod() cannot change x bits after sealing +*/ +static void test_seal_exec (void ) +{ +int fd; + +printf ("%s SEAL-EXEC\n" , memfd_str); + + fd =mfd_assert_new ("kern_memfd_seal_exec" , + mfd_def_size, +MFD_CLOEXEC |MFD_ALLOW_SEALING ); + +mfd_assert_mode (fd,0777 ); + +mfd_assert_chmod (fd,0644 ); + +mfd_assert_has_seals (fd,0 ); +mfd_assert_add_seals (fd,F_SEAL_EXEC ); +mfd_assert_has_seals (fd,F_SEAL_EXEC ); + +mfd_assert_chmod (fd,0600 ); +mfd_fail_chmod (fd,0777 ); +mfd_fail_chmod (fd,0670 ); +mfd_fail_chmod (fd,0605 ); +mfd_fail_chmod (fd,0700 ); +mfd_fail_chmod (fd,0100 ); +mfd_assert_chmod (fd,0666 ); + +close (fd); +} +/* * Test sharing via dup() * Test that seals are shared between dupped FDs and they're all equal. @@ -1059,6 +1185,7 @@ int main(int argc, char **argv)test_seal_shrink ();test_seal_grow ();test_seal_resize (); +test_seal_exec ();test_share_dup ("SHARE-DUP" ,"" );test_share_mmap ("SHARE-MMAP" ,"" ); --2.37.1.455.g008518b4e5-goog
Re: [PATCH 3/4] selftests/memfd: add tests for F_SEAL_EXEC
In reply to Jeff XuHi Shuah Khan
I will continue Daniel Verkamp's work on this patch.
Could you please take a look at the new patch set I sent to see if all your comments are addressed ?
Much appreciated.
Best Regards,
Jeff.
Quoted 194 lines · Jeff Xu
From: Daniel Verkamp <[email protected]>
Basic tests to ensure that user/group/other execute bits cannot be changed after applying
F_SEAL_EXECto a memfd.Co-developed-by Jeff Xu <[email protected]>Signed-off-by Jeff Xu <[email protected]>Signed-off-by Daniel Verkamp <[email protected]>--- tools/testing/selftests/memfd/memfd_test.c | 129 ++++++++++++++++++++- 1 file changed, 128 insertions(+), 1 deletion(-)tools/testing/selftests/memfd/memfd_test.c +128
-2@@ -28,12 +28,44 @@#define MFD_DEF_SIZE 8192#define STACK_SIZE 65536 +#ifndef F_SEAL_EXEC +#define F_SEAL_EXEC 0x0020 +#endif + +#ifndef MAX_PATH +#define MAX_PATH 256 +#endif +/* * Default is not to test hugetlbfs */ static size_t mfd_def_size =MFD_DEF_SIZE ;static const char *memfd_str =MEMFD_STR ; +static ssize_t fd2name (int fd,char *buf,size_t bufsize) +{ +char buf1[MAX_PATH ]; +int size; +ssize_t nbytes; + + size =snprintf (buf1,MAX_PATH ,"/proc/self/fd/%d" , fd); +if (size <0 ) { +printf ("snprintf(%d) failed on %m\n" , fd); +abort (); + } + +/* +* reserver one byte for string termination. +*/ + nbytes =readlink (buf1, buf, bufsize-1 ); +if (nbytes ==-1 ) { +printf ("readlink(%s) failed %m\n" , buf1); +abort (); + } + buf[nbytes] ='\0' ; +return nbytes; +} +static int mfd_assert_new (const char *name,loff_t sz,unsigned int flags) {int r, fd;@@ -98,11 +130,14 @@ static unsigned int mfd_assert_get_seals(int fd)static void mfd_assert_has_seals (int fd,unsigned int seals) { +char buf[MAX_PATH ]; +int nbytes;unsigned int s; +fd2name (fd, buf,MAX_PATH ); s =mfd_assert_get_seals (fd);if (s != seals) { -printf ("%u != %u = GET_SEALS(%d)\n" , seals, s, fd); +printf ("%u != %u = GET_SEALS(%s)\n" , seals, s, buf);abort (); } }@@ -594,6 +629,64 @@ static void mfd_fail_grow_write(int fd) } } +static void mfd_assert_mode (int fd,int mode) +{ +struct stat st; +char buf[MAX_PATH ]; +int nbytes; + +fd2name (fd, buf,MAX_PATH ); + +if (fstat (fd, &st) <0 ) { +printf ("fstat(%s) failed: %m\n" , buf); +abort (); + } + +if ((st.st_mode &07777 ) != mode) { +printf ("fstat(%s) wrong file mode 0%04o, but expected 0%04o\n" , + buf, (int )st.st_mode &07777 , mode); +abort (); + } +} + +static void mfd_assert_chmod (int fd,int mode) +{ +char buf[MAX_PATH ]; +int nbytes; + +fd2name (fd, buf,MAX_PATH ); + +if (fchmod (fd, mode) <0 ) { +printf ("fchmod(%s, 0%04o) failed: %m\n" , buf, mode); +abort (); + } + +mfd_assert_mode (fd, mode); +} + +static void mfd_fail_chmod (int fd,int mode) +{ +struct stat st; +char buf[MAX_PATH ]; +int nbytes; + +fd2name (fd, buf,MAX_PATH ); + +if (fstat (fd, &st) <0 ) { +printf ("fstat(%s) failed: %m\n" , buf); +abort (); + } + +if (fchmod (fd, mode) ==0 ) { +printf ("fchmod(%s, 0%04o) didn't fail as expected\n" , + buf, mode); +abort (); + } + +/* verify that file mode bits did not change */ +mfd_assert_mode (fd, st.st_mode &07777 ); +} +static int idle_thread_fn (void *arg) {sigset_t set;@@ -880,6 +973,39 @@ static void test_seal_resize(void)close (fd); } +/* +* Test SEAL_EXEC +* Test that chmod() cannot change x bits after sealing +*/ +static void test_seal_exec (void ) +{ +int fd; + +printf ("%s SEAL-EXEC\n" , memfd_str); + + fd =mfd_assert_new ("kern_memfd_seal_exec" , + mfd_def_size, +MFD_CLOEXEC |MFD_ALLOW_SEALING ); + +mfd_assert_mode (fd,0777 ); + +mfd_assert_chmod (fd,0644 ); + +mfd_assert_has_seals (fd,0 ); +mfd_assert_add_seals (fd,F_SEAL_EXEC ); +mfd_assert_has_seals (fd,F_SEAL_EXEC ); + +mfd_assert_chmod (fd,0600 ); +mfd_fail_chmod (fd,0777 ); +mfd_fail_chmod (fd,0670 ); +mfd_fail_chmod (fd,0605 ); +mfd_fail_chmod (fd,0700 ); +mfd_fail_chmod (fd,0100 ); +mfd_assert_chmod (fd,0666 ); + +close (fd); +} +/* * Test sharing via dup() * Test that seals are shared between dupped FDs and they're all equal. @@ -1059,6 +1185,7 @@ int main(int argc, char **argv)test_seal_shrink ();test_seal_grow ();test_seal_resize (); +test_seal_exec ();test_share_dup ("SHARE-DUP" ,"" );test_share_mmap ("SHARE-MMAP" ,"" ); --2.37.1.455.g008518b4e5-goog
PATCH 4/4 selftests/memfd: add tests for MFD_NOEXEC
In reply to Daniel VerkampTests that ensure MFD_NOEXEC memfds have the appropriate mode bits and cannot be chmod-ed into being executable.
--- tools/testing/selftests/memfd/memfd_test.c | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+)
tools/testing/selftests/memfd/memfd_test.c +34 -0
@@ -32,6 +32,10 @@#define F_SEAL_EXEC 0x0020#endif +#ifndef MFD_NOEXEC +#define MFD_NOEXEC 0x0008U +#endif +/* * Default is not to test hugetlbfs */
@@ -959,6 +963,35 @@ static void test_seal_exec(void)close (fd); } +/* +* Test memfd_create with MFD_NOEXEC flag +* Test that MFD_NOEXEC applies F_SEAL_EXEC and prevents change of exec bits +*/ +static void test_noexec (void ) +{ +int fd; + +printf ("%s NOEXEC\n" , memfd_str); + +/* Create with NOEXEC and ALLOW_SEALING */ + fd =mfd_assert_new ("kern_memfd_noexec" , + mfd_def_size, +MFD_CLOEXEC |MFD_ALLOW_SEALING |MFD_NOEXEC ); +mfd_assert_mode (fd,0666 ); +mfd_assert_has_seals (fd,F_SEAL_EXEC ); +mfd_fail_chmod (fd,0777 ); +close (fd); + +/* Create with NOEXEC but without ALLOW_SEALING */ + fd =mfd_assert_new ("kern_memfd_noexec" , + mfd_def_size, +MFD_CLOEXEC |MFD_NOEXEC ); +mfd_assert_mode (fd,0666 ); +mfd_assert_has_seals (fd,F_SEAL_EXEC |F_SEAL_SEAL ); +mfd_fail_chmod (fd,0777 ); +close (fd); +} +/* * Test sharing via dup() * Test that seals are shared between dupped FDs and they're all equal.
@@ -1132,6 +1165,7 @@ int main(int argc, char **argv)test_create ();test_basic (); +test_noexec ();test_seal_write ();test_seal_future_write ();
Signature · 1 line
2.35.1.1094.g7c7d902a7c-goog
Shuah Khan
maintainerRe: [PATCH 4/4] selftests/memfd: add tests for MFD_NOEXEC
Quoted 31 lines · Daniel Verkamp
Tests that ensure
MFD_NOEXECmemfds have the appropriate mode bits and cannot be chmod-ed into being executable.Signed-off-by Daniel Verkamp <[email protected]>--- tools/testing/selftests/memfd/memfd_test.c | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+)tools/testing/selftests/memfd/memfd_test.c +8
-0@@ -32,6 +32,10 @@#define F_SEAL_EXEC 0x0020#endif +#ifndef MFD_NOEXEC +#define MFD_NOEXEC 0x0008U +#endif +/* * Default is not to test hugetlbfs */ @@ -959,6 +963,35 @@ static void test_seal_exec(void)close (fd); } +/* +* Test memfd_create with MFD_NOEXEC flag +* Test that MFD_NOEXEC applies F_SEAL_EXEC and prevents change of exec bits +*/
tools/testing/selftests/memfd/memfd_test.c static void test_seal_exec(void) · line 979
Quoted 10 lines
tools/testing/selftests/memfd/memfd_test.c +10
-0+static void test_noexec (void ) +{ +int fd; + +printf ("%s NOEXEC\n" , memfd_str); + +/* Create with NOEXEC and ALLOW_SEALING */ + fd =mfd_assert_new ("kern_memfd_noexec" , + mfd_def_size, +MFD_CLOEXEC |MFD_ALLOW_SEALING |MFD_NOEXEC );
Don't we need to check fd here?
tools/testing/selftests/memfd/memfd_test.c static void test_seal_exec(void) · line 988
Quoted 9 lines
tools/testing/selftests/memfd/memfd_test.c +9
-0+mfd_assert_mode (fd,0666 ); +mfd_assert_has_seals (fd,F_SEAL_EXEC ); +mfd_fail_chmod (fd,0777 ); +close (fd); + +/* Create with NOEXEC but without ALLOW_SEALING */ + fd =mfd_assert_new ("kern_memfd_noexec" , + mfd_def_size, +MFD_CLOEXEC |MFD_NOEXEC );
What happens when mfd_assert_new() fails - don't we need to check fd?
Quoted 11 lines
tools/testing/selftests/memfd/memfd_test.c +6
-0+mfd_assert_mode (fd,0666 ); +mfd_assert_has_seals (fd,F_SEAL_EXEC |F_SEAL_SEAL ); +mfd_fail_chmod (fd,0777 ); +close (fd); +} +/* * Test sharing via dup() * Test that seals are shared between dupped FDs and they're all equal.
tools/testing/selftests/memfd/memfd_test.c int main(int argc, char **argv) · line 1171
Quoted 7 lines
tools/testing/selftests/memfd/memfd_test.c +1
-0@@ -1132,6 +1165,7 @@ int main(int argc, char **argv)test_create ();test_basic (); +test_noexec ();test_seal_write ();test_seal_future_write ();
fd isn't checked in the other test F_SEAL_EXEC in the 3/4 patch.
thanks,
-- Shuah