Re: [PATCH 3/4] selftests/memfd: add tests for F_SEAL_EXEC

On 4/1/22 4:08 PM, Daniel Verkamp wrote:

Basic tests to ensure that user/group/other execute bits cannot be changed after applying F_SEAL_EXEC to a memfd.

Signed-off-by: Daniel Verkamp dverkamp@chromium.org

tools/testing/selftests/memfd/memfd_test.c | 80 ++++++++++++++++++++++ 1 file changed, 80 insertions(+)

diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/selftests/memfd/memfd_test.c index 94df2692e6e4..fdb0e46e9df9 100644 --- a/tools/testing/selftests/memfd/memfd_test.c +++ b/tools/testing/selftests/memfd/memfd_test.c @@ -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);
  

Let's print the filename here - just printing fd isn't useful.

• 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.

• 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.

• 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

• abort();
  

• }
• if (fchmod(fd, mode) == 0) {

• printf("fchmod(0%04o) didn't fail as expected\n");
  

Same comment about filename

• 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", "");

The rest looks good.

thanks, -- Shuah