linux-kernel - Re: [PATCH v2] vfs: prevent copy_file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOQ4uxgPtqG6eTi2AnAV4jTAaNDbeez+Xi2858mz1KLGMFntfg@mail.gmail.com>
Date:   Mon, 15 Feb 2021 19:24:32 +0200
From:   Amir Goldstein <amir73il@...il.com>
To:     Trond Myklebust <trondmy@...merspace.com>
Cc:     "lhenriques@...e.de" <lhenriques@...e.de>,
        "samba-technical@...ts.samba.org" <samba-technical@...ts.samba.org>,
        "drinkcat@...omium.org" <drinkcat@...omium.org>,
        "iant@...gle.com" <iant@...gle.com>,
        "linux-cifs@...r.kernel.org" <linux-cifs@...r.kernel.org>,
        "darrick.wong@...cle.com" <darrick.wong@...cle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jlayton@...nel.org" <jlayton@...nel.org>,
        "anna.schumaker@...app.com" <anna.schumaker@...app.com>,
        "llozano@...omium.org" <llozano@...omium.org>,
        "miklos@...redi.hu" <miklos@...redi.hu>,
        "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "dchinner@...hat.com" <dchinner@...hat.com>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "sfrench@...ba.org" <sfrench@...ba.org>,
        "ceph-devel@...r.kernel.org" <ceph-devel@...r.kernel.org>
Subject: Re: [PATCH v2] vfs: prevent copy_file_range to copy across devices

On Mon, Feb 15, 2021 at 6:53 PM Trond Myklebust <trondmy@...merspace.com> wrote:
>
> On Mon, 2021-02-15 at 18:34 +0200, Amir Goldstein wrote:
> > On Mon, Feb 15, 2021 at 5:42 PM Luis Henriques <lhenriques@...e.de>
> > wrote:
> > >
> > > Nicolas Boichat reported an issue when trying to use the
> > > copy_file_range
> > > syscall on a tracefs file.  It failed silently because the file
> > > content is
> > > generated on-the-fly (reporting a size of zero) and copy_file_range
> > > needs
> > > to know in advance how much data is present.
> > >
> > > This commit restores the cross-fs restrictions that existed prior
> > > to
> > > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices")
> > > and
> > > removes generic_copy_file_range() calls from ceph, cifs, fuse, and
> > > nfs.
> > >
> > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across
> > > devices")
> > > Link:
> > > https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drinkcat@chromium.org/
> > > Cc: Nicolas Boichat <drinkcat@...omium.org>
> > > Signed-off-by: Luis Henriques <lhenriques@...e.de>
> >
> > Code looks ok.
> > You may add:
> >
> > Reviewed-by: Amir Goldstein <amir73il@...il.com>
> >
> > I agree with Trond that the first paragraph of the commit message
> > could
> > be improved.
> > The purpose of this change is to fix the change of behavior that
> > caused the regression.
> >
> > Before v5.3, behavior was -EXDEV and userspace could fallback to
> > read.
> > After v5.3, behavior is zero size copy.
> >
> > It does not matter so much what makes sense for CFR to do in this
> > case (generic cross-fs copy).  What matters is that nobody asked for
> > this change and that it caused problems.
> >
>
> No. I'm saying that this patch should be NACKed unless there is a real
> explanation for why we give crap about this tracefs corner case and why
> it can't be fixed.
>
> There are plenty of reasons why copy offload across filesystems makes
> sense, and particularly when you're doing NAS. Clone just doesn't cut
> it when it comes to disaster recovery (whereas backup to a different
> storage unit does). If the client has to do the copy, then you're
> effectively doubling the load on the server, and you're adding
> potentially unnecessary network traffic (or at the very least you are
> doubling that traffic).
>

I don't understand the use case you are describing.

Which filesystem types are you talking about for source and target
of copy_file_range()?

To be clear, the original change was done to support NFS/CIFS server-side
copy and those should not be affected by this change.

Thanks,
Amir.