On Mon, Feb 10, 2025 at 02:26:56PM +0100, Amir Goldstein wrote:
> On Mon, Feb 10, 2025 at 1:39 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> >
> > Let FSCONFIG_SET_FD handle O_PATH file descriptors. This is particularly
> > useful in the context of overlayfs where layers can be specified via
> > file descriptors instead of paths. But userspace must currently use
> > non-O_PATH file desriptors which is often pointless especially if
> > the file descriptors have been created via open_tree(OPEN_TREE_CLONE).
> >
> > Fixes: a08557d19ef41 ("ovl: specify layers via file descriptors")
> > Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
> > ---
> > fs/autofs/autofs_i.h | 2 ++
> > fs/fsopen.c | 2 +-
> > 2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
> > index 77c7991d89aa..23cea74f9933 100644
> > --- a/fs/autofs/autofs_i.h
> > +++ b/fs/autofs/autofs_i.h
> > @@ -218,6 +218,8 @@ void autofs_clean_ino(struct autofs_info *);
> >
> > static inline int autofs_check_pipe(struct file *pipe)
> > {
> > + if (pipe->f_mode & FMODE_PATH)
> > + return -EINVAL;
> > if (!(pipe->f_mode & FMODE_CAN_WRITE))
> > return -EINVAL;
>
> I thought you said the above check is redundant due to the lower check.
It is but that's only obvious to people quite familiar with VFS code.
So I like the explicitly check here.
>
> In any case feel free to add
>
> Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
>
> > if (!S_ISFIFO(file_inode(pipe)->i_mode))
> > diff --git a/fs/fsopen.c b/fs/fsopen.c
> > index 094a7f510edf..1aaf4cb2afb2 100644
> > --- a/fs/fsopen.c
> > +++ b/fs/fsopen.c
> > @@ -453,7 +453,7 @@ SYSCALL_DEFINE5(fsconfig,
> > case FSCONFIG_SET_FD:
> > param.type = fs_value_is_file;
> > ret = -EBADF;
> > - param.file = fget(aux);
> > + param.file = fget_raw(aux);
> > if (!param.file)
> > goto out_key;
> > param.dirfd = aux;
> >
> > --
> > 2.47.2
> >
