diff mbox series

[v3,2/3] fs: name_to_handle_at() support for "explicit connectable" file handles

Message ID 20241008152118.453724-3-amir73il@gmail.com (mailing list archive)
State New
Headers show
Series API for exporting connectable file handles to userspace | expand

Commit Message

Amir Goldstein Oct. 8, 2024, 3:21 p.m. UTC
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@gmail.com>
---
 fs/fhandle.c               | 48 ++++++++++++++++++++++++++++++++++----
 include/linux/exportfs.h   |  2 ++
 include/uapi/linux/fcntl.h |  1 +
 3 files changed, 46 insertions(+), 5 deletions(-)

Comments

Jeff Layton Oct. 8, 2024, 6:31 p.m. UTC | #1
On Tue, 2024-10-08 at 17:21 +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@gmail.com>
> ---
>  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 c5792cf3c6e9..7b4c8945efcb 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))
> +		return -EINVAL;
> +

Is it possible to get a DCACHE_DISCONNECTED dentry here? This thing
comes from a userland path walk (a'la name_to_handle_at()). That means
that it necessarily is connected.

I'd drop the fh_flags check, since it seems like having that set on any
dentry we get in this interface would be a bug.

>  	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 &&
> +	    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 76a3050b3593..230b0e1d669d 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		0x40000

nit: why skip 0x20000 ?

>  #define FILEID_VALID_USER_FLAGS	(0)
>  
>  #define FILEID_USER_TYPE_IS_VALID(type) \
> 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
Amir Goldstein Oct. 8, 2024, 7:43 p.m. UTC | #2
On Tue, Oct 8, 2024 at 8:31 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Tue, 2024-10-08 at 17:21 +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@gmail.com>
> > ---
> >  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 c5792cf3c6e9..7b4c8945efcb 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))
> > +             return -EINVAL;
> > +
>
> Is it possible to get a DCACHE_DISCONNECTED dentry here? This thing
> comes from a userland path walk (a'la name_to_handle_at()). That means
> that it necessarily is connected.

Can't you get it with AT_EMPTY_PATH if dfd is obtained by open_by_handle()?

>
> I'd drop the fh_flags check, since it seems like having that set on any
> dentry we get in this interface would be a bug.
>

Well I don't know if giving a disconnected dentry to name_to_handle
is intentional, but it is not wrong, unless user actually requests a
connectable file handle. No?

> >       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 &&
> > +         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 76a3050b3593..230b0e1d669d 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                0x40000
>
> nit: why skip 0x20000 ?

Well in v2, the values were just shifting the EXPORT_FH_ flags left
so I kept it this way, although I am not sure if FILEID_ID_FID is ever
going to be useful, so I guess there is no good reason to skip 0x20000

Thanks,
Amir.
diff mbox series

Patch

diff --git a/fs/fhandle.c b/fs/fhandle.c
index c5792cf3c6e9..7b4c8945efcb 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))
+		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 &&
+	    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 76a3050b3593..230b0e1d669d 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		0x40000
 #define FILEID_VALID_USER_FLAGS	(0)
 
 #define FILEID_USER_TYPE_IS_VALID(type) \
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