DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5B5212080A
MIME-Version: 1.0
References: <20181019152905.32418-1-olga.kornievskaia@gmail.com>
 <20181019152905.32418-2-olga.kornievskaia@gmail.com> <20181031165430.GA3377@fieldses.org>
In-Reply-To: <20181031165430.GA3377@fieldses.org>
From:   Olga Kornievskaia <olga.kornievskaia@gmail.com>
Date:   Wed, 31 Oct 2018 13:07:11 -0400
Message-ID: <CAN-5tyHTZV9LYnfD9Ag_pSdWA2Zs69JNvVBjHPQTMnDm2kRuEQ@mail.gmail.com>
Subject: Re: [PATCH v1 01/13] fs: Don't copy beyond the end of the file
To:     "J. Bruce Fields" <bfields@fieldses.org>
Cc:     "J. Bruce Fields" <bfields@redhat.com>,
        linux-nfs <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Wed, Oct 31, 2018 at 12:54 PM J. Bruce Fields <bfields@fieldses.org> wrote:
>
> On Fri, Oct 19, 2018 at 11:28:53AM -0400, Olga Kornievskaia wrote:
> > From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>
> I have some idea we've had some discussion about this before, but if so
> I've forgotten the conclusion.  Could we have more of a changelog?:
>
>         - isn't there a race condition, or is there something preventing
>           the file size from changing here?

No there is nothing preventing the size from changing. Just like there
is nothing that prevents the file from changing if you are doing a
traditional copy either.

>         - why are we doing this?  Does this change the behavior of
>           copy_file_range()?

We are doing this because 1. NFS spec and 2. copy_file_range semantics
mandate that too. There is a whole different discussion under the
client-side patch for this where the plan now is that VFS themselves
are interested in making sure they are indeed enforcing the check
stated by the documentation of copy_file_range call which states
"copying a range beyond the end of the file" is EINVAL. I recall you
argued for a "short" read instead of a EINVAL but unless VFS community
is convinced to change it it'll be enforced (soon).

>
> --b.
>
> >
> > Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> > ---
> >  fs/read_write.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/read_write.c b/fs/read_write.c
> > index 39b4a21..c60790f 100644
> > --- a/fs/read_write.c
> > +++ b/fs/read_write.c
> > @@ -1570,6 +1570,9 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
> >       if (unlikely(ret))
> >               return ret;
> >
> > +     if (pos_in >= i_size_read(inode_in))
> > +             return -EINVAL;
> > +
> >       if (!(file_in->f_mode & FMODE_READ) ||
> >           !(file_out->f_mode & FMODE_WRITE) ||
> >           (file_out->f_flags & O_APPEND))
> > --
> > 1.8.3.1