Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <Yv1jwsHVWI+lguAT@ZenIV> <CAN-5tyFvV7QOxyAQXu3UM5swQVB2roDpQ5CBRVc64Epp1gj9hg@mail.gmail.com>
 <Yv2BVKuzZdMDY2Td@ZenIV> <CAN-5tyF0ZMX8a6M6Qbbco3EmOzwVnnGZmqak8=t4Cvtzc45g7Q@mail.gmail.com>
 <CAOQ4uxgA8jD6KnbuHDevNLsjD-LbEs_y1W6uYMEY6EG_es0o+Q@mail.gmail.com> <Yv3Ti/niVd5ZVPP+@ZenIV>
In-Reply-To: <Yv3Ti/niVd5ZVPP+@ZenIV>
From:   Olga Kornievskaia <aglo@umich.edu>
Date:   Thu, 18 Aug 2022 09:13:59 -0400
Message-ID: <CAN-5tyHpDHzmo-rSw1X+0oX0xbxR+x13eP57osB0qhFLKbXzVA@mail.gmail.com>
Subject: Re: [RFC] problems with alloc_file_pseudo() use in __nfs42_ssc_open()
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     Amir Goldstein <amir73il@gmail.com>,
        linux-nfs <linux-nfs@vger.kernel.org>,
        Olga Kornievskaia <kolga@netapp.com>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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;
+       }
+
        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;
        }