Skip to contentSkip to conversation

Providing mount in memfd_restricted() syscall

RFC PATCH Superseded Ackerley Tng 2 participants 5 messages Mar 21, 2023 – Mar 31, 2023

Subsystems (7)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover 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/#m7e944d7892afdd1d62a03a287bd488c56e377b0c

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

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 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 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                            |  63 ++-
 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             | 459 ++++++++++++++++++
 9 files changed, 561 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.rc2.332.ga46443480c-goog

#1 lore raw

RFC PATCH v2 1/2 mm: restrictedmem: Allow userspace to specify mount for memfd_restricted

By default, the backing shmem file for a restrictedmem fd is created on shmem's kernel space mount.

With this patch, an optional tmpfs mount can be specified via an fd, which will be used as the mountpoint for backing the shmem file associated with a restrictedmem fd.

This change is modeled after how sys_open() can create an unnamed temporary file in a given directory with O_TMPFILE.

This will help restrictedmem fds inherit the properties of the provided tmpfs mounts, for example, hugepage allocation hints, NUMA binding hints, etc.

---
 include/linux/syscalls.h           |  2 +-
 include/uapi/linux/restrictedmem.h |  8 ++++
 mm/restrictedmem.c                 | 63 +++++++++++++++++++++++++++---
 3 files changed, 66 insertions(+), 7 deletions(-)
 create mode 100644 include/uapi/linux/restrictedmem.h
include/linux/syscalls.h +1 -1
index f9e9e0c820c5..a23c4c385cd3 100644
@@ -1056,7 +1056,7 @@ asmlinkage long sys_memfd_secret(unsigned int flags);
 asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
 					    unsigned long home_node,
 					    unsigned long flags);
-asmlinkage long sys_memfd_restricted(unsigned int flags);
+asmlinkage long sys_memfd_restricted(unsigned int flags, int mount_fd);
 
 /*
  * Architecture-specific system calls
include/uapi/linux/restrictedmem.h new file +8 -0
new file mode 100644
index 000000000000..9f108dd1ac4c
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_RESTRICTEDMEM_H
+#define _UAPI_LINUX_RESTRICTEDMEM_H
+
+/* flags for memfd_restricted */
+#define RMFD_TMPFILE		0x0001U
+
+#endif /* _UAPI_LINUX_RESTRICTEDMEM_H */
mm/restrictedmem.c +57 -6

93 lines not on this page · raw message

Signature · 1 line
2.40.0.rc2.332.ga46443480c-goog
#2 lore raw

Re: [RFC PATCH v2 1/2] mm: restrictedmem: Allow userspace to specify mount for memfd_restricted

Quoted 108 lines · Ackerley Tng

By default, the backing shmem file for a restrictedmem fd is created on shmem's kernel space mount.

With this patch, an optional tmpfs mount can be specified via an fd, which will be used as the mountpoint for backing the shmem file associated with a restrictedmem fd.

This change is modeled after how sys_open() can create an unnamed temporary file in a given directory with O_TMPFILE.

This will help restrictedmem fds inherit the properties of the provided tmpfs mounts, for example, hugepage allocation hints, NUMA binding hints, etc.

---
 include/linux/syscalls.h           |  2 +-
 include/uapi/linux/restrictedmem.h |  8 ++++
 mm/restrictedmem.c                 | 63 +++++++++++++++++++++++++++---
 3 files changed, 66 insertions(+), 7 deletions(-)
 create mode 100644 include/uapi/linux/restrictedmem.h
include/linux/syscalls.h +1 -1
index f9e9e0c820c5..a23c4c385cd3 100644
@@ -1056,7 +1056,7 @@ asmlinkage long sys_memfd_secret(unsigned int flags);
 asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
 					    unsigned long home_node,
 					    unsigned long flags);
-asmlinkage long sys_memfd_restricted(unsigned int flags);
+asmlinkage long sys_memfd_restricted(unsigned int flags, int mount_fd);
 
 /*
  * Architecture-specific system calls
include/uapi/linux/restrictedmem.h new file +8 -0
new file mode 100644
index 000000000000..9f108dd1ac4c
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_RESTRICTEDMEM_H
+#define _UAPI_LINUX_RESTRICTEDMEM_H
+
+/* flags for memfd_restricted */
+#define RMFD_TMPFILE		0x0001U
+
+#endif /* _UAPI_LINUX_RESTRICTEDMEM_H */
mm/restrictedmem.c +19 -6
index c5d869d8c2d8..4d83b949d84e 100644
@@ -1,11 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0
-#include "linux/sbitmap.h"
+#include <linux/namei.h>
 #include <linux/pagemap.h>
 #include <linux/pseudo_fs.h>
 #include <linux/shmem_fs.h>
 #include <linux/syscalls.h>
 #include <uapi/linux/falloc.h>
 #include <uapi/linux/magic.h>
+#include <uapi/linux/restrictedmem.h>
 #include <linux/restrictedmem.h>
 
 struct restrictedmem {
@@ -189,19 +190,20 @@ static struct file *restrictedmem_file_create(struct file *memfd)
 	return file;
 }
 
-SYSCALL_DEFINE1(memfd_restricted, unsigned int, flags)
+static int restrictedmem_create(struct vfsmount *mount)
 {
 	struct file *file, *restricted_file;
 	int fd, err;
 
-	if (flags)
-		return -EINVAL;
-
 	fd = get_unused_fd_flags(0);
 	if (fd < 0)
 		return fd;
 
-	file = shmem_file_setup("memfd:restrictedmem", 0, VM_NORESERVE);
+	if (mount)
+		file = shmem_file_setup_with_mnt(mount, "memfd:restrictedmem", 0, VM_NORESERVE);
+	else
+		file = shmem_file_setup("memfd:restrictedmem", 0, VM_NORESERVE);
+
 	if (IS_ERR(file)) {
 		err = PTR_ERR(file);
 		goto err_fd;
@@ -223,6 +225,55 @@ SYSCALL_DEFINE1(memfd_restricted, unsigned int, flags)
 	return err;
 }
 
+static bool is_shmem_mount(struct vfsmount *mnt)
+{
+	return mnt && mnt->mnt_sb && mnt->mnt_sb->s_magic == TMPFS_MAGIC;
+}
+
+static int restrictedmem_create_from_file(int mount_fd)
+{
+	int ret;
+	struct fd f;
+	struct vfsmount *mnt;
+

mm/restrictedmem.c SYSCALL_DEFINE1(memfd_restricted, unsigned int, flags) · line 248

Quoted 10 lines
mm/restrictedmem.c +10 -0
+	f = fdget_raw(mount_fd);
+	if (!f.file)
+		return -EBADF;
+
+	mnt = f.file->f_path.mnt;
+	if (!is_shmem_mount(mnt)) {
+		ret = -EINVAL;
+		goto out;
+	}
+

This looks like you can just pass in some tmpfs fd and you just use it to identify the mnt and then you create a restricted memfd area in that instance. So if I did:

mount -t tmpfs tmpfs /mnt
mknod /mnt/bla c 0 0
fd = open("/mnt/bla")
memfd_restricted(fd)

then it would create a memfd restricted entry in the tmpfs instance using the arbitrary dummy device node to infer the tmpfs instance.

Looking at the older thread briefly and the cover letter. Afaict, the new mount api shouldn't figure into the design of this. fsopen() returns fds referencing a VFS-internal fs_context object. They can't be used to create or lookup files or identify mounts. The mount doesn't exist at that time. Not even a superblock might exist at the time before fsconfig(FSCONFIG_CMD_CREATE).

When fsmount() is called after superblock setup then it's similar to any other fd from open() or open_tree() or whatever (glossing over some details that are irrelevant here). Difference is that open_tree() and fsmount() would refer to the root of a mount.

At first I wondered why this doesn't just use standard *at() semantics but I guess the restricted memfd is unlinked and doesn't show up in the tmpfs instance.

So if you go down that route then I would suggest to enforce that the provided fd refer to the root of a tmpfs mount. IOW, it can't just be an arbitrary file descriptor in a tmpfs instance. That seems cleaner to me:

sb = f_path->mnt->mnt_sb;
sb->s_magic == TMPFS_MAGIC && f_path->mnt->mnt_root == sb->s_root

and has much tigher semantics than just allowing any kind of fd.

Another wrinkly I find odd but that's for you to judge is that this bypasses the permission model of the tmpfs instance. IOW, as long as you have a handle to the root of a tmpfs mount you can just create restricted memfds in there. So if I provided a completely sandboxed service - running in a user namespace or whatever - with an fd to the host's tmpfs instance they can just create restricted memfds in there no questions asked.

Maybe that's fine but it's certainly something to spell out and think about the implications.

#3 lore raw

Re: [RFC PATCH v2 1/2] mm: restrictedmem: Allow userspace to specify mount for memfd_restricted

Quoted 5 lines · Christian Brauner
Quoted 4 lines · Ackerley Tng

By default, the backing shmem file for a restrictedmem fd is created on shmem's kernel space mount.

...

Thanks for reviewing this patch!

Quoted 19 lines

This looks like you can just pass in some tmpfs fd and you just use it to identify the mnt and then you create a restricted memfd area in that instance. So if I did:

mount -t tmpfs tmpfs /mnt
mknod /mnt/bla c 0 0
fd = open("/mnt/bla")
memfd_restricted(fd)

then it would create a memfd restricted entry in the tmpfs instance using the arbitrary dummy device node to infer the tmpfs instance.

Looking at the older thread briefly and the cover letter. Afaict, the new mount api shouldn't figure into the design of this. fsopen() returns fds referencing a VFS-internal fs_context object. They can't be used to create or lookup files or identify mounts. The mount doesn't exist at that time. Not even a superblock might exist at the time before fsconfig(FSCONFIG_CMD_CREATE).

Quoted 4 lines

When fsmount() is called after superblock setup then it's similar to any other fd from open() or open_tree() or whatever (glossing over some details that are irrelevant here). Difference is that open_tree() and fsmount() would refer to the root of a mount.

This is correct, memfd_restricted() needs an fd returned from fsmount() and not fsopen(). Usage examples of this new parameter in memfd_restricted() are available in selftests.

Quoted 4 lines

At first I wondered why this doesn't just use standard *at() semantics but I guess the restricted memfd is unlinked and doesn't show up in the tmpfs instance.

Quoted 8 lines

So if you go down that route then I would suggest to enforce that the provided fd refer to the root of a tmpfs mount. IOW, it can't just be an arbitrary file descriptor in a tmpfs instance. That seems cleaner to me:

sb = f_path->mnt->mnt_sb;
sb->s_magic == TMPFS_MAGIC && f_path->mnt->mnt_root == sb->s_root

and has much tigher semantics than just allowing any kind of fd.

Thanks for your suggestion, I've tightened the semantics as you suggested. memfd_restricted() now only accepts fds representing the root of the mount.

Quoted 10 lines

Another wrinkly I find odd but that's for you to judge is that this bypasses the permission model of the tmpfs instance. IOW, as long as you have a handle to the root of a tmpfs mount you can just create restricted memfds in there. So if I provided a completely sandboxed service - running in a user namespace or whatever - with an fd to the host's tmpfs instance they can just create restricted memfds in there no questions asked.

Maybe that's fine but it's certainly something to spell out and think about the implications.

Thanks for pointing this out! I added a permissions check in RFC v3, and clarified the permissions model (please see patch 1 of 2):
https://lore.kernel.org/lkml/[email protected]/

#4 lore raw

RFC PATCH v2 2/2 selftests: restrictedmem: Check hugepage-ness of shmem file backing restrictedmem fd

In reply to Ackerley Tng

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.

---
 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             | 459 ++++++++++++++++++
 6 files changed, 495 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
tools/testing/selftests/Makefile +1 -0
index f07aef7c592c..44078eeefb79 100644
@@ -60,6 +60,7 @@ TARGETS += pstore
 TARGETS += ptrace
 TARGETS += openat2
 TARGETS += resctrl
+TARGETS += restrictedmem
 TARGETS += rlimits
 TARGETS += rseq
 TARGETS += rtc
tools/testing/selftests/restrictedmem/.gitignore new file +3 -0
new file mode 100644
index 000000000000..2581bcc8ff29
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+restrictedmem_hugepage_test
tools/testing/selftests/restrictedmem/Makefile new file +15 -0
new file mode 100644
index 000000000000..8e5378d20226
@@ -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
tools/testing/selftests/restrictedmem/common.c new file +9 -0
new file mode 100644
index 000000000000..03dac843404f
@@ -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);
+}
tools/testing/selftests/restrictedmem/common.h new file +8 -0
new file mode 100644
index 000000000000..06284ed86baf
@@ -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
tools/testing/selftests/restrictedmem/restrictedmem_hugepage_test.c new file +459 -0

459 lines not on this page · raw message

Signature · 1 line
2.40.0.rc2.332.ga46443480c-goog