Re: [RFC] problems with alloc_file_pseudo() use in __nfs42_ssc_open()

Trond Myklebust
2022-08-18T14:38:23Z

On Thu, 2022-08-18 at 09:13 -0400, Olga Kornievskaia wrote:
> On Thu, Aug 18, 2022 at 1:52 AM Al Viro <viro@zeniv.linux.org.uk>
> wrote:
> >
> > On Thu, Aug 18, 2022 at 08:19:54AM +0300, Amir Goldstein wrote:
> >
> > > NFS spec does not guarantee the safety of the server.
> > > It's like saying that the Law makes Crime impossible.
> > > The law needs to be enforced, so if server gets a request
> > > to COPY from/to an fhandle that resolves as a non-regular file
> > > (from a rogue or buggy NFS client) the server should return an
> > > error and not continue to alloc_file_pseudo().
> >
> > FWIW, my preference would be to have alloc_file_pseudo() reject
> > directory inodes if it ever gets such.
> >
> > I'm still not sure that my (and yours, apparently) interpretation
> > of what Olga said is correct, though.
>
> Would it be appropriate to do the following then:
>
> diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
> index e88f6b18445e..112134b6438d 100644
> --- a/fs/nfs/nfs4file.c
> +++ b/fs/nfs/nfs4file.c
> @@ -340,6 +340,11 @@ static struct file *__nfs42_ssc_open(struct
> vfsmount *ss_mnt,
>                 goto out;
>         }
>
> +       if (S_ISDIR(fattr->mode)) {
> +               res = ERR_PTR(-EBADF);
> +               goto out;
> +       }

You're better off going with 'if (!S_ISREG())' so that we also reject
symlinks, pipes, devices, etc. The requirement for the NFSv4.2 copy
offload protocol is that both the source and destination be regular
files.

> +
>         res = ERR_PTR(-ENOMEM);
>         len = strlen(SSC_READ_NAME_BODY) + 16;
>         read_name = kzalloc(len, GFP_KERNEL);
> @@ -357,6 +362,7 @@ static struct file *__nfs42_ssc_open(struct
> vfsmount *ss_mnt,
>                                      r_ino->i_fop);
>         if (IS_ERR(filep)) {
>                 res = ERR_CAST(filep);
> +               iput(r_ino);
>                 goto out_free_name;
>         }

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com