On Wed, Nov 13, 2024 at 8:11 PM Erin Shepherd <erin.shepherd@xxxxxx> wrote:
>
> For pidfs, there is no reason to restrict file handle decoding by
> CAP_DAC_READ_SEARCH. Introduce an export_ops flag that can indicate
> this
>
> Signed-off-by: Erin Shepherd <erin.shepherd@xxxxxx>
> ---
> fs/fhandle.c | 36 +++++++++++++++++++++---------------
> include/linux/exportfs.h | 3 +++
> 2 files changed, 24 insertions(+), 15 deletions(-)
>
> diff --git a/fs/fhandle.c b/fs/fhandle.c
> index 82df28d45cd70a7df525f50bbb398d646110cd99..056116e58f43983bc7bb86da170fb554c7a2fac7 100644
> --- a/fs/fhandle.c
> +++ b/fs/fhandle.c
> @@ -235,26 +235,32 @@ static int do_handle_to_path(struct file_handle *handle, struct path *path,
> return 0;
> }
>
> -/*
> - * Allow relaxed permissions of file handles if the caller has the
> - * ability to mount the filesystem or create a bind-mount of the
> - * provided @mountdirfd.
> - *
> - * In both cases the caller may be able to get an unobstructed way to
> - * the encoded file handle. If the caller is only able to create a
> - * bind-mount we need to verify that there are no locked mounts on top
> - * of it that could prevent us from getting to the encoded file.
> - *
> - * In principle, locked mounts can prevent the caller from mounting the
> - * filesystem but that only applies to procfs and sysfs neither of which
> - * support decoding file handles.
> - */
> static inline bool may_decode_fh(struct handle_to_path_ctx *ctx,
> unsigned int o_flags)
> {
> struct path *root = &ctx->root;
> + struct export_operations *nop = root->mnt->mnt_sb->s_export_op;
> +
> + if (nop && nop->flags & EXPORT_OP_UNRESTRICTED_OPEN)
> + return true;
> +
> + if (capable(CAP_DAC_READ_SEARCH))
> + return true;
>
> /*
> + * Allow relaxed permissions of file handles if the caller has the
> + * ability to mount the filesystem or create a bind-mount of the
> + * provided @mountdirfd.
> + *
> + * In both cases the caller may be able to get an unobstructed way to
> + * the encoded file handle. If the caller is only able to create a
> + * bind-mount we need to verify that there are no locked mounts on top
> + * of it that could prevent us from getting to the encoded file.
> + *
> + * In principle, locked mounts can prevent the caller from mounting the
> + * filesystem but that only applies to procfs and sysfs neither of which
> + * support decoding file handles.
> + *
> * Restrict to O_DIRECTORY to provide a deterministic API that avoids a
> * confusing api in the face of disconnected non-dir dentries.
> *
> @@ -293,7 +299,7 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh,
> if (retval)
> goto out_err;
>
> - if (!capable(CAP_DAC_READ_SEARCH) && !may_decode_fh(&ctx, o_flags)) {
> + if (!may_decode_fh(&ctx, o_flags)) {
> retval = -EPERM;
> goto out_path;
> }
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 893a1d21dc1c4abc7e52325d7a4cf0adb407f039..459508b53e77ed0597cee217ffe3d82cc7cc11a4 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -247,6 +247,9 @@ struct export_operations {
> */
> #define EXPORT_OP_FLUSH_ON_CLOSE (0x20) /* fs flushes file data on close */
> #define EXPORT_OP_ASYNC_LOCK (0x40) /* fs can do async lock request */
> +#define EXPORT_OP_UNRESTRICTED_OPEN (0x80) /* FS allows open_by_handle_at
> + without CAP_DAC_READ_SEARCH
> + */
Don't love the name, but I wonder, isn't SB_NOUSER already a good
enough indication that CAP_DAC_READ_SEARCH is irrelevant?
Essentially, mnt_fd is the user's proof that they can access the mount
and CAP_DAC_READ_SEARCH is the legacy "proof" that the user can
reach from mount the inode by path lookup.
Which reminds me, what is the mnt_fd expected for opening a pidfd
file by handle?
Thanks,
Amir.
