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>
In-Reply-To: <CAN-5tyF0ZMX8a6M6Qbbco3EmOzwVnnGZmqak8=t4Cvtzc45g7Q@mail.gmail.com>
From:   Amir Goldstein <amir73il@gmail.com>
Date:   Thu, 18 Aug 2022 08:19:54 +0300
Message-ID: <CAOQ4uxgA8jD6KnbuHDevNLsjD-LbEs_y1W6uYMEY6EG_es0o+Q@mail.gmail.com>
Subject: Re: [RFC] problems with alloc_file_pseudo() use in __nfs42_ssc_open()
To:     Olga Kornievskaia <aglo@umich.edu>
Cc:     Al Viro <viro@zeniv.linux.org.uk>,
        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 3:23 AM Olga Kornievskaia <aglo@umich.edu> wrote:
>
> On Wed, Aug 17, 2022 at 8:01 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Wed, Aug 17, 2022 at 06:32:15PM -0400, Olga Kornievskaia wrote:
> > > On Wed, Aug 17, 2022 at 6:18 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> > > >
> > > >         My apologies for having missed that back when the SSC
> > > > patchset had been done (and missing the problems after it got
> > > > merged, actually).
> > > >
> > > > 1) if this
> > > >         r_ino = nfs_fhget(ss_mnt->mnt_sb, src_fh, fattr);
> > > > in __nfs42_ssc_open() yields a directory inode, we are screwed
> > > > as soon as it's passed to alloc_file_pseudo() - a *lot* of places
> > > > in dcache handling would break if we do that.  It's not too
> > > > nice for a regular file from non-cooperating filesystem, but for
> > > > directory ones it's deadly.
> > >
> > > This inode is created to make an appearance of an opened file to do
> > > (an NFS) read, it's never a directory.
> >
> > Er...  Where does the fhandle come from?  From my reading it's a client-sent
> > data; I don't know what trust model do you assume, but the price of
> > getting multiple dentries over the same directory inode is high.
> > Bogus or compromised client should not be able to cause severe corruption
> > of kernel data structures...
>
> This is an NFS spec specified operation. The (source file's)
> filehandle comes from the COPY operation compound that the destination
> server gets and then uses -- creates an inode from using the code you
> are looking at now -- to access from the source server. Security is
> all described in the spec. The uniqueness of the filehandle is
> provided by the source server that created it.

Olga,

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().

Thanks,
Amir.