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

"Kirill A. Shutemov" [off-list ref] writes:

On Thu, Feb 16, 2023 at 12:41:16AM +0000, Ackerley Tng wrote:

quoted

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

quoted

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

quoted

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

quoted

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

quoted

Signed-off-by: Ackerley Tng <redacted>
---
  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

quoted

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index f9e9e0c820c5..4b8efe9a8680 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h

@@ -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, const char __user *mount_path);

quoted

  /*
   * Architecture-specific system calls

I'm not sure what the right practice now: do we provide string that
contains mount path or fd that represents the filesystem (returned from
fsmount(2) or open_tree(2)).

fd seems more flexible: it allows to specify unbind mounts.

I tried out the suggestion of passing fds to memfd_restricted() instead
of strings.

One benefit I see of using fds is interface uniformity: it feels more
aligned with other syscalls like fsopen(), fsconfig(), and fsmount() in
terms of using and passing around fds.

Other than being able to use a mount without a path attached to the
mount, are there any other benefits of using fds over using the path string?

Should I post the patches that allows specifying a mount using fds?
Should I post them as a separate RFC, or as a new revision to this RFC?

On Thu, Feb 23, 2023 at 12:55:16AM +0000, Ackerley Tng wrote:

"Kirill A. Shutemov" [off-list ref] writes:

quoted

On Thu, Feb 16, 2023 at 12:41:16AM +0000, Ackerley Tng wrote:

quoted

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

quoted

quoted

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

quoted

quoted

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

quoted

quoted

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

quoted

quoted

Signed-off-by: Ackerley Tng <redacted>
---
  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

quoted

quoted

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index f9e9e0c820c5..4b8efe9a8680 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h

@@ -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, const char
__user *mount_path);

quoted

quoted

  /*
   * Architecture-specific system calls

quoted

I'm not sure what the right practice now: do we provide string that
contains mount path or fd that represents the filesystem (returned from
fsmount(2) or open_tree(2)).

quoted

fd seems more flexible: it allows to specify unbind mounts.

I tried out the suggestion of passing fds to memfd_restricted() instead
of strings.

One benefit I see of using fds is interface uniformity: it feels more
aligned with other syscalls like fsopen(), fsconfig(), and fsmount() in
terms of using and passing around fds.

Other than being able to use a mount without a path attached to the
mount, are there any other benefits of using fds over using the path string?

It would be nice if anyone from fs folks comment on this.

Should I post the patches that allows specifying a mount using fds?
Should I post them as a separate RFC, or as a new revision to this RFC?

Let's first decide what the right direction is.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov