On Fri, 2024-10-11 at 11:00 +0200, Amir Goldstein wrote:
> nfsd encodes "connectable" file handles for the subtree_check feature,
> which can be resolved to an open file with a connected path.
> So far, userspace nfs server could not make use of this functionality.
>
> Introduce a new flag AT_HANDLE_CONNECTABLE to name_to_handle_at(2).
> When used, the encoded file handle is "explicitly connectable".
>
> The "explicitly connectable" file handle sets bits in the high 16bit of
> the handle_type field, so open_by_handle_at(2) will know that it needs
> to open a file with a connected path.
>
> old kernels will now recognize the handle_type with high bits set,
> so "explicitly connectable" file handles cannot be decoded by
> open_by_handle_at(2) on old kernels.
>
> The flag AT_HANDLE_CONNECTABLE is not allowed together with either
> AT_HANDLE_FID or AT_EMPTY_PATH.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/fhandle.c | 48 ++++++++++++++++++++++++++++++++++----
> include/linux/exportfs.h | 2 ++
> include/uapi/linux/fcntl.h | 1 +
> 3 files changed, 46 insertions(+), 5 deletions(-)
>
> diff --git a/fs/fhandle.c b/fs/fhandle.c
> index 218511f38cbb..8339a1041025 100644
> --- a/fs/fhandle.c
> +++ b/fs/fhandle.c
> @@ -31,6 +31,14 @@ static long do_sys_name_to_handle(const struct path *path,
> if (!exportfs_can_encode_fh(path->dentry->d_sb->s_export_op, fh_flags))
> return -EOPNOTSUPP;
>
> + /*
> + * A request to encode a connectable handle for a disconnected dentry
> + * is unexpected since AT_EMPTY_PATH is not allowed.
> + */
> + if (fh_flags & EXPORT_FH_CONNECTABLE &&
> + WARN_ON(path->dentry->d_flags & DCACHE_DISCONNECTED))
Is this even possible? The dentry in this case will have been reached
by pathwalk. Oh, but I guess the dfd could point to a disconnected
dentry and then you pass in AT_EMPTY_PATH.
I'm not sure we want to warn in that case though, since this is a
situation that an unprivileged user could be able to arrange. Maybe we
should just return a more distinct error code in this case?
Since the scenario involves a dfd that is disconnected, how about:
#define EBADFD 77 /* File descriptor in bad state */
> + return -EINVAL;
> +
> if (copy_from_user(&f_handle, ufh, sizeof(struct file_handle)))
> return -EFAULT;
>
> @@ -45,7 +53,7 @@ static long do_sys_name_to_handle(const struct path *path,
> /* convert handle size to multiple of sizeof(u32) */
> handle_dwords = f_handle.handle_bytes >> 2;
>
> - /* we ask for a non connectable maybe decodeable file handle */
> + /* Encode a possibly decodeable/connectable file handle */
> retval = exportfs_encode_fh(path->dentry,
> (struct fid *)handle->f_handle,
> &handle_dwords, fh_flags);
> @@ -67,8 +75,23 @@ static long do_sys_name_to_handle(const struct path *path,
> * non variable part of the file_handle
> */
> handle_bytes = 0;
> - } else
> + } else {
> + /*
> + * When asked to encode a connectable file handle, encode this
> + * property in the file handle itself, so that we later know
> + * how to decode it.
> + * For sanity, also encode in the file handle if the encoded
> + * object is a directory and verify this during decode, because
> + * decoding directory file handles is quite different than
> + * decoding connectable non-directory file handles.
> + */
> + if (fh_flags & EXPORT_FH_CONNECTABLE) {
> + handle->handle_type |= FILEID_IS_CONNECTABLE;
> + if (d_is_dir(path->dentry))
> + fh_flags |= FILEID_IS_DIR;
> + }
> retval = 0;
> + }
> /* copy the mount id */
> if (unique_mntid) {
> if (put_user(real_mount(path->mnt)->mnt_id_unique,
> @@ -109,15 +132,30 @@ SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
> {
> struct path path;
> int lookup_flags;
> - int fh_flags;
> + int fh_flags = 0;
> int err;
>
> if (flag & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH | AT_HANDLE_FID |
> - AT_HANDLE_MNT_ID_UNIQUE))
> + AT_HANDLE_MNT_ID_UNIQUE | AT_HANDLE_CONNECTABLE))
> + return -EINVAL;
> +
> + /*
> + * AT_HANDLE_FID means there is no intention to decode file handle
> + * AT_HANDLE_CONNECTABLE means there is an intention to decode a
> + * connected fd (with known path), so these flags are conflicting.
> + * AT_EMPTY_PATH could be used along with a dfd that refers to a
> + * disconnected non-directory, which cannot be used to encode a
> + * connectable file handle, because its parent is unknown.
> + */
> + if (flag & AT_HANDLE_CONNECTABLE &&
nit: might need parenthesis around the above & check.
> + flag & (AT_HANDLE_FID | AT_EMPTY_PATH))
> return -EINVAL;
> + else if (flag & AT_HANDLE_FID)
> + fh_flags |= EXPORT_FH_FID;
> + else if (flag & AT_HANDLE_CONNECTABLE)
> + fh_flags |= EXPORT_FH_CONNECTABLE;
>
> lookup_flags = (flag & AT_SYMLINK_FOLLOW) ? LOOKUP_FOLLOW : 0;
> - fh_flags = (flag & AT_HANDLE_FID) ? EXPORT_FH_FID : 0;
> if (flag & AT_EMPTY_PATH)
> lookup_flags |= LOOKUP_EMPTY;
> err = user_path_at(dfd, name, lookup_flags, &path);
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 5e14d4500a75..4ee42b2cf4ab 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -169,6 +169,8 @@ struct fid {
> #define FILEID_USER_FLAGS(type) ((type) & FILEID_USER_FLAGS_MASK)
>
> /* Flags supported in encoded handle_type that is exported to user */
> +#define FILEID_IS_CONNECTABLE 0x10000
> +#define FILEID_IS_DIR 0x20000
> #define FILEID_VALID_USER_FLAGS (0)
>
> /**
> diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
> index 87e2dec79fea..56ff2100e021 100644
> --- a/include/uapi/linux/fcntl.h
> +++ b/include/uapi/linux/fcntl.h
> @@ -153,6 +153,7 @@
> 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. */
> +#define AT_HANDLE_CONNECTABLE 0x002 /* Request a connectable file handle */
>
> #if defined(__KERNEL__)
> #define AT_GETATTR_NOSEC 0x80000000
--
Jeff Layton <jlayton@xxxxxxxxxx>
