Re: [PATCH v3 3/6] generic: copy_file_range swapfile test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 10, 2019 at 9:37 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Mon, Jun 10, 2019 at 6:58 AM Theodore Ts'o <tytso@xxxxxxx> wrote:
> >
> > On Sun, Jun 02, 2019 at 03:41:11PM +0300, Amir Goldstein wrote:
> > > This test case was split out of Dave Chinner's copy_file_range bounds
> > > check test to reduce the requirements for running the bounds check.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> >
> > I've just updated to the latest fstests, where this has landed as
> > generic/554.  This test is failing on ext4, and should fail on all
> > file systems which do not support copy_file_range (ext4, nfsv3, etc.),
> > since xfs_io will fall back to emulating this via reading and writing
>
> Why do you think this is xfs_io fall back and not kernel fall back to
> do_splice_direct()? Anyway, both cases allow read from swapfile
> on upstream.
>
> > the file, and this causes a test failure because:
> >
> > > +echo swap files return ETXTBUSY
> > > +_format_swapfile $testdir/swapfile 16m
> > > +swapon $testdir/swapfile
> > > +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/file" $testdir/swapfile
> > > +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/swapfile" $testdir/copy
> > > +swapoff $testdir/swapfile
> >
> > Currently, the VFS doesn't prevent us from reading a swap file.
> > Perhaps it shouldn't, for security (theatre) reasons, but root can
> > read the raw block device anyway, so it's really kind of pointless.
> >
>
> Hmm, my intention with the copy_file_range() behavior was that
> it mostly follows user copy limitations/semantics.
> I guess preventing copy *from* swapfile doesn't make much sense
> and we should relax this check in the new c_f_r bounds check in VFS.
>
> > I'm not sure what's the best way fix this, but I'm going to exclude
> > this test in my test appliance builds for now.
> >
>
> Trying to understand the desired flow of tests and fixes.
> I agree that generic/554 failure may be a test/interface bug that
> we should fix in a way that current upstream passes the test for
> ext4. Unless there is objection, I will send a patch to fix the test
> to only test copy *to* swapfile.

I made this change and test still failed on upstream ext4, because
kernel performs copy_file_range() to swapfile.
To me that seems like a real kernel bug, which is addressed by vfs
c_f_r patches, so I don't know if you should be excluding the test
from test appliance after all ?

Thanks,
Amir.

>
> generic/553, OTOH, is expected to fail on upstream kernel.
> Are you leaving 553 in appliance build in anticipation to upstream fix?
> I guess the answer is in the ext4 IS_IMMUTABLE patch that you
> posted and plan to push to upstream/stable sooner than VFS patches.
>
> In any case, the VFS c_f_r patches are aiming for v5.3 and
> I will make sure to promote them for stable as well.
>
> Do you think that should there be a different policy w.r.t timing of
> merging xfstests tests that fail on upstream kernel?
>
> Thanks,
> Amir.



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux