2023-03-31 23:53:21

by Ackerley Tng

[permalink] [raw]
Subject: [RFC PATCH v3 0/2] Providing mount in memfd_restricted() syscall

Hello,

This patchset builds upon the memfd_restricted() system call that was
discussed in the ‘KVM: mm: fd-based approach for supporting KVM’ patch
series, at
https://lore.kernel.org/lkml/[email protected]/T/

The tree can be found at:
https://github.com/googleprodkernel/linux-cc/tree/restrictedmem-provide-mount-fd-rfc-v3

In this patchset, a modification to the memfd_restricted() syscall is
proposed, which allows userspace to provide a mount, on which the
restrictedmem file will be created and returned from the
memfd_restricted().

Allowing userspace to provide a mount allows userspace to control
various memory binding policies via tmpfs mount options, such as
Transparent HugePage memory allocation policy through
‘huge=always/never’ and NUMA memory allocation policy through
‘mpol=local/bind:*’.

Changes since RFCv2:
+ Tightened semantics to accept only fds of the root of a tmpfs mount,
as Christian suggested
+ Added permissions check on the inode represented by the fd to guard
against creation of restrictedmem files on read-only tmpfs
filesystems or mounts
+ Renamed RMFD_TMPFILE to RMFD_USERMNT to better represent providing a
userspace mount to create a restrictedmem file on
+ Updated selftests for tighter semantics and added selftests to check
for permissions

Changes since RFCv1:
+ Use fd to represent mount instead of path string, as Kirill
suggested. I believe using fds makes this syscall interface more
aligned with the other syscalls like fsopen(), fsconfig(), and
fsmount() in terms of using and passing around fds
+ Remove unused variable char *orig_shmem_enabled from selftests

Dependencies:
+ Sean’s iteration of the ‘KVM: mm: fd-based approach for supporting
KVM’ patch series at
https://github.com/sean-jc/linux/tree/x86/upm_base_support
+ Proposed fixes for these issues mentioned on the mailing list:
+ https://lore.kernel.org/lkml/[email protected]/

Links to earlier patch series:
+ RFC v2: https://lore.kernel.org/lkml/[email protected]/T/
+ RFC v1: https://lore.kernel.org/lkml/[email protected]/T/

---

Ackerley Tng (2):
mm: restrictedmem: Allow userspace to specify mount for
memfd_restricted
selftests: restrictedmem: Check hugepage-ness of shmem file backing
restrictedmem fd

include/linux/syscalls.h | 2 +-
include/uapi/linux/restrictedmem.h | 8 +
mm/restrictedmem.c | 74 ++-
tools/testing/selftests/Makefile | 1 +
.../selftests/restrictedmem/.gitignore | 3 +
.../testing/selftests/restrictedmem/Makefile | 15 +
.../testing/selftests/restrictedmem/common.c | 9 +
.../testing/selftests/restrictedmem/common.h | 8 +
.../restrictedmem_hugepage_test.c | 486 ++++++++++++++++++
9 files changed, 599 insertions(+), 7 deletions(-)
create mode 100644 include/uapi/linux/restrictedmem.h
create mode 100644 tools/testing/selftests/restrictedmem/.gitignore
create mode 100644 tools/testing/selftests/restrictedmem/Makefile
create mode 100644 tools/testing/selftests/restrictedmem/common.c
create mode 100644 tools/testing/selftests/restrictedmem/common.h
create mode 100644 tools/testing/selftests/restrictedmem/restrictedmem_hugepage_test.c

--
2.40.0.348.gf938b09366-goog


2023-03-31 23:54:43

by Ackerley Tng

[permalink] [raw]
Subject: [RFC PATCH v3 2/2] selftests: restrictedmem: Check hugepage-ness of shmem file backing restrictedmem fd

For memfd_restricted() calls without a userspace mount, the backing
file should be the shmem mount in the kernel, and the size of backing
pages should be as defined by system-wide shmem configuration.

If a userspace mount is provided, the size of backing pages should be
as defined in the mount.

Also includes negative tests for invalid inputs, including fds
representing read-only superblocks/mounts.

Signed-off-by: Ackerley Tng <[email protected]>
---
tools/testing/selftests/Makefile | 1 +
.../selftests/restrictedmem/.gitignore | 3 +
.../testing/selftests/restrictedmem/Makefile | 15 +
.../testing/selftests/restrictedmem/common.c | 9 +
.../testing/selftests/restrictedmem/common.h | 8 +
.../restrictedmem_hugepage_test.c | 486 ++++++++++++++++++
6 files changed, 522 insertions(+)
create mode 100644 tools/testing/selftests/restrictedmem/.gitignore
create mode 100644 tools/testing/selftests/restrictedmem/Makefile
create mode 100644 tools/testing/selftests/restrictedmem/common.c
create mode 100644 tools/testing/selftests/restrictedmem/common.h
create mode 100644 tools/testing/selftests/restrictedmem/restrictedmem_hugepage_test.c

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index f07aef7c592c..44078eeefb79 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -60,6 +60,7 @@ TARGETS += pstore
TARGETS += ptrace
TARGETS += openat2
TARGETS += resctrl
+TARGETS += restrictedmem
TARGETS += rlimits
TARGETS += rseq
TARGETS += rtc
diff --git a/tools/testing/selftests/restrictedmem/.gitignore b/tools/testing/selftests/restrictedmem/.gitignore
new file mode 100644
index 000000000000..2581bcc8ff29
--- /dev/null
+++ b/tools/testing/selftests/restrictedmem/.gitignore
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+restrictedmem_hugepage_test
diff --git a/tools/testing/selftests/restrictedmem/Makefile b/tools/testing/selftests/restrictedmem/Makefile
new file mode 100644
index 000000000000..8e5378d20226
--- /dev/null
+++ b/tools/testing/selftests/restrictedmem/Makefile
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+
+CFLAGS = $(KHDR_INCLUDES)
+CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -std=gnu99
+
+TEST_GEN_PROGS += restrictedmem_hugepage_test
+
+include ../lib.mk
+
+EXTRA_CLEAN = $(OUTPUT)/common.o
+
+$(OUTPUT)/common.o: common.c
+ $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c -ffreestanding $< -o $@
+
+$(TEST_GEN_PROGS): $(OUTPUT)/common.o
diff --git a/tools/testing/selftests/restrictedmem/common.c b/tools/testing/selftests/restrictedmem/common.c
new file mode 100644
index 000000000000..03dac843404f
--- /dev/null
+++ b/tools/testing/selftests/restrictedmem/common.c
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <sys/syscall.h>
+#include <unistd.h>
+
+int memfd_restricted(unsigned int flags, int mount_fd)
+{
+ return syscall(__NR_memfd_restricted, flags, mount_fd);
+}
diff --git a/tools/testing/selftests/restrictedmem/common.h b/tools/testing/selftests/restrictedmem/common.h
new file mode 100644
index 000000000000..06284ed86baf
--- /dev/null
+++ b/tools/testing/selftests/restrictedmem/common.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef SELFTESTS_RESTRICTEDMEM_COMMON_H
+#define SELFTESTS_RESTRICTEDMEM_COMMON_H
+
+int memfd_restricted(unsigned int flags, int mount_fd);
+
+#endif // SELFTESTS_RESTRICTEDMEM_COMMON_H
diff --git a/tools/testing/selftests/restrictedmem/restrictedmem_hugepage_test.c b/tools/testing/selftests/restrictedmem/restrictedmem_hugepage_test.c
new file mode 100644
index 000000000000..9ed319b83cb8
--- /dev/null
+++ b/tools/testing/selftests/restrictedmem/restrictedmem_hugepage_test.c
@@ -0,0 +1,486 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define _GNU_SOURCE /* for O_PATH */
+#define _POSIX_C_SOURCE /* for PATH_MAX */
+#include <limits.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "linux/restrictedmem.h"
+
+#include "common.h"
+#include "../kselftest_harness.h"
+
+/*
+ * Expect policy to be one of always, within_size, advise, never,
+ * deny, force
+ */
+#define POLICY_BUF_SIZE 12
+
+static int get_hpage_pmd_size(void)
+{
+ FILE *fp;
+ char buf[100];
+ char *ret;
+ int size;
+
+ fp = fopen("/sys/kernel/mm/transparent_hugepage/hpage_pmd_size", "r");
+ if (!fp)
+ return -1;
+
+ ret = fgets(buf, 100, fp);
+ if (ret != buf) {
+ size = -1;
+ goto out;
+ }
+
+ if (sscanf(buf, "%d\n", &size) != 1)
+ size = -1;
+
+out:
+ fclose(fp);
+
+ return size;
+}
+
+static bool is_valid_shmem_thp_policy(char *policy)
+{
+ if (strcmp(policy, "always") == 0)
+ return true;
+ if (strcmp(policy, "within_size") == 0)
+ return true;
+ if (strcmp(policy, "advise") == 0)
+ return true;
+ if (strcmp(policy, "never") == 0)
+ return true;
+ if (strcmp(policy, "deny") == 0)
+ return true;
+ if (strcmp(policy, "force") == 0)
+ return true;
+
+ return false;
+}
+
+static int get_shmem_thp_policy(char *policy)
+{
+ FILE *fp;
+ char buf[100];
+ char *left = NULL;
+ char *right = NULL;
+ int ret = -1;
+
+ fp = fopen("/sys/kernel/mm/transparent_hugepage/shmem_enabled", "r");
+ if (!fp)
+ return -1;
+
+ if (fgets(buf, 100, fp) != buf)
+ goto out;
+
+ /*
+ * Expect shmem_enabled to be of format like "always within_size advise
+ * [never] deny force"
+ */
+ left = memchr(buf, '[', 100);
+ if (!left)
+ goto out;
+
+ right = memchr(buf, ']', 100);
+ if (!right)
+ goto out;
+
+ memcpy(policy, left + 1, right - left - 1);
+
+ ret = !is_valid_shmem_thp_policy(policy);
+
+out:
+ fclose(fp);
+ return ret;
+}
+
+static int write_string_to_file(const char *path, const char *string)
+{
+ FILE *fp;
+ size_t len = strlen(string);
+ int ret = -1;
+
+ fp = fopen(path, "w");
+ if (!fp)
+ return ret;
+
+ if (fwrite(string, 1, len, fp) != len)
+ goto out;
+
+ ret = 0;
+
+out:
+ fclose(fp);
+ return ret;
+}
+
+static int set_shmem_thp_policy(char *policy)
+{
+ int ret = -1;
+ /* +1 for newline */
+ char to_write[POLICY_BUF_SIZE + 1] = { 0 };
+
+ if (!is_valid_shmem_thp_policy(policy))
+ return ret;
+
+ ret = snprintf(to_write, POLICY_BUF_SIZE + 1, "%s\n", policy);
+ if (ret != strlen(policy) + 1)
+ return -1;
+
+ ret = write_string_to_file(
+ "/sys/kernel/mm/transparent_hugepage/shmem_enabled", to_write);
+
+ return ret;
+}
+
+FIXTURE(reset_shmem_enabled)
+{
+ char shmem_enabled[POLICY_BUF_SIZE];
+};
+
+FIXTURE_SETUP(reset_shmem_enabled)
+{
+ memset(self->shmem_enabled, 0, POLICY_BUF_SIZE);
+ ASSERT_EQ(get_shmem_thp_policy(self->shmem_enabled), 0);
+}
+
+FIXTURE_TEARDOWN(reset_shmem_enabled)
+{
+ ASSERT_EQ(set_shmem_thp_policy(self->shmem_enabled), 0);
+}
+
+TEST_F(reset_shmem_enabled, restrictedmem_fstat_shmem_enabled_never)
+{
+ int fd = -1;
+ struct stat stat;
+
+ ASSERT_EQ(set_shmem_thp_policy("never"), 0);
+
+ fd = memfd_restricted(0, -1);
+ ASSERT_GT(fd, 0);
+
+ ASSERT_EQ(fstat(fd, &stat), 0);
+
+ /*
+ * st_blksize is set based on the superblock's s_blocksize_bits. For
+ * shmem, this is set to PAGE_SHIFT
+ */
+ ASSERT_EQ(stat.st_blksize, getpagesize());
+
+ close(fd);
+}
+
+TEST_F(reset_shmem_enabled, restrictedmem_fstat_shmem_enabled_always)
+{
+ int fd = -1;
+ struct stat stat;
+
+ ASSERT_EQ(set_shmem_thp_policy("always"), 0);
+
+ fd = memfd_restricted(0, -1);
+ ASSERT_GT(fd, 0);
+
+ ASSERT_EQ(fstat(fd, &stat), 0);
+
+ ASSERT_EQ(stat.st_blksize, get_hpage_pmd_size());
+
+ close(fd);
+}
+
+TEST(restrictedmem_tmpfile_invalid_fd)
+{
+ int fd = memfd_restricted(RMFD_USERMNT, -2);
+
+ ASSERT_EQ(fd, -1);
+ ASSERT_EQ(errno, EINVAL);
+}
+
+TEST(restrictedmem_tmpfile_fd_not_a_mount)
+{
+ int fd = memfd_restricted(RMFD_USERMNT, STDOUT_FILENO);
+
+ ASSERT_EQ(fd, -1);
+ ASSERT_EQ(errno, EINVAL);
+}
+
+TEST(restrictedmem_tmpfile_not_tmpfs_mount)
+{
+ int fd = -1;
+ int mfd = -1;
+
+ mfd = open("/proc", O_PATH);
+ ASSERT_NE(mfd, -1);
+
+ fd = memfd_restricted(RMFD_USERMNT, mfd);
+
+ ASSERT_EQ(fd, -1);
+ ASSERT_EQ(errno, EINVAL);
+}
+
+FIXTURE(tmpfs_hugepage_sfd)
+{
+ int sfd;
+};
+
+FIXTURE_SETUP(tmpfs_hugepage_sfd)
+{
+ self->sfd = fsopen("tmpfs", 0);
+ ASSERT_NE(self->sfd, -1);
+}
+
+FIXTURE_TEARDOWN(tmpfs_hugepage_sfd)
+{
+ EXPECT_EQ(close(self->sfd), 0);
+}
+
+TEST_F(tmpfs_hugepage_sfd, restrictedmem_fstat_tmpfs_huge_always)
+{
+ int ret = -1;
+ int fd = -1;
+ int mfd = -1;
+ struct stat stat;
+
+ fsconfig(self->sfd, FSCONFIG_SET_STRING, "huge", "always", 0);
+ fsconfig(self->sfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0);
+
+ mfd = fsmount(self->sfd, 0, 0);
+ ASSERT_NE(mfd, -1);
+
+ fd = memfd_restricted(RMFD_USERMNT, mfd);
+ ASSERT_GT(fd, 0);
+
+ /* User can close reference to mount */
+ ret = close(mfd);
+ ASSERT_EQ(ret, 0);
+
+ ret = fstat(fd, &stat);
+ ASSERT_EQ(ret, 0);
+ ASSERT_EQ(stat.st_blksize, get_hpage_pmd_size());
+
+ close(fd);
+}
+
+TEST_F(tmpfs_hugepage_sfd, restrictedmem_fstat_tmpfs_huge_never)
+{
+ int ret = -1;
+ int fd = -1;
+ int mfd = -1;
+ struct stat stat;
+
+ fsconfig(self->sfd, FSCONFIG_SET_STRING, "huge", "never", 0);
+ fsconfig(self->sfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0);
+
+ mfd = fsmount(self->sfd, 0, 0);
+ ASSERT_NE(mfd, -1);
+
+ fd = memfd_restricted(RMFD_USERMNT, mfd);
+ ASSERT_GT(fd, 0);
+
+ /* User can close reference to mount */
+ ret = close(mfd);
+ ASSERT_EQ(ret, 0);
+
+ ret = fstat(fd, &stat);
+ ASSERT_EQ(ret, 0);
+ ASSERT_EQ(stat.st_blksize, getpagesize());
+
+ close(fd);
+}
+
+TEST_F(tmpfs_hugepage_sfd, restrictedmem_check_mount_flags)
+{
+ int ret = -1;
+ int fd = -1;
+ int mfd = -1;
+
+ fsconfig(self->sfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0);
+
+ mfd = fsmount(self->sfd, 0, MOUNT_ATTR_RDONLY);
+ ASSERT_NE(mfd, -1);
+
+ fd = memfd_restricted(RMFD_USERMNT, mfd);
+ ASSERT_EQ(fd, -1);
+ ASSERT_EQ(errno, EROFS);
+
+ ret = close(mfd);
+ ASSERT_EQ(ret, 0);
+}
+
+TEST_F(tmpfs_hugepage_sfd, restrictedmem_check_superblock_flags)
+{
+ int ret = -1;
+ int fd = -1;
+ int mfd = -1;
+
+ fsconfig(self->sfd, FSCONFIG_SET_FLAG, "ro", NULL, 0);
+ fsconfig(self->sfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0);
+
+ mfd = fsmount(self->sfd, 0, 0);
+ ASSERT_NE(mfd, -1);
+
+ fd = memfd_restricted(RMFD_USERMNT, mfd);
+ ASSERT_EQ(fd, -1);
+ ASSERT_EQ(errno, EROFS);
+
+ ret = close(mfd);
+ ASSERT_EQ(ret, 0);
+}
+
+static bool directory_exists(const char *path)
+{
+ struct stat sb;
+
+ return stat(path, &sb) == 0 && S_ISDIR(sb.st_mode);
+}
+
+FIXTURE(tmpfs_hugepage_mount_path)
+{
+ char *mount_path;
+};
+
+FIXTURE_SETUP(tmpfs_hugepage_mount_path)
+{
+ int ret = -1;
+
+ /* /tmp is an FHS-mandated world-writable directory */
+ self->mount_path = "/tmp/restrictedmem-selftest-mnt";
+
+ if (!directory_exists(self->mount_path)) {
+ ret = mkdir(self->mount_path, 0777);
+ ASSERT_EQ(ret, 0);
+ }
+}
+
+FIXTURE_TEARDOWN(tmpfs_hugepage_mount_path)
+{
+ int ret = -1;
+
+ if (!directory_exists(self->mount_path))
+ return;
+
+ ret = umount2(self->mount_path, MNT_FORCE);
+ EXPECT_EQ(ret, 0);
+ if (ret == -1 && errno == EINVAL)
+ fprintf(stderr, " %s was not mounted\n", self->mount_path);
+
+ ret = rmdir(self->mount_path);
+ EXPECT_EQ(ret, 0);
+ if (ret == -1)
+ fprintf(stderr, " rmdir(%s) failed: %m\n", self->mount_path);
+}
+
+/*
+ * memfd_restricted() syscall can only be used with the fd of the root of the
+ * mount. When the restrictedmem's fd is open, a user should not be able to
+ * unmount or remove the mounted directory
+ */
+TEST_F(tmpfs_hugepage_mount_path, restrictedmem_umount_rmdir_while_file_open)
+{
+ int ret = -1;
+ int fd = -1;
+ int mfd = -1;
+ struct stat stat;
+
+ ret = mount("name", self->mount_path, "tmpfs", 0, "huge=always");
+ ASSERT_EQ(ret, 0);
+
+ mfd = open(self->mount_path, O_PATH);
+ ASSERT_NE(mfd, -1);
+
+ fd = memfd_restricted(RMFD_USERMNT, mfd);
+ ASSERT_GT(fd, 0);
+
+ /* We don't need this reference to the mount anymore */
+ ret = close(mfd);
+ ASSERT_EQ(ret, 0);
+
+ /* restrictedmem's fd should still be usable */
+ ret = fstat(fd, &stat);
+ ASSERT_EQ(ret, 0);
+ ASSERT_EQ(stat.st_blksize, get_hpage_pmd_size());
+
+ /* User should not be able to unmount directory */
+ ret = umount2(self->mount_path, MNT_FORCE);
+ ASSERT_EQ(ret, -1);
+ ASSERT_EQ(errno, EBUSY);
+
+ ret = rmdir(self->mount_path);
+ ASSERT_EQ(ret, -1);
+ ASSERT_EQ(errno, EBUSY);
+
+ close(fd);
+}
+
+/* The fd of a file on the mount cannot be provided as mount_fd */
+TEST_F(tmpfs_hugepage_mount_path, restrictedmem_provide_fd_of_file)
+{
+ int ret = -1;
+ int fd = -1;
+ int ffd = -1;
+ char tmp_file_path[PATH_MAX] = { 0 };
+
+ ret = mount("name", self->mount_path, "tmpfs", 0, "huge=always");
+ ASSERT_EQ(ret, 0);
+
+ snprintf(tmp_file_path, PATH_MAX, "%s/tmp-file", self->mount_path);
+ ret = write_string_to_file(tmp_file_path, "filler\n");
+ ASSERT_EQ(ret, 0);
+
+ ffd = open(tmp_file_path, O_RDWR);
+ ASSERT_GT(ffd, 0);
+
+ fd = memfd_restricted(RMFD_USERMNT, ffd);
+ ASSERT_LT(fd, 0);
+ ASSERT_EQ(errno, EINVAL);
+
+ ret = close(ffd);
+ ASSERT_EQ(ret, 0);
+
+ close(fd);
+ remove(tmp_file_path);
+}
+
+/* The fd of files on the mount cannot be provided as mount_fd */
+TEST_F(tmpfs_hugepage_mount_path, restrictedmem_provide_fd_of_file_in_subdir)
+{
+ int ret = -1;
+ int fd = -1;
+ int ffd = -1;
+ char tmp_dir_path[PATH_MAX] = { 0 };
+ char tmp_file_path[PATH_MAX] = { 0 };
+
+ ret = mount("name", self->mount_path, "tmpfs", 0, "huge=always");
+ ASSERT_EQ(ret, 0);
+
+ snprintf(tmp_dir_path, PATH_MAX, "%s/tmp-subdir", self->mount_path);
+ ret = mkdir(tmp_dir_path, 0777);
+ ASSERT_EQ(ret, 0);
+
+ snprintf(tmp_file_path, PATH_MAX, "%s/tmp-subdir/tmp-file",
+ self->mount_path);
+ ret = write_string_to_file(tmp_file_path, "filler\n");
+ ASSERT_EQ(ret, 0);
+
+ ffd = open(tmp_file_path, O_RDWR);
+ ASSERT_NE(ffd, -1);
+
+ fd = memfd_restricted(RMFD_USERMNT, ffd);
+ ASSERT_LT(fd, 0);
+ ASSERT_EQ(errno, EINVAL);
+
+ ret = close(ffd);
+ ASSERT_EQ(ret, 0);
+
+ close(fd);
+ remove(tmp_file_path);
+ rmdir(tmp_dir_path);
+}
+
+TEST_HARNESS_MAIN
--
2.40.0.348.gf938b09366-goog

2023-04-03 08:26:01

by David Hildenbrand

[permalink] [raw]
Subject: Re: [RFC PATCH v3 2/2] selftests: restrictedmem: Check hugepage-ness of shmem file backing restrictedmem fd

On 01.04.23 01:50, Ackerley Tng wrote:
> For memfd_restricted() calls without a userspace mount, the backing
> file should be the shmem mount in the kernel, and the size of backing
> pages should be as defined by system-wide shmem configuration.
>
> If a userspace mount is provided, the size of backing pages should be
> as defined in the mount.
>
> Also includes negative tests for invalid inputs, including fds
> representing read-only superblocks/mounts.
>

When you talk about "hugepage" in this patch, do you mean THP or
hugetlb? I suspect thp, so it might be better to spell that out. IIRC,
there are plans to support actual huge pages in the future, at which
point "hugepage" terminology could be misleading.

> Signed-off-by: Ackerley Tng <[email protected]>
> ---
> tools/testing/selftests/Makefile | 1 +
> .../selftests/restrictedmem/.gitignore | 3 +
> .../testing/selftests/restrictedmem/Makefile | 15 +
> .../testing/selftests/restrictedmem/common.c | 9 +
> .../testing/selftests/restrictedmem/common.h | 8 +
> .../restrictedmem_hugepage_test.c | 486 ++++++++++++++++++
> 6 files changed, 522 insertions(+)
> create mode 100644 tools/testing/selftests/restrictedmem/.gitignore
> create mode 100644 tools/testing/selftests/restrictedmem/Makefile
> create mode 100644 tools/testing/selftests/restrictedmem/common.c
> create mode 100644 tools/testing/selftests/restrictedmem/common.h
> create mode 100644 tools/testing/selftests/restrictedmem/restrictedmem_hugepage_test.c
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index f07aef7c592c..44078eeefb79 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -60,6 +60,7 @@ TARGETS += pstore
> TARGETS += ptrace
> TARGETS += openat2
> TARGETS += resctrl
> +TARGETS += restrictedmem
> TARGETS += rlimits
> TARGETS += rseq
> TARGETS += rtc
> diff --git a/tools/testing/selftests/restrictedmem/.gitignore b/tools/testing/selftests/restrictedmem/.gitignore
> new file mode 100644
> index 000000000000..2581bcc8ff29
> --- /dev/null
> +++ b/tools/testing/selftests/restrictedmem/.gitignore
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +restrictedmem_hugepage_test
> diff --git a/tools/testing/selftests/restrictedmem/Makefile b/tools/testing/selftests/restrictedmem/Makefile
> new file mode 100644
> index 000000000000..8e5378d20226
> --- /dev/null
> +++ b/tools/testing/selftests/restrictedmem/Makefile
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +CFLAGS = $(KHDR_INCLUDES)
> +CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -std=gnu99
> +
> +TEST_GEN_PROGS += restrictedmem_hugepage_test
> +
> +include ../lib.mk
> +
> +EXTRA_CLEAN = $(OUTPUT)/common.o
> +
> +$(OUTPUT)/common.o: common.c
> + $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c -ffreestanding $< -o $@
> +
> +$(TEST_GEN_PROGS): $(OUTPUT)/common.o
> diff --git a/tools/testing/selftests/restrictedmem/common.c b/tools/testing/selftests/restrictedmem/common.c
> new file mode 100644
> index 000000000000..03dac843404f
> --- /dev/null
> +++ b/tools/testing/selftests/restrictedmem/common.c
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <sys/syscall.h>
> +#include <unistd.h>
> +
> +int memfd_restricted(unsigned int flags, int mount_fd)
> +{
> + return syscall(__NR_memfd_restricted, flags, mount_fd);
> +}
> diff --git a/tools/testing/selftests/restrictedmem/common.h b/tools/testing/selftests/restrictedmem/common.h
> new file mode 100644
> index 000000000000..06284ed86baf
> --- /dev/null
> +++ b/tools/testing/selftests/restrictedmem/common.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef SELFTESTS_RESTRICTEDMEM_COMMON_H
> +#define SELFTESTS_RESTRICTEDMEM_COMMON_H
> +
> +int memfd_restricted(unsigned int flags, int mount_fd);
> +
> +#endif // SELFTESTS_RESTRICTEDMEM_COMMON_H
> diff --git a/tools/testing/selftests/restrictedmem/restrictedmem_hugepage_test.c b/tools/testing/selftests/restrictedmem/restrictedmem_hugepage_test.c
> new file mode 100644
> index 000000000000..9ed319b83cb8
> --- /dev/null
> +++ b/tools/testing/selftests/restrictedmem/restrictedmem_hugepage_test.c
> @@ -0,0 +1,486 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#define _GNU_SOURCE /* for O_PATH */
> +#define _POSIX_C_SOURCE /* for PATH_MAX */
> +#include <limits.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <sys/mount.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +
> +#include "linux/restrictedmem.h"
> +
> +#include "common.h"
> +#include "../kselftest_harness.h"
> +
> +/*
> + * Expect policy to be one of always, within_size, advise, never,
> + * deny, force
> + */
> +#define POLICY_BUF_SIZE 12
> +
> +static int get_hpage_pmd_size(void)
> +{
> + FILE *fp;
> + char buf[100];
> + char *ret;
> + int size;
> +
> + fp = fopen("/sys/kernel/mm/transparent_hugepage/hpage_pmd_size", "r");
> + if (!fp)
> + return -1;
> +
> + ret = fgets(buf, 100, fp);
> + if (ret != buf) {
> + size = -1;
> + goto out;
> + }
> +
> + if (sscanf(buf, "%d\n", &size) != 1)
> + size = -1;
> +
> +out:
> + fclose(fp);
> +
> + return size;
> +}
> +
> +static bool is_valid_shmem_thp_policy(char *policy)
> +{
> + if (strcmp(policy, "always") == 0)
> + return true;
> + if (strcmp(policy, "within_size") == 0)
> + return true;
> + if (strcmp(policy, "advise") == 0)
> + return true;
> + if (strcmp(policy, "never") == 0)
> + return true;
> + if (strcmp(policy, "deny") == 0)
> + return true;
> + if (strcmp(policy, "force") == 0)
> + return true;
> +
> + return false;
> +}
> +
> +static int get_shmem_thp_policy(char *policy)
> +{
> + FILE *fp;
> + char buf[100];
> + char *left = NULL;
> + char *right = NULL;
> + int ret = -1;
> +
> + fp = fopen("/sys/kernel/mm/transparent_hugepage/shmem_enabled", "r");
> + if (!fp)
> + return -1;
> +
> + if (fgets(buf, 100, fp) != buf)
> + goto out;
> +
> + /*
> + * Expect shmem_enabled to be of format like "always within_size advise
> + * [never] deny force"
> + */
> + left = memchr(buf, '[', 100);
> + if (!left)
> + goto out;
> +
> + right = memchr(buf, ']', 100);
> + if (!right)
> + goto out;
> +
> + memcpy(policy, left + 1, right - left - 1);
> +
> + ret = !is_valid_shmem_thp_policy(policy);
> +
> +out:
> + fclose(fp);
> + return ret;
> +}
> +
> +static int write_string_to_file(const char *path, const char *string)
> +{
> + FILE *fp;
> + size_t len = strlen(string);
> + int ret = -1;
> +
> + fp = fopen(path, "w");
> + if (!fp)
> + return ret;
> +
> + if (fwrite(string, 1, len, fp) != len)
> + goto out;
> +
> + ret = 0;
> +
> +out:
> + fclose(fp);
> + return ret;
> +}
> +
> +static int set_shmem_thp_policy(char *policy)
> +{
> + int ret = -1;
> + /* +1 for newline */
> + char to_write[POLICY_BUF_SIZE + 1] = { 0 };
> +
> + if (!is_valid_shmem_thp_policy(policy))
> + return ret;
> +
> + ret = snprintf(to_write, POLICY_BUF_SIZE + 1, "%s\n", policy);
> + if (ret != strlen(policy) + 1)
> + return -1;
> +
> + ret = write_string_to_file(
> + "/sys/kernel/mm/transparent_hugepage/shmem_enabled", to_write);
> +
> + return ret;
> +}
> +
> +FIXTURE(reset_shmem_enabled)
> +{
> + char shmem_enabled[POLICY_BUF_SIZE];
> +};
> +
> +FIXTURE_SETUP(reset_shmem_enabled)
> +{
> + memset(self->shmem_enabled, 0, POLICY_BUF_SIZE);
> + ASSERT_EQ(get_shmem_thp_policy(self->shmem_enabled), 0);
> +}
> +
> +FIXTURE_TEARDOWN(reset_shmem_enabled)
> +{
> + ASSERT_EQ(set_shmem_thp_policy(self->shmem_enabled), 0);
> +}
> +
> +TEST_F(reset_shmem_enabled, restrictedmem_fstat_shmem_enabled_never)
> +{
> + int fd = -1;
> + struct stat stat;
> +
> + ASSERT_EQ(set_shmem_thp_policy("never"), 0);
> +
> + fd = memfd_restricted(0, -1);
> + ASSERT_GT(fd, 0);
> +
> + ASSERT_EQ(fstat(fd, &stat), 0);
> +
> + /*
> + * st_blksize is set based on the superblock's s_blocksize_bits. For
> + * shmem, this is set to PAGE_SHIFT
> + */
> + ASSERT_EQ(stat.st_blksize, getpagesize());
> +
> + close(fd);
> +}
> +
> +TEST_F(reset_shmem_enabled, restrictedmem_fstat_shmem_enabled_always)
> +{
> + int fd = -1;
> + struct stat stat;
> +
> + ASSERT_EQ(set_shmem_thp_policy("always"), 0);
> +
> + fd = memfd_restricted(0, -1);
> + ASSERT_GT(fd, 0);
> +
> + ASSERT_EQ(fstat(fd, &stat), 0);
> +
> + ASSERT_EQ(stat.st_blksize, get_hpage_pmd_size());
> +
> + close(fd);
> +}
> +
> +TEST(restrictedmem_tmpfile_invalid_fd)
> +{
> + int fd = memfd_restricted(RMFD_USERMNT, -2);
> +
> + ASSERT_EQ(fd, -1);
> + ASSERT_EQ(errno, EINVAL);
> +}
> +
> +TEST(restrictedmem_tmpfile_fd_not_a_mount)
> +{
> + int fd = memfd_restricted(RMFD_USERMNT, STDOUT_FILENO);
> +
> + ASSERT_EQ(fd, -1);
> + ASSERT_EQ(errno, EINVAL);
> +}
> +
> +TEST(restrictedmem_tmpfile_not_tmpfs_mount)
> +{
> + int fd = -1;
> + int mfd = -1;
> +
> + mfd = open("/proc", O_PATH);
> + ASSERT_NE(mfd, -1);
> +
> + fd = memfd_restricted(RMFD_USERMNT, mfd);
> +
> + ASSERT_EQ(fd, -1);
> + ASSERT_EQ(errno, EINVAL);
> +}
> +
> +FIXTURE(tmpfs_hugepage_sfd)
> +{
> + int sfd;
> +};
> +
> +FIXTURE_SETUP(tmpfs_hugepage_sfd)
> +{
> + self->sfd = fsopen("tmpfs", 0);
> + ASSERT_NE(self->sfd, -1);
> +}
> +
> +FIXTURE_TEARDOWN(tmpfs_hugepage_sfd)
> +{
> + EXPECT_EQ(close(self->sfd), 0);
> +}
> +
> +TEST_F(tmpfs_hugepage_sfd, restrictedmem_fstat_tmpfs_huge_always)
> +{
> + int ret = -1;
> + int fd = -1;
> + int mfd = -1;
> + struct stat stat;
> +
> + fsconfig(self->sfd, FSCONFIG_SET_STRING, "huge", "always", 0);
> + fsconfig(self->sfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0);
> +
> + mfd = fsmount(self->sfd, 0, 0);
> + ASSERT_NE(mfd, -1);
> +
> + fd = memfd_restricted(RMFD_USERMNT, mfd);
> + ASSERT_GT(fd, 0);
> +
> + /* User can close reference to mount */
> + ret = close(mfd);
> + ASSERT_EQ(ret, 0);
> +
> + ret = fstat(fd, &stat);
> + ASSERT_EQ(ret, 0);
> + ASSERT_EQ(stat.st_blksize, get_hpage_pmd_size());
> +
> + close(fd);
> +}
> +
> +TEST_F(tmpfs_hugepage_sfd, restrictedmem_fstat_tmpfs_huge_never)
> +{
> + int ret = -1;
> + int fd = -1;
> + int mfd = -1;
> + struct stat stat;
> +
> + fsconfig(self->sfd, FSCONFIG_SET_STRING, "huge", "never", 0);
> + fsconfig(self->sfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0);
> +
> + mfd = fsmount(self->sfd, 0, 0);
> + ASSERT_NE(mfd, -1);
> +
> + fd = memfd_restricted(RMFD_USERMNT, mfd);
> + ASSERT_GT(fd, 0);
> +
> + /* User can close reference to mount */
> + ret = close(mfd);
> + ASSERT_EQ(ret, 0);
> +
> + ret = fstat(fd, &stat);
> + ASSERT_EQ(ret, 0);
> + ASSERT_EQ(stat.st_blksize, getpagesize());
> +
> + close(fd);
> +}
> +
> +TEST_F(tmpfs_hugepage_sfd, restrictedmem_check_mount_flags)
> +{
> + int ret = -1;
> + int fd = -1;
> + int mfd = -1;
> +
> + fsconfig(self->sfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0);
> +
> + mfd = fsmount(self->sfd, 0, MOUNT_ATTR_RDONLY);
> + ASSERT_NE(mfd, -1);
> +
> + fd = memfd_restricted(RMFD_USERMNT, mfd);
> + ASSERT_EQ(fd, -1);
> + ASSERT_EQ(errno, EROFS);
> +
> + ret = close(mfd);
> + ASSERT_EQ(ret, 0);
> +}
> +
> +TEST_F(tmpfs_hugepage_sfd, restrictedmem_check_superblock_flags)
> +{
> + int ret = -1;
> + int fd = -1;
> + int mfd = -1;
> +
> + fsconfig(self->sfd, FSCONFIG_SET_FLAG, "ro", NULL, 0);
> + fsconfig(self->sfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0);
> +
> + mfd = fsmount(self->sfd, 0, 0);
> + ASSERT_NE(mfd, -1);
> +
> + fd = memfd_restricted(RMFD_USERMNT, mfd);
> + ASSERT_EQ(fd, -1);
> + ASSERT_EQ(errno, EROFS);
> +
> + ret = close(mfd);
> + ASSERT_EQ(ret, 0);
> +}
> +
> +static bool directory_exists(const char *path)
> +{
> + struct stat sb;
> +
> + return stat(path, &sb) == 0 && S_ISDIR(sb.st_mode);
> +}
> +
> +FIXTURE(tmpfs_hugepage_mount_path)
> +{
> + char *mount_path;
> +};
> +
> +FIXTURE_SETUP(tmpfs_hugepage_mount_path)
> +{
> + int ret = -1;
> +
> + /* /tmp is an FHS-mandated world-writable directory */
> + self->mount_path = "/tmp/restrictedmem-selftest-mnt";
> +
> + if (!directory_exists(self->mount_path)) {
> + ret = mkdir(self->mount_path, 0777);
> + ASSERT_EQ(ret, 0);
> + }
> +}
> +
> +FIXTURE_TEARDOWN(tmpfs_hugepage_mount_path)
> +{
> + int ret = -1;
> +
> + if (!directory_exists(self->mount_path))
> + return;
> +
> + ret = umount2(self->mount_path, MNT_FORCE);
> + EXPECT_EQ(ret, 0);
> + if (ret == -1 && errno == EINVAL)
> + fprintf(stderr, " %s was not mounted\n", self->mount_path);
> +
> + ret = rmdir(self->mount_path);
> + EXPECT_EQ(ret, 0);
> + if (ret == -1)
> + fprintf(stderr, " rmdir(%s) failed: %m\n", self->mount_path);
> +}
> +
> +/*
> + * memfd_restricted() syscall can only be used with the fd of the root of the
> + * mount. When the restrictedmem's fd is open, a user should not be able to
> + * unmount or remove the mounted directory
> + */
> +TEST_F(tmpfs_hugepage_mount_path, restrictedmem_umount_rmdir_while_file_open)
> +{
> + int ret = -1;
> + int fd = -1;
> + int mfd = -1;
> + struct stat stat;
> +
> + ret = mount("name", self->mount_path, "tmpfs", 0, "huge=always");
> + ASSERT_EQ(ret, 0);
> +
> + mfd = open(self->mount_path, O_PATH);
> + ASSERT_NE(mfd, -1);
> +
> + fd = memfd_restricted(RMFD_USERMNT, mfd);
> + ASSERT_GT(fd, 0);
> +
> + /* We don't need this reference to the mount anymore */
> + ret = close(mfd);
> + ASSERT_EQ(ret, 0);
> +
> + /* restrictedmem's fd should still be usable */
> + ret = fstat(fd, &stat);
> + ASSERT_EQ(ret, 0);
> + ASSERT_EQ(stat.st_blksize, get_hpage_pmd_size());
> +
> + /* User should not be able to unmount directory */
> + ret = umount2(self->mount_path, MNT_FORCE);
> + ASSERT_EQ(ret, -1);
> + ASSERT_EQ(errno, EBUSY);
> +
> + ret = rmdir(self->mount_path);
> + ASSERT_EQ(ret, -1);
> + ASSERT_EQ(errno, EBUSY);
> +
> + close(fd);
> +}
> +
> +/* The fd of a file on the mount cannot be provided as mount_fd */
> +TEST_F(tmpfs_hugepage_mount_path, restrictedmem_provide_fd_of_file)
> +{
> + int ret = -1;
> + int fd = -1;
> + int ffd = -1;
> + char tmp_file_path[PATH_MAX] = { 0 };
> +
> + ret = mount("name", self->mount_path, "tmpfs", 0, "huge=always");
> + ASSERT_EQ(ret, 0);
> +
> + snprintf(tmp_file_path, PATH_MAX, "%s/tmp-file", self->mount_path);
> + ret = write_string_to_file(tmp_file_path, "filler\n");
> + ASSERT_EQ(ret, 0);
> +
> + ffd = open(tmp_file_path, O_RDWR);
> + ASSERT_GT(ffd, 0);
> +
> + fd = memfd_restricted(RMFD_USERMNT, ffd);
> + ASSERT_LT(fd, 0);
> + ASSERT_EQ(errno, EINVAL);
> +
> + ret = close(ffd);
> + ASSERT_EQ(ret, 0);
> +
> + close(fd);
> + remove(tmp_file_path);
> +}
> +
> +/* The fd of files on the mount cannot be provided as mount_fd */
> +TEST_F(tmpfs_hugepage_mount_path, restrictedmem_provide_fd_of_file_in_subdir)
> +{
> + int ret = -1;
> + int fd = -1;
> + int ffd = -1;
> + char tmp_dir_path[PATH_MAX] = { 0 };
> + char tmp_file_path[PATH_MAX] = { 0 };
> +
> + ret = mount("name", self->mount_path, "tmpfs", 0, "huge=always");
> + ASSERT_EQ(ret, 0);
> +
> + snprintf(tmp_dir_path, PATH_MAX, "%s/tmp-subdir", self->mount_path);
> + ret = mkdir(tmp_dir_path, 0777);
> + ASSERT_EQ(ret, 0);
> +
> + snprintf(tmp_file_path, PATH_MAX, "%s/tmp-subdir/tmp-file",
> + self->mount_path);
> + ret = write_string_to_file(tmp_file_path, "filler\n");
> + ASSERT_EQ(ret, 0);
> +
> + ffd = open(tmp_file_path, O_RDWR);
> + ASSERT_NE(ffd, -1);
> +
> + fd = memfd_restricted(RMFD_USERMNT, ffd);
> + ASSERT_LT(fd, 0);
> + ASSERT_EQ(errno, EINVAL);
> +
> + ret = close(ffd);
> + ASSERT_EQ(ret, 0);
> +
> + close(fd);
> + remove(tmp_file_path);
> + rmdir(tmp_dir_path);
> +}
> +
> +TEST_HARNESS_MAIN

--
Thanks,

David / dhildenb

2023-04-11 01:52:01

by Ackerley Tng

[permalink] [raw]
Subject: Re: [RFC PATCH v3 2/2] selftests: restrictedmem: Check hugepage-ness of shmem file backing restrictedmem fd

David Hildenbrand <[email protected]> writes:

> On 01.04.23 01:50, Ackerley Tng wrote:
>> For memfd_restricted() calls without a userspace mount, the backing
>> file should be the shmem mount in the kernel, and the size of backing
>> pages should be as defined by system-wide shmem configuration.

>> If a userspace mount is provided, the size of backing pages should be
>> as defined in the mount.

>> Also includes negative tests for invalid inputs, including fds
>> representing read-only superblocks/mounts.


> When you talk about "hugepage" in this patch, do you mean THP or
> hugetlb? I suspect thp, so it might be better to spell that out. IIRC,
> there are plans to support actual huge pages in the future, at which
> point "hugepage" terminology could be misleading.


Thanks for pointing this out! I've replaced references to hugepage with
thp, please see RFC v4 at
https://lore.kernel.org/lkml/[email protected]/T/

>> Signed-off-by: Ackerley Tng <[email protected]>
>> ---
>> tools/testing/selftests/Makefile | 1 +
>> .../selftests/restrictedmem/.gitignore | 3 +
>> .../testing/selftests/restrictedmem/Makefile | 15 +
>> .../testing/selftests/restrictedmem/common.c | 9 +
>> .../testing/selftests/restrictedmem/common.h | 8 +
>> .../restrictedmem_hugepage_test.c | 486 ++++++++++++++++++
>> 6 files changed, 522 insertions(+)
>> create mode 100644 tools/testing/selftests/restrictedmem/.gitignore
>> create mode 100644 tools/testing/selftests/restrictedmem/Makefile
>> create mode 100644 tools/testing/selftests/restrictedmem/common.c
>> create mode 100644 tools/testing/selftests/restrictedmem/common.h
>> create mode 100644
>> tools/testing/selftests/restrictedmem/restrictedmem_hugepage_test.c

>> ...