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

[Jeff Layton <jlayton@kernel.org>]

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@cyphar.com>
> ---
>  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@kernel.org>