[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240902125419.qe34oqqkizumocta@quack3>
Date: Mon, 2 Sep 2024 14:54:19 +0200
From: Jan Kara <jack@...e.cz>
To: Aleksa Sarai <cyphar@...har.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
Chuck Lever <chuck.lever@...cle.com>,
Jeff Layton <jlayton@...nel.org>,
Amir Goldstein <amir73il@...il.com>,
Alexander Aring <alex.aring@...il.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Christoph Hellwig <hch@...radead.org>,
Josef Bacik <josef@...icpanda.com>, linux-fsdevel@...r.kernel.org,
linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-api@...r.kernel.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH RESEND v3 2/2] fhandle: expose u64 mount id to
name_to_handle_at(2)
On Wed 28-08-24 20:19:43, 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);
>
> Reviewed-by: Jeff Layton <jlayton@...nel.org>
> Signed-off-by: Aleksa Sarai <cyphar@...har.com>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
> ---
> 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
>
> --
> 2.46.0
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists