DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CE60720811
Date:   Wed, 12 Dec 2018 11:42:58 -0800
From:   Matthew Wilcox <willy@infradead.org>
To:     Olga Kornievskaia <olga.kornievskaia@gmail.com>
Cc:     lhenriques@suse.com, david@fromorbit.com,
        "Darrick J. Wong" <darrick.wong@oracle.com>,
        linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org,
        linux-nfs <linux-nfs@vger.kernel.org>,
        linux-unionfs@vger.kernel.org, ceph-devel@vger.kernel.org,
        linux-cifs@vger.kernel.org
Subject: Re: [PATCH 04/11] vfs: add missing checks to copy_file_range
Message-ID: <20181212194258.GK6830@bombadil.infradead.org>
References: <20181203083416.28978-1-david@fromorbit.com>
 <20181203083416.28978-5-david@fromorbit.com>
 <87a7lbrng4.fsf@suse.com>
 <CAN-5tyHNGtLxeFyEO6qX_+8=K4tpkgYjB11OkfCTdWfokfAyEg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAN-5tyHNGtLxeFyEO6qX_+8=K4tpkgYjB11OkfCTdWfokfAyEg@mail.gmail.com>
User-Agent: Mutt/1.9.2 (2017-12-15)
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Wed, Dec 12, 2018 at 01:55:28PM -0500, Olga Kornievskaia wrote:
> On Wed, Dec 12, 2018 at 6:31 AM Luis Henriques <lhenriques@suse.com> wrote:
> > I was wondering if, with the above check, it would make sense to also
> > have an extra patch changing some filesystems (ceph, nfs and cifs) to
> > simply return -EOPNOTSUPP (instead of -EINVAL) when inode_in ==
> > inode_out.  Something like the diff below (not tested!).

> > +++ b/fs/nfs/nfs4file.c
> > @@ -136,7 +136,7 @@ static ssize_t nfs4_copy_file_range(struct file *file_in, loff_t pos_in,
> >         ssize_t ret;
> >
> >         if (file_inode(file_in) == file_inode(file_out))
> > -               return -EINVAL;
> > +               return -EOPNOTSUPP;
> 
> Please don't change the NFS bits. This is against the NFS
> specifications. RFC 7862 15.2.3
> 
> (snippet)
> SAVED_FH and CURRENT_FH must be different files.  If SAVED_FH and
>    CURRENT_FH refer to the same file, the operation MUST fail with
>    NFS4ERR_INVAL.

I don't see how that applies.  That refers to a requirement _in the
protocol_ that determines what the server MUST do if the client sends
it two FHs which refer to the same file.

What we're talking about here is how a Linux filesystem behaves when
receiving a copy_file_range() referring to the same file.  As long as
the Linux filesystem doesn't react by sending out one of these invalid
protocol messages, I don't see the problem.