| Message ID | 20241008152118.453724-4-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | API for exporting connectable file handles to userspace | expand |
On Tue, 2024-10-08 at 17:21 +0200, Amir Goldstein wrote: > Teach open_by_handle_at(2) about the type format of "explicit connectable" > file handles that were created using the AT_HANDLE_CONNECTABLE flag to > name_to_handle_at(2). > > When decoding an "explicit connectable" file handles, name_to_handle_at(2) > should fail if it cannot open a "connected" fd with known path, which is > accessible (to capable user) from mount fd path. > > Note that this does not check if the path is accessible to the calling > user, just that it is accessible wrt the mount namesapce, so if there > is no "connected" alias, or if parts of the path are hidden in the > mount namespace, open_by_handle_at(2) will return -ESTALE. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > fs/fhandle.c | 20 +++++++++++++++++++- > include/linux/exportfs.h | 2 +- > 2 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/fs/fhandle.c b/fs/fhandle.c > index 7b4c8945efcb..6a5458c3c6c9 100644 > --- a/fs/fhandle.c > +++ b/fs/fhandle.c > @@ -246,7 +246,13 @@ static int vfs_dentry_acceptable(void *context, struct dentry *dentry) > > if (!(ctx->flags & HANDLE_CHECK_SUBTREE) || d == root) > retval = 1; > - WARN_ON_ONCE(d != root && d != root->d_sb->s_root); > + /* > + * exportfs_decode_fh_raw() does not call acceptable() callback with > + * a disconnected directory dentry, so we should have reached either > + * mount fd directory or sb root. > + */ > + if (ctx->fh_flags & EXPORT_FH_DIR_ONLY) > + WARN_ON_ONCE(d != root && d != root->d_sb->s_root); I don't quite get the test for EXPORT_FH_DIR_ONLY here. Why does this change require that instead of just continuing to WARN unconditionally? > dput(d); > return retval; > } > @@ -349,6 +355,7 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh, > retval = -EINVAL; > goto out_path; > } > + > handle = kmalloc(struct_size(handle, f_handle, f_handle.handle_bytes), > GFP_KERNEL); > if (!handle) { > @@ -364,6 +371,17 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh, > goto out_handle; > } > > + /* > + * If handle was encoded with AT_HANDLE_CONNECTABLE, verify that we > + * are decoding an fd with connected path, which is accessible from > + * the mount fd path. > + */ > + if (f_handle.handle_type & FILEID_IS_CONNECTABLE) { > + ctx.fh_flags |= EXPORT_FH_CONNECTABLE; > + ctx.flags |= HANDLE_CHECK_SUBTREE; > + } > + if (f_handle.handle_type & FILEID_IS_DIR) > + ctx.fh_flags |= EXPORT_FH_DIR_ONLY; > /* Filesystem code should not be exposed to user flags */ > handle->handle_type &= ~FILEID_USER_FLAGS_MASK; > retval = do_handle_to_path(handle, path, &ctx); > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h > index 230b0e1d669d..a48d4c94ff74 100644 > --- a/include/linux/exportfs.h > +++ b/include/linux/exportfs.h > @@ -171,7 +171,7 @@ struct fid { > /* 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_VALID_USER_FLAGS (FILEID_IS_CONNECTABLE | FILEID_IS_DIR) > > #define FILEID_USER_TYPE_IS_VALID(type) \ > (!(FILEID_USER_FLAGS(type) & ~FILEID_VALID_USER_FLAGS))
On Tue, Oct 8, 2024 at 8:37 PM Jeff Layton <jlayton@kernel.org> wrote: > > On Tue, 2024-10-08 at 17:21 +0200, Amir Goldstein wrote: > > Teach open_by_handle_at(2) about the type format of "explicit connectable" > > file handles that were created using the AT_HANDLE_CONNECTABLE flag to > > name_to_handle_at(2). > > > > When decoding an "explicit connectable" file handles, name_to_handle_at(2) > > should fail if it cannot open a "connected" fd with known path, which is > > accessible (to capable user) from mount fd path. > > > > Note that this does not check if the path is accessible to the calling > > user, just that it is accessible wrt the mount namesapce, so if there > > is no "connected" alias, or if parts of the path are hidden in the > > mount namespace, open_by_handle_at(2) will return -ESTALE. > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > --- > > fs/fhandle.c | 20 +++++++++++++++++++- > > include/linux/exportfs.h | 2 +- > > 2 files changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/fs/fhandle.c b/fs/fhandle.c > > index 7b4c8945efcb..6a5458c3c6c9 100644 > > --- a/fs/fhandle.c > > +++ b/fs/fhandle.c > > @@ -246,7 +246,13 @@ static int vfs_dentry_acceptable(void *context, struct dentry *dentry) > > > > if (!(ctx->flags & HANDLE_CHECK_SUBTREE) || d == root) > > retval = 1; > > - WARN_ON_ONCE(d != root && d != root->d_sb->s_root); > > + /* > > + * exportfs_decode_fh_raw() does not call acceptable() callback with > > + * a disconnected directory dentry, so we should have reached either > > + * mount fd directory or sb root. > > + */ > > + if (ctx->fh_flags & EXPORT_FH_DIR_ONLY) > > + WARN_ON_ONCE(d != root && d != root->d_sb->s_root); > > I don't quite get the test for EXPORT_FH_DIR_ONLY here. Why does this > change require that instead of just continuing to WARN unconditionally? > The reason is at the end of may_decode_fh(), you have: ctx->fh_flags = EXPORT_FH_DIR_ONLY; return true; So until THIS patch, vfs_dentry_acceptable() was always called with EXPORT_FH_DIR_ONLY. THIS patch adds another use case where HANDLE_CHECK_SUBTREE is being requested, but this time EXPORT_FH_DIR_ONLY is optional. The comment above "exportfs_decode_fh_raw() does not call acceptable()..." explains why the assertion is only true for directories. Thanks, Amir.
diff --git a/fs/fhandle.c b/fs/fhandle.c index 7b4c8945efcb..6a5458c3c6c9 100644 --- a/fs/fhandle.c +++ b/fs/fhandle.c @@ -246,7 +246,13 @@ static int vfs_dentry_acceptable(void *context, struct dentry *dentry) if (!(ctx->flags & HANDLE_CHECK_SUBTREE) || d == root) retval = 1; - WARN_ON_ONCE(d != root && d != root->d_sb->s_root); + /* + * exportfs_decode_fh_raw() does not call acceptable() callback with + * a disconnected directory dentry, so we should have reached either + * mount fd directory or sb root. + */ + if (ctx->fh_flags & EXPORT_FH_DIR_ONLY) + WARN_ON_ONCE(d != root && d != root->d_sb->s_root); dput(d); return retval; } @@ -349,6 +355,7 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh, retval = -EINVAL; goto out_path; } + handle = kmalloc(struct_size(handle, f_handle, f_handle.handle_bytes), GFP_KERNEL); if (!handle) { @@ -364,6 +371,17 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh, goto out_handle; } + /* + * If handle was encoded with AT_HANDLE_CONNECTABLE, verify that we + * are decoding an fd with connected path, which is accessible from + * the mount fd path. + */ + if (f_handle.handle_type & FILEID_IS_CONNECTABLE) { + ctx.fh_flags |= EXPORT_FH_CONNECTABLE; + ctx.flags |= HANDLE_CHECK_SUBTREE; + } + if (f_handle.handle_type & FILEID_IS_DIR) + ctx.fh_flags |= EXPORT_FH_DIR_ONLY; /* Filesystem code should not be exposed to user flags */ handle->handle_type &= ~FILEID_USER_FLAGS_MASK; retval = do_handle_to_path(handle, path, &ctx); diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h index 230b0e1d669d..a48d4c94ff74 100644 --- a/include/linux/exportfs.h +++ b/include/linux/exportfs.h @@ -171,7 +171,7 @@ struct fid { /* 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_VALID_USER_FLAGS (FILEID_IS_CONNECTABLE | FILEID_IS_DIR) #define FILEID_USER_TYPE_IS_VALID(type) \ (!(FILEID_USER_FLAGS(type) & ~FILEID_VALID_USER_FLAGS))
Teach open_by_handle_at(2) about the type format of "explicit connectable" file handles that were created using the AT_HANDLE_CONNECTABLE flag to name_to_handle_at(2). When decoding an "explicit connectable" file handles, name_to_handle_at(2) should fail if it cannot open a "connected" fd with known path, which is accessible (to capable user) from mount fd path. Note that this does not check if the path is accessible to the calling user, just that it is accessible wrt the mount namesapce, so if there is no "connected" alias, or if parts of the path are hidden in the mount namespace, open_by_handle_at(2) will return -ESTALE. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- fs/fhandle.c | 20 +++++++++++++++++++- include/linux/exportfs.h | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-)