Re: [PATCH RFC v3 2/2] fhandle: expose u64 mount id to name_to_handle_at(2)

From: Jeff Layton
Date: Thu Aug 01 2024 - 15:46:52 EST


On Thu, 2024-08-01 at 13:52 +1000, Aleksa Sarai wrote:
> Now that we provide a unique 64-bit mount ID interface in statx(2), we
> can now provide a race-free way for name_to_handle_at(2) to provide a
> file handle and corresponding mount without needing to worry about
> racing with /proc/mountinfo parsing or having to open a file just to do
> statx(2).
>
> While this is not necessary if you are using AT_EMPTY_PATH and don't
> care about an extra statx(2) call, users that pass full paths into
> name_to_handle_at(2) need to know which mount the file handle comes from
> (to make sure they don't try to open_by_handle_at a file handle from a
> different filesystem) and switching to AT_EMPTY_PATH would require
> allocating a file for every name_to_handle_at(2) call, turning
>
>   err = name_to_handle_at(-EBADF, "/foo/bar/baz", &handle, &mntid,
>                           AT_HANDLE_MNT_ID_UNIQUE);
>
> into
>
>   int fd = openat(-EBADF, "/foo/bar/baz", O_PATH | O_CLOEXEC);
>   err1 = name_to_handle_at(fd, "", &handle, &unused_mntid, AT_EMPTY_PATH);
>   err2 = statx(fd, "", AT_EMPTY_PATH, STATX_MNT_ID_UNIQUE, &statxbuf);
>   mntid = statxbuf.stx_mnt_id;
>   close(fd);
>
> Signed-off-by: Aleksa Sarai <cyphar@xxxxxxxxxx>
> ---
>  fs/fhandle.c                                       | 29 ++++++++++++++++------
>  include/linux/syscalls.h                           |  2 +-
>  include/uapi/linux/fcntl.h                         |  1 +
>  tools/perf/trace/beauty/include/uapi/linux/fcntl.h |  1 +
>  4 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/fs/fhandle.c b/fs/fhandle.c
> index 6e8cea16790e..8cb665629f4a 100644
> --- a/fs/fhandle.c
> +++ b/fs/fhandle.c
> @@ -16,7 +16,8 @@
>  
>  static long do_sys_name_to_handle(const struct path *path,
>     struct file_handle __user *ufh,
> -   int __user *mnt_id, int fh_flags)
> +   void __user *mnt_id, bool unique_mntid,
> +   int fh_flags)
>  {
>   long retval;
>   struct file_handle f_handle;
> @@ -69,9 +70,19 @@ static long do_sys_name_to_handle(const struct path *path,
>   } else
>   retval = 0;
>   /* copy the mount id */
> - if (put_user(real_mount(path->mnt)->mnt_id, mnt_id) ||
> -     copy_to_user(ufh, handle,
> - struct_size(handle, f_handle, handle_bytes)))
> + if (unique_mntid) {
> + if (put_user(real_mount(path->mnt)->mnt_id_unique,
> +      (u64 __user *) mnt_id))
> + retval = -EFAULT;
> + } else {
> + if (put_user(real_mount(path->mnt)->mnt_id,
> +      (int __user *) mnt_id))
> + retval = -EFAULT;
> + }
> + /* copy the handle */
> + if (retval != -EFAULT &&
> + copy_to_user(ufh, handle,
> +      struct_size(handle, f_handle, handle_bytes)))
>   retval = -EFAULT;
>   kfree(handle);
>   return retval;
> @@ -83,6 +94,7 @@ static long do_sys_name_to_handle(const struct path *path,
>   * @name: name that should be converted to handle.
>   * @handle: resulting file handle
>   * @mnt_id: mount id of the file system containing the file
> + *          (u64 if AT_HANDLE_MNT_ID_UNIQUE, otherwise int)
>   * @flag: flag value to indicate whether to follow symlink or not
>   *        and whether a decodable file handle is required.
>   *
> @@ -92,7 +104,7 @@ static long do_sys_name_to_handle(const struct path *path,
>   * value required.
>   */
>  SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
> - struct file_handle __user *, handle, int __user *, mnt_id,
> + struct file_handle __user *, handle, void __user *, mnt_id,
>   int, flag)
>  {
>   struct path path;
> @@ -100,7 +112,8 @@ SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
>   int fh_flags;
>   int err;
>  
> - if (flag & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH | AT_HANDLE_FID))
> + if (flag & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH | AT_HANDLE_FID |
> +      AT_HANDLE_MNT_ID_UNIQUE))
>   return -EINVAL;
>  
>   lookup_flags = (flag & AT_SYMLINK_FOLLOW) ? LOOKUP_FOLLOW : 0;
> @@ -109,7 +122,9 @@ SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
>   lookup_flags |= LOOKUP_EMPTY;
>   err = user_path_at(dfd, name, lookup_flags, &path);
>   if (!err) {
> - err = do_sys_name_to_handle(&path, handle, mnt_id, fh_flags);
> + err = do_sys_name_to_handle(&path, handle, mnt_id,
> +     flag & AT_HANDLE_MNT_ID_UNIQUE,
> +     fh_flags);
>   path_put(&path);
>   }
>   return err;
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 4bcf6754738d..5758104921e6 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -870,7 +870,7 @@ asmlinkage long sys_fanotify_mark(int fanotify_fd, unsigned int flags,
>  #endif
>  asmlinkage long sys_name_to_handle_at(int dfd, const char __user *name,
>         struct file_handle __user *handle,
> -       int __user *mnt_id, int flag);
> +       void __user *mnt_id, int flag);
>  asmlinkage long sys_open_by_handle_at(int mountdirfd,
>         struct file_handle __user *handle,
>         int flags);
> diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
> index 38a6d66d9e88..87e2dec79fea 100644
> --- a/include/uapi/linux/fcntl.h
> +++ b/include/uapi/linux/fcntl.h
> @@ -152,6 +152,7 @@
>  #define AT_HANDLE_FID 0x200 /* File handle is needed to compare
>      object identity and may not be
>      usable with open_by_handle_at(2). */
> +#define AT_HANDLE_MNT_ID_UNIQUE 0x001 /* Return the u64 unique mount ID. */
>  
>  #if defined(__KERNEL__)
>  #define AT_GETATTR_NOSEC 0x80000000
> diff --git a/tools/perf/trace/beauty/include/uapi/linux/fcntl.h b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
> index 38a6d66d9e88..87e2dec79fea 100644
> --- a/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
> +++ b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
> @@ -152,6 +152,7 @@
>  #define AT_HANDLE_FID 0x200 /* File handle is needed to compare
>      object identity and may not be
>      usable with open_by_handle_at(2). */
> +#define AT_HANDLE_MNT_ID_UNIQUE 0x001 /* Return the u64 unique mount ID. */
>  
>  #if defined(__KERNEL__)
>  #define AT_GETATTR_NOSEC 0x80000000
>

Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>