fstests: copy_file_range() bounds testing
cover fstests: copy_file_range() bounds testing
Hi folks,
We suck at bounds testing new system calls. This is a test that exercises the expected failure cases for copy_file_range(). It's going to fail miserably on existing kernels - I'm about to post a series of fixes to linux-fsdevel that make this test pass.
The test is also dependent on xfs_io changes that I posted a few hours ago. The three (not 2!) patches can be found here:
https://marc.info/?l=linux-xfs&m=154378403323889&w=2
https://marc.info/?l=linux-xfs&m=154378403523890&w=2
https://marc.info/?l=linux-xfs&m=154378403323888&w=2
https://marc.info/?l=linux-xfs&m=154379644526132&w=2
Comments welcome.
-Dave
---
As an aside, one thing that I've discovered in writing this test is that certain things we do to test certain file conditions are not being tested corectly. e.g. immutable files.
When we set a file as immutable and then go to modify it with xfs_io like this:
# xfs_io -c "chattr +i" test_file # xfs_io -c "pwrite 0 4k" test_file
We are not actually testing whether the pwrite() syscall detected that it can't write to an immutable file. xfs_io actually fails when the open(O_RDWR) syscall fails because we can't open an immutable file for writing. IOWs, it's not testing pwrite() at all.
Hence tests like generic/159 and generic/160 are not actually testing whether cloning/deduping files fails on immutable files.
Instead, what we need to do is open the file O_RDWR, then set the file immutable, then perform the modification operation. i.e. this:
# xfs_io -c "chattr +i" -c "pwrite 0 4k" test_file
Will exercise the pwrite() syscall hitting an immutable file. A similar thing happens with trying to write/modify to read only files
- the open() call fails, not the call that we want to test. That's why I added the "chmod" operation to xfs_io, to allow us to open a file for write, then turn it read only while we still have a writeable fd open. This then exercises trying to write/modify a read-only file.
I'm sure there's lots of tests that have these problems. I don't have time to audit them right now, but I'm bringing it up so that people are aware of the issue and at least catch problems like this in new tests....
PATCH 1/3 common: add _require_test_swapfile
From: Dave Chinner <[email protected]>
Because we can host swap files on the test device, not just the scratch device.
Also, move the tests for the utilities needed to manipulate swap files into the functions that test whether swap files are supported so they are checked for existence /before/ we try to us them. This fixes all the tests that currently check for these utilities manually /after/ checking if swapfiles are supported.
--- common/rc | 29 +++++++++++++++++++++-------- tests/generic/472 | 2 -- tests/generic/495 | 2 -- tests/generic/496 | 2 -- tests/generic/497 | 2 -- 5 files changed, 21 insertions(+), 16 deletions(-)
common/rc +21 -8
@@ -2214,22 +2214,35 @@ _format_swapfile() { } # Check that the filesystem supports swapfiles -_require_scratch_swapfile() +_require_swapfile() { - _require_scratch + dir=$1 - _scratch_mkfs >/dev/null - _scratch_mount + # fstests also has custom binaries for mkswap/swapon + _require_test_program mkswap + _require_test_program swapon # Minimum size for mkswap is 10 pages - _format_swapfile "$SCRATCH_MNT/swap" $(($(get_page_size) * 10)) + _format_swapfile "$dir/swap" $(($(get_page_size) * 10)) - if ! swapon "$SCRATCH_MNT/swap" >/dev/null 2>&1; then - _scratch_unmount + if ! swapon "$dir/swap" >/dev/null 2>&1; then _notrun "swapfiles are not supported" fi - swapoff "$SCRATCH_MNT/swap" >/dev/null 2>&1 + swapoff "$dir/swap" >/dev/null 2>&1 +} + +_require_test_swapfile() +{ + _require_swapfile $TEST_DIR +} + +_require_scratch_swapfile() +{ + _require_scratch + _scratch_mkfs >/dev/null + _scratch_mount + _require_swapfile $SCRATCH_MNT _scratch_unmount }
tests/generic/472 +0 -2
@@ -33,8 +33,6 @@ rm -f $seqres.full _supported_fs generic _supported_os Linux _require_scratch_swapfile -_require_test_program mkswap -_require_test_program swapon rm -f $seqres.full _scratch_mkfs >>$seqres.full 2>&1
tests/generic/495 +0 -2
@@ -31,8 +31,6 @@ rm -f $seqres.full _supported_fs generic _supported_os Linux _require_scratch_swapfile -_require_test_program mkswap -_require_test_program swapon _scratch_mkfs >> $seqres.full 2>&1 _scratch_mount
tests/generic/496 +0 -2
@@ -34,8 +34,6 @@ rm -f $seqres.full _supported_fs generic _supported_os Linux _require_scratch_swapfile -_require_test_program mkswap -_require_test_program swapon _require_xfs_io_command "falloc" rm -f $seqres.full
tests/generic/497 +0 -2
@@ -34,8 +34,6 @@ rm -f $seqres.full _supported_fs generic _supported_os Linux _require_scratch_swapfile -_require_test_program mkswap -_require_test_program swapon _require_xfs_io_command "fcollapse" rm -f $seqres.full
Signature · 1 line
2.19.1
Re: [PATCH 1/3] common: add _require_test_swapfile
Quoted 5 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Because we can host swap files on the test device, not just the scratch device.
Quoted 7 lines
Also, move the tests for the utilities needed to manipulate swap files into the functions that test whether swap files are supported so they are checked for existence /before/ we try to us them. This fixes all the tests that currently check for these utilities manually /after/ checking if swapfiles are supported.
Signed-off-by Dave Chinner <[email protected]>
Looks good,
--D
Quoted 111 lines
--- common/rc | 29 +++++++++++++++++++++-------- tests/generic/472 | 2 -- tests/generic/495 | 2 -- tests/generic/496 | 2 -- tests/generic/497 | 2 -- 5 files changed, 21 insertions(+), 16 deletions(-)common/rc +21
-8@@ -2214,22 +2214,35 @@ _format_swapfile() { } # Check that the filesystem supports swapfiles -_require_scratch_swapfile() +_require_swapfile() { - _require_scratch + dir=$1 - _scratch_mkfs >/dev/null - _scratch_mount + # fstests also has custom binaries for mkswap/swapon + _require_test_program mkswap + _require_test_program swapon # Minimum size for mkswap is 10 pages - _format_swapfile "$SCRATCH_MNT/swap" $(($(get_page_size) * 10)) + _format_swapfile "$dir/swap" $(($(get_page_size) * 10)) - if ! swapon "$SCRATCH_MNT/swap" >/dev/null 2>&1; then - _scratch_unmount + if ! swapon "$dir/swap" >/dev/null 2>&1; then _notrun "swapfiles are not supported" fi - swapoff "$SCRATCH_MNT/swap" >/dev/null 2>&1 + swapoff "$dir/swap" >/dev/null 2>&1 +} + +_require_test_swapfile() +{ + _require_swapfile $TEST_DIR +} + +_require_scratch_swapfile() +{ + _require_scratch + _scratch_mkfs >/dev/null + _scratch_mount + _require_swapfile $SCRATCH_MNT _scratch_unmount }tests/generic/472 +0
-2@@ -33,8 +33,6 @@ rm -f $seqres.full _supported_fs generic _supported_os Linux _require_scratch_swapfile -_require_test_program mkswap -_require_test_program swapon rm -f $seqres.full _scratch_mkfs >>$seqres.full 2>&1tests/generic/495 +0
-2@@ -31,8 +31,6 @@ rm -f $seqres.full _supported_fs generic _supported_os Linux _require_scratch_swapfile -_require_test_program mkswap -_require_test_program swapon _scratch_mkfs >> $seqres.full 2>&1 _scratch_mounttests/generic/496 +0
-2@@ -34,8 +34,6 @@ rm -f $seqres.full _supported_fs generic _supported_os Linux _require_scratch_swapfile -_require_test_program mkswap -_require_test_program swapon _require_xfs_io_command "falloc" rm -f $seqres.fulltests/generic/497 +0
-2@@ -34,8 +34,6 @@ rm -f $seqres.full _supported_fs generic _supported_os Linux _require_scratch_swapfile -_require_test_program mkswap -_require_test_program swapon _require_xfs_io_command "fcollapse" rm -f $seqres.fullSignature · 1 line
2.19.1
Re: [PATCH 1/3] common: add _require_test_swapfile
In reply to Dave ChinnerQuoted 29 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Because we can host swap files on the test device, not just the scratch device.
Also, move the tests for the utilities needed to manipulate swap files into the functions that test whether swap files are supported so they are checked for existence /before/ we try to us them. This fixes all the tests that currently check for these utilities manually /after/ checking if swapfiles are supported.
Signed-off-by Dave Chinner <[email protected]>--- common/rc | 29 +++++++++++++++++++++-------- tests/generic/472 | 2 -- tests/generic/495 | 2 -- tests/generic/496 | 2 -- tests/generic/497 | 2 -- 5 files changed, 21 insertions(+), 16 deletions(-)common/rc +0
-1@@ -2214,22 +2214,35 @@ _format_swapfile() { } # Check that the filesystem supports swapfiles -_require_scratch_swapfile()
common/rc _format_swapfile() { · line 2223
Quoted 10 lines
common/rc +5
-3+_require_swapfile() { - _require_scratch + dir=$1 - _scratch_mkfs >/dev/null - _scratch_mount + # fstests also has custom binaries for mkswap/swapon + _require_test_program mkswap + _require_test_program swapon
Hi Dave,
Is it necessary to include the check for custom mkswap/swapon in _require_swapfile()? In fact, some tests calling _require_scratch_swapfile() just use default mkswap/swapon command(e.g. generic/356, generic/357, generic/493, generic/494). I perfer to keep it in separate tests.
Other than that, it looks good to me.
Best Regards,
Xiao Yang
Quoted 81 lines
common/rc +16
-4# Minimum size for mkswap is 10 pages - _format_swapfile "$SCRATCH_MNT/swap" $(($(get_page_size) * 10)) + _format_swapfile "$dir/swap" $(($(get_page_size) * 10)) - if ! swapon "$SCRATCH_MNT/swap" >/dev/null 2>&1; then - _scratch_unmount + if ! swapon "$dir/swap" >/dev/null 2>&1; then _notrun "swapfiles are not supported" fi - swapoff "$SCRATCH_MNT/swap" >/dev/null 2>&1 + swapoff "$dir/swap" >/dev/null 2>&1 +} + +_require_test_swapfile() +{ + _require_swapfile $TEST_DIR +} + +_require_scratch_swapfile() +{ + _require_scratch + _scratch_mkfs >/dev/null + _scratch_mount + _require_swapfile $SCRATCH_MNT _scratch_unmount }tests/generic/472 +0
-2@@ -33,8 +33,6 @@ rm -f $seqres.full _supported_fs generic _supported_os Linux _require_scratch_swapfile -_require_test_program mkswap -_require_test_program swapon rm -f $seqres.full _scratch_mkfs >>$seqres.full 2>&1tests/generic/495 +0
-2@@ -31,8 +31,6 @@ rm -f $seqres.full _supported_fs generic _supported_os Linux _require_scratch_swapfile -_require_test_program mkswap -_require_test_program swapon _scratch_mkfs >> $seqres.full 2>&1 _scratch_mounttests/generic/496 +0
-2@@ -34,8 +34,6 @@ rm -f $seqres.full _supported_fs generic _supported_os Linux _require_scratch_swapfile -_require_test_program mkswap -_require_test_program swapon _require_xfs_io_command "falloc" rm -f $seqres.fulltests/generic/497 +0
-2@@ -34,8 +34,6 @@ rm -f $seqres.full _supported_fs generic _supported_os Linux _require_scratch_swapfile -_require_test_program mkswap -_require_test_program swapon _require_xfs_io_command "fcollapse" rm -f $seqres.full
Re: [PATCH 1/3] common: add _require_test_swapfile
Quoted 43 lines · Xiao Yang
Quoted 29 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Because we can host swap files on the test device, not just the scratch device.
Also, move the tests for the utilities needed to manipulate swap files into the functions that test whether swap files are supported so they are checked for existence /before/ we try to us them. This fixes all the tests that currently check for these utilities manually /after/ checking if swapfiles are supported.
Signed-off-by Dave Chinner <[email protected]>--- common/rc | 29 +++++++++++++++++++++-------- tests/generic/472 | 2 -- tests/generic/495 | 2 -- tests/generic/496 | 2 -- tests/generic/497 | 2 -- 5 files changed, 21 insertions(+), 16 deletions(-)common/rc +0
-1@@ -2214,22 +2214,35 @@ _format_swapfile() { } # Check that the filesystem supports swapfiles -_require_scratch_swapfile()common/rc _format_swapfile() { · line 2223
Quoted 10 lines
common/rc +5
-3+_require_swapfile() { - _require_scratch + dir=$1 - _scratch_mkfs >/dev/null - _scratch_mount + # fstests also has custom binaries for mkswap/swapon + _require_test_program mkswap + _require_test_program swaponHi Dave,
Quoted 4 lines
Is it necessary to include the check for custom mkswap/swapon in
_require_swapfile()? In fact, some tests calling_require_scratch_swapfile()just use default mkswap/swapon command(e.g. generic/356, generic/357, generic/493, generic/494). I perfer to keep it in separate tests.
Those binaries are built by fstests, so there is absolutely no harm in requiring them. If they didn't get built, then the fstests install is in no shape to be run because it wasn't built correctly.
And by adding them here, we do not need to clutter up tests with lots different requires - we just say "we require swapfile support" and that pulls in everything swapfile related. This makes the tests easier to review and maintain because there is less that needs to be done to write a swapfile related test.
-Dave.
Signature · 2 lines
Dave Chinner [email protected]
PATCH 2/3 generic/43[014]: copy_range beyond source EOF should fail
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
As per the copy_file_range() man page:
EINVAL
Requested range extends beyond the end of the source file;
.....
These tests actually attempt to copy beyond the end of the source file and so should fail with EINVAL. The kernel does not check this and so needs fixing. These operations should fail, so fix the tests.
Also move the tests to the copy_range group so they are distinct from the copy group which refers to xfs_copy tests.
--- tests/generic/430 | 5 +---- tests/generic/430.out | 6 ++---- tests/generic/431 | 1 + tests/generic/431.out | 1 + tests/generic/434 | 5 +++-- tests/generic/434.out | 4 ++-- tests/generic/group | 10 +++++----- 7 files changed, 15 insertions(+), 17 deletions(-)
tests/generic/430 +1 -4
@@ -70,11 +70,8 @@ cmp -n 1000 $testdir/file $testdir/end 4000 echo "md5sums after copying end:" md5sum $testdir/{file,end} | _filter_test_dir -echo "Copy beyond end of original file" +echo "Copy beyond end of original file - should fail" $XFS_IO_PROG -f -c "copy_range -s 4000 -l 2000 $testdir/file" "$testdir/beyond" -cmp -n 1000 $testdir/file $testdir/beyond 4000 -echo "md5sums after copying beyond:" -md5sum $testdir/{file,beyond} | _filter_test_dir echo "Copy creates hole in target file" $XFS_IO_PROG -f -c "copy_range -s 1000 -l 3000 -d 1000 $testdir/file" "$testdir/hole"
tests/generic/430.out +2 -4
@@ -15,10 +15,8 @@ Copy end of original file md5sums after copying end: e11fbace556cba26bf0076e74cab90a3 TEST_DIR/test-430/file e68d4a150c4e42f4f9ea3ffe4c9cf4ed TEST_DIR/test-430/end -Copy beyond end of original file -md5sums after copying beyond: -e11fbace556cba26bf0076e74cab90a3 TEST_DIR/test-430/file -e68d4a150c4e42f4f9ea3ffe4c9cf4ed TEST_DIR/test-430/beyond +Copy beyond end of original file - should fail +copy_range: Invalid argument Copy creates hole in target file md5sums after creating hole: e11fbace556cba26bf0076e74cab90a3 TEST_DIR/test-430/file
tests/generic/431 +1 -0
@@ -52,6 +52,7 @@ $XFS_IO_PROG -f -c "copy_range -s 2 -l 1 $testdir/file" "$testdir/c" $XFS_IO_PROG -f -c "copy_range -s 3 -l 1 $testdir/file" "$testdir/d" $XFS_IO_PROG -f -c "copy_range -s 4 -l 1 $testdir/file" "$testdir/e" $XFS_IO_PROG -f -c "copy_range -s 4 -l 1 -d 1 $testdir/file" "$testdir/f" +# this should fail with EINVAL and leave an empty file behind. $XFS_IO_PROG -f -c "copy_range -s 5 -l 1 $testdir/file" "$testdir/g" echo -n "a" | cmp $testdir/a echo -n "b" | cmp $testdir/b
tests/generic/431.out +1 -0
@@ -4,6 +4,7 @@ Original md5sums: ab56b4d92b40713acc5af89985d4b786 TEST_DIR/test-431/file ab56b4d92b40713acc5af89985d4b786 TEST_DIR/test-431/copy Small copies from various points in the original file +copy_range: Invalid argument md5sums after small copies ab56b4d92b40713acc5af89985d4b786 TEST_DIR/test-431/file 0cc175b9c0f1b6a831c399e269772661 TEST_DIR/test-431/a
tests/generic/434 +3 -2
@@ -41,15 +41,16 @@ $XFS_IO_PROG -f -c 'pwrite -S 0x61 0 1000' $testdir/file >> $seqres.full 2>&1 mknod $testdir/dev1 c 1 3 mkfifo $testdir/fifo -echo "Try to copy when source pos > source size" +echo "Try to copy when source pos > source size - should fail" $XFS_IO_PROG -f -c "copy_range -s 1000 -l 100 $testdir/file" "$testdir/copy" -md5sum $testdir/copy | _filter_test_dir echo "Try to copy to a read-only file" +rm -f $testdir/copy $XFS_IO_PROG -r -f -c "copy_range -s 0 -l 100 $testdir/file" "$testdir/copy" md5sum $testdir/copy | _filter_test_dir echo "Try to copy to an append-only file" +rm -f $testdir/copy $XFS_IO_PROG -a -f -c "copy_range -s 0 -l 100 $testdir/file" "$testdir/copy" md5sum $testdir/copy | _filter_test_dir
tests/generic/434.out +2 -2
@@ -1,7 +1,7 @@ QA output created by 434 Create the original files -Try to copy when source pos > source size -d41d8cd98f00b204e9800998ecf8427e TEST_DIR/test-434/copy +Try to copy when source pos > source size - should fail +copy_range: Invalid argument Try to copy to a read-only file copy_range: Bad file descriptor d41d8cd98f00b204e9800998ecf8427e TEST_DIR/test-434/copy
tests/generic/group +5 -5
@@ -432,11 +432,11 @@ 427 auto quick aio rw 428 auto quick dax 429 auto encrypt -430 auto quick copy -431 auto quick copy -432 auto quick copy -433 auto quick copy -434 auto quick copy +430 auto quick copy_range +431 auto quick copy_range +432 auto quick copy_range +433 auto quick copy_range +434 auto quick copy_range 435 auto encrypt 436 auto quick rw 437 auto quick dax
Signature · 1 line
2.19.1
Re: [PATCH 2/3] generic/43[014]: copy_range beyond source EOF should fail
Quoted 5 lines · Dave Chinner
From: Dave Chinner <[email protected]>
As per the
copy_file_range()man page:
Quoted 8 lines
EINVAL
Requested range extends beyond the end of the source file;.....
These tests actually attempt to copy beyond the end of the source file and so should fail with EINVAL. The kernel does not check this and so needs fixing. These operations should fail, so fix the tests.
[cc: linux-api]
Probably better to discuss this on the kernel patch itself, but well...
How confident are you about this change not breaking existing programs?
Thanks,
Amir.
Re: [PATCH 2/3] generic/43[014]: copy_range beyond source EOF should fail
Quoted 10 lines · Amir Goldstein
Quoted 9 lines · Dave Chinner
From: Dave Chinner <[email protected]>
As per the
copy_file_range()man page:EINVAL
Requested range extends beyond the end of the source file;.....
Quoted 8 lines
Quoted 4 lines
These tests actually attempt to copy beyond the end of the source file and so should fail with EINVAL. The kernel does not check this and so needs fixing. These operations should fail, so fix the tests.
[cc: linux-api]
Probably better to discuss this on the kernel patch itself, but well...
I'm getting there - the problem is I need to refer to the tests in the series description that these changes are made.
Quoted 1 line
How confident are you about this change not breaking existing programs?
My care factor is way below zero. The API implementation is so broken right now that fixing it is almost guaranteed to break something somewhere.
-Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH 2/3] generic/43[014]: copy_range beyond source EOF should fail
In reply to Dave ChinnerQuoted 8 lines · Dave Chinner
From: Dave Chinner <[email protected]>
As per the
copy_file_range()man page:EINVAL
Requested range extends beyond the end of the source file;.....
Quoted 8 lines
These tests actually attempt to copy beyond the end of the source file and so should fail with EINVAL. The kernel does not check this and so needs fixing. These operations should fail, so fix the tests.
Also move the tests to the
copy_rangegroup so they are distinct from the copy group which refers toxfs_copytests.Signed-off-by Dave Chinner <[email protected]>
Looks ok; sorry I didn't notice this on the original patch.
--D
Quoted 129 lines
--- tests/generic/430 | 5 +---- tests/generic/430.out | 6 ++---- tests/generic/431 | 1 + tests/generic/431.out | 1 + tests/generic/434 | 5 +++-- tests/generic/434.out | 4 ++-- tests/generic/group | 10 +++++----- 7 files changed, 15 insertions(+), 17 deletions(-)tests/generic/430 +1
-4@@ -70,11 +70,8 @@ cmp -n 1000 $testdir/file $testdir/end 4000 echo "md5sums after copying end:" md5sum $testdir/{file,end} | _filter_test_dir -echo "Copy beyond end of original file" +echo "Copy beyond end of original file - should fail" $XFS_IO_PROG -f -c "copy_range -s 4000 -l 2000 $testdir/file" "$testdir/beyond" -cmp -n 1000 $testdir/file $testdir/beyond 4000 -echo "md5sums after copying beyond:" -md5sum $testdir/{file,beyond} | _filter_test_dir echo "Copy creates hole in target file" $XFS_IO_PROG -f -c "copy_range -s 1000 -l 3000 -d 1000 $testdir/file" "$testdir/hole"tests/generic/430.out +2
-4@@ -15,10 +15,8 @@ Copy end of original file md5sums after copying end: e11fbace556cba26bf0076e74cab90a3 TEST_DIR/test-430/file e68d4a150c4e42f4f9ea3ffe4c9cf4ed TEST_DIR/test-430/end -Copy beyond end of original file -md5sums after copying beyond: -e11fbace556cba26bf0076e74cab90a3 TEST_DIR/test-430/file -e68d4a150c4e42f4f9ea3ffe4c9cf4ed TEST_DIR/test-430/beyond +Copy beyond end of original file - should fail +copy_range: Invalid argument Copy creates hole in target file md5sums after creating hole: e11fbace556cba26bf0076e74cab90a3 TEST_DIR/test-430/filetests/generic/431 +1
-0@@ -52,6 +52,7 @@ $XFS_IO_PROG -f -c "copy_range -s 2 -l 1 $testdir/file" "$testdir/c" $XFS_IO_PROG -f -c "copy_range -s 3 -l 1 $testdir/file" "$testdir/d" $XFS_IO_PROG -f -c "copy_range -s 4 -l 1 $testdir/file" "$testdir/e" $XFS_IO_PROG -f -c "copy_range -s 4 -l 1 -d 1 $testdir/file" "$testdir/f" +# this should fail with EINVAL and leave an empty file behind. $XFS_IO_PROG -f -c "copy_range -s 5 -l 1 $testdir/file" "$testdir/g" echo -n "a" | cmp $testdir/a echo -n "b" | cmp $testdir/btests/generic/431.out +1
-0@@ -4,6 +4,7 @@ Original md5sums: ab56b4d92b40713acc5af89985d4b786 TEST_DIR/test-431/file ab56b4d92b40713acc5af89985d4b786 TEST_DIR/test-431/copy Small copies from various points in the original file +copy_range: Invalid argument md5sums after small copies ab56b4d92b40713acc5af89985d4b786 TEST_DIR/test-431/file 0cc175b9c0f1b6a831c399e269772661 TEST_DIR/test-431/atests/generic/434 +3
-2@@ -41,15 +41,16 @@ $XFS_IO_PROG -f -c 'pwrite -S 0x61 0 1000' $testdir/file >> $seqres.full 2>&1 mknod $testdir/dev1 c 1 3 mkfifo $testdir/fifo -echo "Try to copy when source pos > source size" +echo "Try to copy when source pos > source size - should fail" $XFS_IO_PROG -f -c "copy_range -s 1000 -l 100 $testdir/file" "$testdir/copy" -md5sum $testdir/copy | _filter_test_dir echo "Try to copy to a read-only file" +rm -f $testdir/copy $XFS_IO_PROG -r -f -c "copy_range -s 0 -l 100 $testdir/file" "$testdir/copy" md5sum $testdir/copy | _filter_test_dir echo "Try to copy to an append-only file" +rm -f $testdir/copy $XFS_IO_PROG -a -f -c "copy_range -s 0 -l 100 $testdir/file" "$testdir/copy" md5sum $testdir/copy | _filter_test_dirtests/generic/434.out +2
-2@@ -1,7 +1,7 @@ QA output created by 434 Create the original files -Try to copy when source pos > source size -d41d8cd98f00b204e9800998ecf8427e TEST_DIR/test-434/copy +Try to copy when source pos > source size - should fail +copy_range: Invalid argument Try to copy to a read-only file copy_range: Bad file descriptor d41d8cd98f00b204e9800998ecf8427e TEST_DIR/test-434/copytests/generic/group +5
-5@@ -432,11 +432,11 @@ 427 auto quick aio rw 428 auto quick dax 429 auto encrypt -430 auto quick copy -431 auto quick copy -432 auto quick copy -433 auto quick copy -434 auto quick copy +430 auto quick copy_range +431 auto quick copy_range +432 auto quick copy_range +433 auto quick copy_range +434 auto quick copy_range 435 auto encrypt 436 auto quick rw 437 auto quick daxSignature · 1 line
2.19.1
Re: [PATCH 2/3] generic/43[014]: copy_range beyond source EOF should fail
In reply to Dave ChinnerQuoted 8 lines · Dave Chinner
From: Dave Chinner <[email protected]>
As per the
copy_file_range()man page:EINVAL
Requested range extends beyond the end of the source file;.....
Quoted 8 lines
These tests actually attempt to copy beyond the end of the source file and so should fail with EINVAL. The kernel does not check this and so needs fixing. These operations should fail, so fix the tests.
Also move the tests to the
copy_rangegroup so they are distinct from the copy group which refers toxfs_copytests.Signed-off-by Dave Chinner <[email protected]>
Self NACK on this - the behaviour for this corner case we've decided on is going to be different to what the man page says, so this patch is going to have to be reworked.
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
PATCH 3/3 generic: copy_file_range bounds test
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
Test that copy_file_range will return the correct errors for various error conditions and boundary constraints.
--- tests/generic/530 | 165 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/530.out | 62 ++++++++++++++++ tests/generic/group | 1 + 3 files changed, 228 insertions(+) create mode 100755 tests/generic/530 create mode 100644 tests/generic/530.out
tests/generic/530 new file +165 -0
@@ -0,0 +1,165 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2018 Red Hat, Inc. All Rights Reserved. +# +# FS QA Test No. 530 +# +# Exercise copy_file_range() syscall error conditions. +# +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 7 15 + +_cleanup() +{ + cd / + rm -rf $tmp.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# real QA test starts here +_supported_os Linux +_supported_fs generic + +rm -f $seqres.full + +_require_test +_require_scratch +_require_xfs_io_command "copy_range" +_require_user +_require_test_swapfile + +_scratch_mkfs 2>&1 >> $seqres.full +_scratch_mount + + +testdir=$TEST_DIR/test-$seq +rm -rf $testdir +mkdir $testdir +rm -f $seqres.full + +$XFS_IO_PROG -f -c "pwrite -S 0x61 0 128k" $testdir/file >> $seqres.full 2>&1 +chmod 777 $testdir/file + +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 + +# we have to open the file to be immutable rw and hold it open over the +# chattr command to set it immutable, otherwise we won't be able to open it for +# writing after it's been made immutable. (i.e. would exercise file mode checks, +# not immutable inode flag checks). +echo +echo immutable file returns EPERM +$XFS_IO_PROG -f -c "pwrite -S 0x61 0 64k" -c fsync $testdir/immutable | _filter_xfs_io +$XFS_IO_PROG -f -c "chattr +i" -c "copy_range -l 32k $testdir/file" $testdir/immutable +$XFS_IO_PROG -f -r -c "chattr -i" $testdir/immutable +rm -f $testdir/immutable + +# can't test this as root, because root is allowed to write to files do not +# have write permission bits set. +echo +echo no write perms on destination returns EACCES +chown $qa_user:12345 $testdir/copy +su $qa_user -c "$XFS_IO_PROG -f -c 'chmod -r' -c 'copy_range -l 32k $testdir/file' $testdir/copy" +rm -f $testdir/copy + +echo +echo source range overlaps EOF returns EINVAL +$XFS_IO_PROG -f -c "copy_range -s 112k -l 32k $testdir/file" $testdir/copy + +echo +echo source range beyond EOF returns EINVAL +$XFS_IO_PROG -f -c "copy_range -s 128k -l 32k $testdir/file" $testdir/copy + +echo +echo source range overlaps destination range in same file returns EINVAL +$XFS_IO_PROG -f -c "copy_range -s 32k -d 48k -l 32k $testdir/file" $testdir/file + +echo +echo destination file O_RDONLY returns EBADF +$XFS_IO_PROG -f -r -c "copy_range -l 32k $testdir/file" $testdir/copy + +echo +echo destination file O_APPEND returns EBADF +$XFS_IO_PROG -f -a -c "copy_range -l 32k $testdir/file" $testdir/copy + +echo +echo source/destination as directory returns EISDIR +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir" $testdir/copy + +echo +echo source/destination as blkdev returns EINVAL +mknod $testdir/dev1 b 1 3 +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir/dev1 +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/dev1" $testdir/copy + +echo +echo source/destination as chardev returns EINVAL +mknod $testdir/dev2 c 1 3 +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir/dev2 +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/dev2" $testdir/copy + +echo +echo source/destination as FIFO returns EINVAL +mkfifo $testdir/fifo +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir/fifo +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/fifo" $testdir/copy + +max_off=$((8 * 2**60 - 65536 - 1)) +min_off=65537 + +echo +echo length beyond 8EiB wraps around 0 returns EOVERFLOW +$XFS_IO_PROG -f -c "copy_range -l 10e -s $max_off $testdir/file" $testdir/copy +$XFS_IO_PROG -f -c "copy_range -l 10e -d $max_off $testdir/file" $testdir/copy + +echo +echo source range beyond 8EiB returns EINVAL +$XFS_IO_PROG -c "truncate $((max_off + 65536))" $testdir/file +$XFS_IO_PROG -c "truncate $((max_off + 65536))" $testdir/copy +$XFS_IO_PROG -c "copy_range -s $max_off -l $min_off -d 0 $testdir/file" $testdir/copy +$XFS_IO_PROG -c "copy_range -s $min_off -l $max_off -d 0 $testdir/file" $testdir/copy + +echo +echo destination range beyond 8TiB returns EFBIG +$XFS_IO_PROG -c "copy_range -l $min_off -s 0 -d $max_off $testdir/file" $testdir/copy + +echo +echo destination larger than rlimit returns EFBIG +rm -f $testdir/copy +$XFS_IO_PROG -c "truncate 128k" $testdir/file + +# need a wrapper so the "File size limit exceeded" error can be filtered +do_rlimit_copy() +{ + $XFS_IO_PROG -f -c "copy_range -l 32k -s 0 -d 16m $testdir/file" $testdir/copy +} + +ulimit -f $((8 * 1024)) +ulimit -c 0 +do_rlimit_copy 2>&1 | grep -o "File size limit exceeded" +ulimit -f unlimited + +echo +echo copy across devices +$XFS_IO_PROG -f -c "copy_range -l 128k $testdir/file" $SCRATCH_MNT/copy +cmp $testdir/file $SCRATCH_MNT/copy +echo "md5sums after xdev copy:" +md5sum $testdir/file $SCRATCH_MNT/copy | _filter_test_dir | _filter_scratch + +# success, all done +status=0 +exit
tests/generic/530.out new file +62 -0
62 lines not on this page · raw message
tests/generic/group +1 -0
@@ -523,3 +523,4 @@ 517 auto quick dedupe clone 518 auto quick clone 519 auto quick +530 auto quick copy_range
Signature · 1 line
2.19.1
Re: [PATCH 3/3] generic: copy_file_range bounds test
Quoted 173 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Test that
copy_file_rangewill return the correct errors for various error conditions and boundary constraints.Signed-off-by Dave Chinner <[email protected]>--- tests/generic/530 | 165 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/530.out | 62 ++++++++++++++++ tests/generic/group | 1 + 3 files changed, 228 insertions(+) create mode 100755 tests/generic/530 create mode 100644 tests/generic/530.outtests/generic/530 new file +152
-0@@ -0,0 +1,165 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2018 Red Hat, Inc. All Rights Reserved. +# +# FS QA Test No. 530 +# +# Exercise copy_file_range() syscall error conditions. +# +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 7 15 + +_cleanup() +{ + cd / + rm -rf $tmp.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# real QA test starts here +_supported_os Linux +_supported_fs generic + +rm -f $seqres.full + +_require_test +_require_scratch +_require_xfs_io_command "copy_range" +_require_user +_require_test_swapfile + +_scratch_mkfs 2>&1 >> $seqres.full +_scratch_mount + + +testdir=$TEST_DIR/test-$seq +rm -rf $testdir +mkdir $testdir +rm -f $seqres.full + +$XFS_IO_PROG -f -c "pwrite -S 0x61 0 128k" $testdir/file >> $seqres.full 2>&1 +chmod 777 $testdir/file + +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 + +# we have to open the file to be immutable rw and hold it open over the +# chattr command to set it immutable, otherwise we won't be able to open it for +# writing after it's been made immutable. (i.e. would exercise file mode checks, +# not immutable inode flag checks). +echo +echo immutable file returns EPERM +$XFS_IO_PROG -f -c "pwrite -S 0x61 0 64k" -c fsync $testdir/immutable | _filter_xfs_io +$XFS_IO_PROG -f -c "chattr +i" -c "copy_range -l 32k $testdir/file" $testdir/immutable +$XFS_IO_PROG -f -r -c "chattr -i" $testdir/immutable +rm -f $testdir/immutable + +# can't test this as root, because root is allowed to write to files do not +# have write permission bits set. +echo +echo no write perms on destination returns EACCES +chown $qa_user:12345 $testdir/copy +su $qa_user -c "$XFS_IO_PROG -f -c 'chmod -r' -c 'copy_range -l 32k $testdir/file' $testdir/copy" +rm -f $testdir/copy + +echo +echo source range overlaps EOF returns EINVAL +$XFS_IO_PROG -f -c "copy_range -s 112k -l 32k $testdir/file" $testdir/copy + +echo +echo source range beyond EOF returns EINVAL +$XFS_IO_PROG -f -c "copy_range -s 128k -l 32k $testdir/file" $testdir/copy + +echo +echo source range overlaps destination range in same file returns EINVAL +$XFS_IO_PROG -f -c "copy_range -s 32k -d 48k -l 32k $testdir/file" $testdir/file + +echo +echo destination file O_RDONLY returns EBADF +$XFS_IO_PROG -f -r -c "copy_range -l 32k $testdir/file" $testdir/copy + +echo +echo destination file O_APPEND returns EBADF +$XFS_IO_PROG -f -a -c "copy_range -l 32k $testdir/file" $testdir/copy + +echo +echo source/destination as directory returns EISDIR +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir" $testdir/copy + +echo +echo source/destination as blkdev returns EINVAL +mknod $testdir/dev1 b 1 3 +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir/dev1 +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/dev1" $testdir/copy + +echo +echo source/destination as chardev returns EINVAL +mknod $testdir/dev2 c 1 3 +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir/dev2 +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/dev2" $testdir/copy + +echo +echo source/destination as FIFO returns EINVAL +mkfifo $testdir/fifo +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir/fifo +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/fifo" $testdir/copy + +max_off=$((8 * 2**60 - 65536 - 1)) +min_off=65537 + +echo +echo length beyond 8EiB wraps around 0 returns EOVERFLOW +$XFS_IO_PROG -f -c "copy_range -l 10e -s $max_off $testdir/file" $testdir/copy +$XFS_IO_PROG -f -c "copy_range -l 10e -d $max_off $testdir/file" $testdir/copy + +echo +echo source range beyond 8EiB returns EINVAL +$XFS_IO_PROG -c "truncate $((max_off + 65536))" $testdir/file +$XFS_IO_PROG -c "truncate $((max_off + 65536))" $testdir/copy +$XFS_IO_PROG -c "copy_range -s $max_off -l $min_off -d 0 $testdir/file" $testdir/copy +$XFS_IO_PROG -c "copy_range -s $min_off -l $max_off -d 0 $testdir/file" $testdir/copy + +echo +echo destination range beyond 8TiB returns EFBIG +$XFS_IO_PROG -c "copy_range -l $min_off -s 0 -d $max_off $testdir/file" $testdir/copy + +echo +echo destination larger than rlimit returns EFBIG +rm -f $testdir/copy +$XFS_IO_PROG -c "truncate 128k" $testdir/file + +# need a wrapper so the "File size limit exceeded" error can be filtered +do_rlimit_copy() +{ + $XFS_IO_PROG -f -c "copy_range -l 32k -s 0 -d 16m $testdir/file" $testdir/copy +} + +ulimit -f $((8 * 1024)) +ulimit -c 0
tests/generic/530 line 162
Quoted 10 lines
tests/generic/530 new file +10
-0+do_rlimit_copy 2>&1 | grep -o "File size limit exceeded" +ulimit -f unlimited + +echo +echo copy across devices +$XFS_IO_PROG -f -c "copy_range -l 128k $testdir/file" $SCRATCH_MNT/copy +cmp $testdir/file $SCRATCH_MNT/copy +echo "md5sums after xdev copy:" +md5sum $testdir/file $SCRATCH_MNT/copy | _filter_test_dir | _filter_scratch +
All the test cases above check for bugs, which I presume your kernel patch series is aimed at fixing(?)
This one last test case tests for new functionality that is not currently available
for any filesystem in upstream kernel.
Does your kernel patch set also add this functionality to xfs? to generic? IMO, it would be better to split this test case for new functionality to a new test, so that this one can pass on stable kernels once all the bug fixes have been
applied.
Thanks,
Amir.
Re: [PATCH 3/3] generic: copy_file_range bounds test
Quoted 6 lines · Amir Goldstein
Quoted 5 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Test that
copy_file_rangewill return the correct errors for various error conditions and boundary constraints.
....
Quoted 3 lines
All the test cases above check for bugs, which I presume your kernel patch series is aimed at fixing(?)
Yes. Document the API (I have a man page patch) write the tests to exercise correct API behaviour (these patches), fix the API implementation until the tests start passing (still to be posted as I wait for these to hit mailing list archives so I can point at them).
Quoted 3 lines
This one last test case tests for new functionality that is not currently available
for any filesystem in upstream kernel.
Yup.
Quoted 1 line
Does your kernel patch set also add this functionality to xfs? to generic?
Yes and yes. overlay works, too, but I gave up caring about it because it doesn't support the ioctls xfs_io uses in this test to change open file state....
Quoted 4 lines
IMO, it would be better to split this test case for new functionality to a new test, so that this one can pass on stable kernels once all the bug fixes have been
applied.
Whatever. I'm tired, I've already put in 13 hours on this today and I'm on the back of four 100+ hour weeks working on nothing but this broken heap of crap.
Take it or leave it, because I'm just about burnt out on this right now...
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH 3/3] generic: copy_file_range bounds test
Quoted 23 lines · Dave Chinner
Quoted 6 lines · Amir Goldstein
Quoted 5 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Test that
copy_file_rangewill return the correct errors for various error conditions and boundary constraints.....
Quoted 3 lines
All the test cases above check for bugs, which I presume your kernel patch series is aimed at fixing(?)
Yes. Document the API (I have a man page patch) write the tests to exercise correct API behaviour (these patches), fix the API implementation until the tests start passing (still to be posted as I wait for these to hit mailing list archives so I can point at them).
Quoted 3 lines
This one last test case tests for new functionality that is not currently available
for any filesystem in upstream kernel.
Quoted 7 lines
Yup.
Quoted 1 line
Does your kernel patch set also add this functionality to xfs? to generic?
Yes and yes. overlay works, too, but I gave up caring about it because it doesn't support the ioctls
xfs_iouses in this test to change open file state....
Oh, not a problem. I can add FS_IOC_FS[SG]ETXATTR
to the overlayfs white list of ioctls.
That raises the issue that the test is missing:
_require_xfs_io_command "chattr" "-i"
and the chattr -i in cleanup().
Quoted 6 lines
Quoted 5 lines
IMO, it would be better to split this test case for new functionality to a new test, so that this one can pass on stable kernels once all the bug fixes have been
applied.
Quoted 7 lines
Whatever. I'm tired, I've already put in 13 hours on this today and I'm on the back of four 100+ hour weeks working on nothing but this broken heap of crap.
Take it or leave it, because I'm just about burnt out on this right now...
I hear you.
I personally have no problem with declaring the cross fs copy_range an interface bug fix that can also go to stable. It's probably going to be harder to get your interface fixes without it.
I will leave the call about insisting on separate test to Eryu. If you want me to submit that separate test, I can do that.
Thanks,
Amir.
Re: [PATCH 3/3] generic: copy_file_range bounds test
Quoted 33 lines · Amir Goldstein
Quoted 23 lines · Dave Chinner
Quoted 6 lines · Amir Goldstein
Quoted 5 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Test that
copy_file_rangewill return the correct errors for various error conditions and boundary constraints.....
Quoted 3 lines
All the test cases above check for bugs, which I presume your kernel patch series is aimed at fixing(?)
Yes. Document the API (I have a man page patch) write the tests to exercise correct API behaviour (these patches), fix the API implementation until the tests start passing (still to be posted as I wait for these to hit mailing list archives so I can point at them).
Quoted 3 lines
This one last test case tests for new functionality that is not currently available
for any filesystem in upstream kernel.Quoted 7 lines
Yup.
Quoted 1 line
Does your kernel patch set also add this functionality to xfs? to generic?
Yes and yes. overlay works, too, but I gave up caring about it because it doesn't support the ioctls
xfs_iouses in this test to change open file state....
Quoted 7 lines
Oh, not a problem. I can add FS_IOC_FS[SG]ETXATTR
to the overlayfs white list of ioctls.That raises the issue that the test is missing:
_require_xfs_io_command"chattr" "-i"
Hmm._require_xfs_io_command "chattr" "+/-x"
is documented in doc/requirement-checking.txt, but that seems like fake news.
Nevermind. I'll add support for it after implementing FS_IOC_FS[SG]ETXATTR for overlayfs.
Quoted 2 lines
and the chattr -i in
cleanup().
That's still important of course.
Thanks,
Amir.
Re: [PATCH 3/3] generic: copy_file_range bounds test
In reply to Amir GoldsteinQuoted 56 lines · Amir Goldstein
Quoted 23 lines · Dave Chinner
Quoted 6 lines · Amir Goldstein
Quoted 5 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Test that
copy_file_rangewill return the correct errors for various error conditions and boundary constraints.....
Quoted 3 lines
All the test cases above check for bugs, which I presume your kernel patch series is aimed at fixing(?)
Yes. Document the API (I have a man page patch) write the tests to exercise correct API behaviour (these patches), fix the API implementation until the tests start passing (still to be posted as I wait for these to hit mailing list archives so I can point at them).
Quoted 3 lines
This one last test case tests for new functionality that is not currently available
for any filesystem in upstream kernel.Quoted 7 lines
Yup.
Quoted 1 line
Does your kernel patch set also add this functionality to xfs? to generic?
Yes and yes. overlay works, too, but I gave up caring about it because it doesn't support the ioctls
xfs_iouses in this test to change open file state....Oh, not a problem. I can add FS_IOC_FS[SG]ETXATTR
to the overlayfs white list of ioctls.That raises the issue that the test is missing:
_require_xfs_io_command"chattr" "-i"and the chattr -i in
cleanup().Quoted 6 lines
Quoted 5 lines
IMO, it would be better to split this test case for new functionality to a new test, so that this one can pass on stable kernels once all the bug fixes have been
applied.Quoted 7 lines
Whatever. I'm tired, I've already put in 13 hours on this today and I'm on the back of four 100+ hour weeks working on nothing but this broken heap of crap.
Take it or leave it, because I'm just about burnt out on this right now...
Quoted 9 lines
I hear you.
I personally have no problem with declaring the cross fs
copy_rangean interface bug fix that can also go to stable. It's probably going to be harder to get your interface fixes without it.I will leave the call about insisting on separate test to Eryu. If you want me to submit that separate test, I can do that.
Getting back to this (how time flies...)
Dave, if you don't mind I am going to re-post your tests splitting the cross device copy test, since Darrick also echoed this request.
Cheers,
Amir.
Re: [PATCH 3/3] generic: copy_file_range bounds test
In reply to Dave ChinnerQuoted 171 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Test that
copy_file_rangewill return the correct errors for various error conditions and boundary constraints.Signed-off-by Dave Chinner <[email protected]>--- tests/generic/530 | 165 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/530.out | 62 ++++++++++++++++ tests/generic/group | 1 + 3 files changed, 228 insertions(+) create mode 100755 tests/generic/530 create mode 100644 tests/generic/530.outtests/generic/530 new file +151
-0@@ -0,0 +1,165 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2018 Red Hat, Inc. All Rights Reserved. +# +# FS QA Test No. 530 +# +# Exercise copy_file_range() syscall error conditions. +# +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 7 15 + +_cleanup() +{ + cd / + rm -rf $tmp.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# real QA test starts here +_supported_os Linux +_supported_fs generic + +rm -f $seqres.full + +_require_test +_require_scratch +_require_xfs_io_command "copy_range" +_require_user +_require_test_swapfile + +_scratch_mkfs 2>&1 >> $seqres.full +_scratch_mount + + +testdir=$TEST_DIR/test-$seq +rm -rf $testdir +mkdir $testdir +rm -f $seqres.full + +$XFS_IO_PROG -f -c "pwrite -S 0x61 0 128k" $testdir/file >> $seqres.full 2>&1 +chmod 777 $testdir/file + +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 + +# we have to open the file to be immutable rw and hold it open over the +# chattr command to set it immutable, otherwise we won't be able to open it for +# writing after it's been made immutable. (i.e. would exercise file mode checks, +# not immutable inode flag checks). +echo +echo immutable file returns EPERM +$XFS_IO_PROG -f -c "pwrite -S 0x61 0 64k" -c fsync $testdir/immutable | _filter_xfs_io +$XFS_IO_PROG -f -c "chattr +i" -c "copy_range -l 32k $testdir/file" $testdir/immutable +$XFS_IO_PROG -f -r -c "chattr -i" $testdir/immutable +rm -f $testdir/immutable + +# can't test this as root, because root is allowed to write to files do not +# have write permission bits set. +echo +echo no write perms on destination returns EACCES +chown $qa_user:12345 $testdir/copy +su $qa_user -c "$XFS_IO_PROG -f -c 'chmod -r' -c 'copy_range -l 32k $testdir/file' $testdir/copy" +rm -f $testdir/copy + +echo +echo source range overlaps EOF returns EINVAL +$XFS_IO_PROG -f -c "copy_range -s 112k -l 32k $testdir/file" $testdir/copy + +echo +echo source range beyond EOF returns EINVAL +$XFS_IO_PROG -f -c "copy_range -s 128k -l 32k $testdir/file" $testdir/copy + +echo +echo source range overlaps destination range in same file returns EINVAL +$XFS_IO_PROG -f -c "copy_range -s 32k -d 48k -l 32k $testdir/file" $testdir/file + +echo +echo destination file O_RDONLY returns EBADF +$XFS_IO_PROG -f -r -c "copy_range -l 32k $testdir/file" $testdir/copy + +echo +echo destination file O_APPEND returns EBADF +$XFS_IO_PROG -f -a -c "copy_range -l 32k $testdir/file" $testdir/copy + +echo +echo source/destination as directory returns EISDIR +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir" $testdir/copy + +echo +echo source/destination as blkdev returns EINVAL +mknod $testdir/dev1 b 1 3 +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir/dev1 +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/dev1" $testdir/copy + +echo +echo source/destination as chardev returns EINVAL +mknod $testdir/dev2 c 1 3 +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir/dev2 +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/dev2" $testdir/copy + +echo +echo source/destination as FIFO returns EINVAL +mkfifo $testdir/fifo +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir/fifo +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/fifo" $testdir/copy + +max_off=$((8 * 2**60 - 65536 - 1)) +min_off=65537 + +echo +echo length beyond 8EiB wraps around 0 returns EOVERFLOW +$XFS_IO_PROG -f -c "copy_range -l 10e -s $max_off $testdir/file" $testdir/copy +$XFS_IO_PROG -f -c "copy_range -l 10e -d $max_off $testdir/file" $testdir/copy + +echo +echo source range beyond 8EiB returns EINVAL +$XFS_IO_PROG -c "truncate $((max_off + 65536))" $testdir/file +$XFS_IO_PROG -c "truncate $((max_off + 65536))" $testdir/copy +$XFS_IO_PROG -c "copy_range -s $max_off -l $min_off -d 0 $testdir/file" $testdir/copy +$XFS_IO_PROG -c "copy_range -s $min_off -l $max_off -d 0 $testdir/file" $testdir/copy + +echo +echo destination range beyond 8TiB returns EFBIG +$XFS_IO_PROG -c "copy_range -l $min_off -s 0 -d $max_off $testdir/file" $testdir/copy + +echo +echo destination larger than rlimit returns EFBIG +rm -f $testdir/copy +$XFS_IO_PROG -c "truncate 128k" $testdir/file + +# need a wrapper so the "File size limit exceeded" error can be filtered +do_rlimit_copy() +{ + $XFS_IO_PROG -f -c "copy_range -l 32k -s 0 -d 16m $testdir/file" $testdir/copy +} + +ulimit -f $((8 * 1024))
tests/generic/530 line 161
Quoted 10 lines
tests/generic/530 new file +10
-0+ulimit -c 0 +do_rlimit_copy 2>&1 | grep -o "File size limit exceeded" +ulimit -f unlimited + +echo +echo copy across devices +$XFS_IO_PROG -f -c "copy_range -l 128k $testdir/file" $SCRATCH_MNT/copy +cmp $testdir/file $SCRATCH_MNT/copy +echo "md5sums after xdev copy:" +md5sum $testdir/file $SCRATCH_MNT/copy | _filter_test_dir | _filter_scratch
Echoing Amir, this should be a separate test because it tests new functionality.
Really wishing we'd put a WARN_ONCE(1, "Using EXPERIMENTAL copy_file_range syscall"); in when this got merged. :( :(
--D
Quoted 83 lines
tests/generic/530 new file +3
-0+# success, all done +status=0 +exittests/generic/530.out new file +62
-0@@ -0,0 +1,62 @@ +QA output created by 530 +swap files return ETXTBUSY +copy_range: Text file busy +copy_range: Text file busy + +immutable file returns EPERM +wrote 65536/65536 bytes at offset 0 +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +copy_range: Operation not permitted + +no write perms on destination returns EACCES +copy_range: Permission denied + +source range overlaps EOF returns EINVAL +copy_range: Invalid argument + +source range beyond EOF returns EINVAL +copy_range: Invalid argument + +source range overlaps destination range in same file returns EINVAL +copy_range: Invalid argument + +destination file O_RDONLY returns EBADF +copy_range: Bad file descriptor + +destination file O_APPEND returns EBADF +copy_range: Bad file descriptor + +source/destination as directory returns EISDIR +copy_range: Is a directory +copy_range: Is a directory + +source/destination as blkdev returns EINVAL +copy_range: Invalid argument +copy_range: Invalid argument + +source/destination as chardev returns EINVAL +copy_range: Invalid argument +copy_range: Invalid argument + +source/destination as FIFO returns EINVAL +copy_range: Invalid argument +copy_range: Invalid argument + +length beyond 8EiB wraps around 0 returns EOVERFLOW +copy_range: Value too large for defined data type +copy_range: Value too large for defined data type + +source range beyond 8EiB returns EINVAL +copy_range: Invalid argument +copy_range: Invalid argument + +destination range beyond 8TiB returns EFBIG +copy_range: File too large + +destination larger than rlimit returns EFBIG +File size limit exceeded + +copy across devices +md5sums after xdev copy: +81615449a98aaaad8dc179b3bec87f38 TEST_DIR/test-530/file +81615449a98aaaad8dc179b3bec87f38 SCRATCH_MNT/copytests/generic/group +1
-0@@ -523,3 +523,4 @@ 517 auto quick dedupe clone 518 auto quick clone 519 auto quick +530 auto quick copy_rangeSignature · 1 line
2.19.1
Re: [PATCH 3/3] generic: copy_file_range bounds test
In reply to Dave ChinnerQuoted 5 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Test that
copy_file_rangewill return the correct errors for various error conditions and boundary constraints.
Below are the changes I have made to V2 on this test in accordance to agreed change of behavior (i.e. short copy up to EOF).
This is a heads up before posting to verify my interpretation is correct. I still have more testing to do before posting.
Quoted 44 lines
Signed-off-by Dave Chinner <[email protected]>--- tests/generic/530 | 165 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/530.out | 62 ++++++++++++++++ tests/generic/group | 1 + 3 files changed, 228 insertions(+) create mode 100755 tests/generic/530 create mode 100644 tests/generic/530.outtests/generic/530 new file +28
-0@@ -0,0 +1,165 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2018 Red Hat, Inc. All Rights Reserved. +# +# FS QA Test No. 530 +# +# Exercise copy_file_range() syscall error conditions. +# +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 7 15 + +_cleanup() +{ + cd / + rm -rf $tmp.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# real QA test starts here
tests/generic/530 line 38
Quoted 10 lines
tests/generic/530 new file +10
-0+_supported_os Linux +_supported_fs generic + +rm -f $seqres.full + +_require_test +_require_scratch +_require_xfs_io_command "copy_range" +_require_user +_require_test_swapfile
Testing on scratch dev, as cross-dev test was split to a new test.
Quoted 25 lines
tests/generic/530 new file +25
-0+ +_scratch_mkfs 2>&1 >> $seqres.full +_scratch_mount + + +testdir=$TEST_DIR/test-$seq +rm -rf $testdir +mkdir $testdir +rm -f $seqres.full + +$XFS_IO_PROG -f -c "pwrite -S 0x61 0 128k" $testdir/file >> $seqres.full 2>&1 +chmod 777 $testdir/file + +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 + +# we have to open the file to be immutable rw and hold it open over the +# chattr command to set it immutable, otherwise we won't be able to open it for +# writing after it's been made immutable. (i.e. would exercise file mode checks, +# not immutable inode flag checks). +echo
tests/generic/530 line 73
Quoted 10 lines
tests/generic/530 new file +10
-0+echo immutable file returns EPERM +$XFS_IO_PROG -f -c "pwrite -S 0x61 0 64k" -c fsync $testdir/immutable | _filter_xfs_io +$XFS_IO_PROG -f -c "chattr +i" -c "copy_range -l 32k $testdir/file" $testdir/immutable +$XFS_IO_PROG -f -r -c "chattr -i" $testdir/immutable +rm -f $testdir/immutable + +# can't test this as root, because root is allowed to write to files do not +# have write permission bits set. +echo +echo no write perms on destination returns EACCES
This test case was removed, because it does not comply with current behavior of write(2).
Quoted 47 lines
tests/generic/530 new file +47
-0+chown $qa_user:12345 $testdir/copy +su $qa_user -c "$XFS_IO_PROG -f -c 'chmod -r' -c 'copy_range -l 32k $testdir/file' $testdir/copy" +rm -f $testdir/copy + +echo +echo source range overlaps EOF returns EINVAL +$XFS_IO_PROG -f -c "copy_range -s 112k -l 32k $testdir/file" $testdir/copy + +echo +echo source range beyond EOF returns EINVAL +$XFS_IO_PROG -f -c "copy_range -s 128k -l 32k $testdir/file" $testdir/copy + +echo +echo source range overlaps destination range in same file returns EINVAL +$XFS_IO_PROG -f -c "copy_range -s 32k -d 48k -l 32k $testdir/file" $testdir/file + +echo +echo destination file O_RDONLY returns EBADF +$XFS_IO_PROG -f -r -c "copy_range -l 32k $testdir/file" $testdir/copy + +echo +echo destination file O_APPEND returns EBADF +$XFS_IO_PROG -f -a -c "copy_range -l 32k $testdir/file" $testdir/copy + +echo +echo source/destination as directory returns EISDIR +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir" $testdir/copy + +echo +echo source/destination as blkdev returns EINVAL +mknod $testdir/dev1 b 1 3 +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir/dev1 +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/dev1" $testdir/copy + +echo +echo source/destination as chardev returns EINVAL +mknod $testdir/dev2 c 1 3 +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir/dev2 +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/dev2" $testdir/copy + +echo +echo source/destination as FIFO returns EINVAL +mkfifo $testdir/fifo +$XFS_IO_PROG -c "copy_range -l 32k $testdir/file" $testdir/fifo +$XFS_IO_PROG -f -c "copy_range -l 32k $testdir/fifo" $testdir/copy +
tests/generic/530 line 130
Quoted 10 lines
tests/generic/530 new file +10
-0+max_off=$((8 * 2**60 - 65536 - 1)) +min_off=65537 + +echo +echo length beyond 8EiB wraps around 0 returns EOVERFLOW +$XFS_IO_PROG -f -c "copy_range -l 10e -s $max_off $testdir/file" $testdir/copy +$XFS_IO_PROG -f -c "copy_range -l 10e -d $max_off $testdir/file" $testdir/copy + +echo +echo source range beyond 8EiB returns EINVAL
With accordance to implemented behavior and short copy to EOF, this was changed to:
echo source range beyond 8TiB returns EFBIG $XFS_IO_PROG -c "copy_range -s $max_off -l $min_off -d 0 $testdir/file" $testdir/copy
tests/generic/530 line 132
Quoted 2 lines
tests/generic/530 new file +2
-0+$XFS_IO_PROG -c "truncate $((max_off + 65536))" $testdir/file +$XFS_IO_PROG -c "truncate $((max_off + 65536))" $testdir/copy
These ftruncates would fail with EFBIG anyway...
tests/generic/530 line 134
Quoted 2 lines
tests/generic/530 new file +2
-0+$XFS_IO_PROG -c "copy_range -s $max_off -l $min_off -d 0 $testdir/file" $testdir/copy +$XFS_IO_PROG -c "copy_range -s $min_off -l $max_off -d 0 $testdir/file" $testdir/copy
This second test does not fail because of short copy.
Quoted 18 lines
tests/generic/530 new file +18
-0+ +echo +echo destination range beyond 8TiB returns EFBIG +$XFS_IO_PROG -c "copy_range -l $min_off -s 0 -d $max_off $testdir/file" $testdir/copy + +echo +echo destination larger than rlimit returns EFBIG +rm -f $testdir/copy +$XFS_IO_PROG -c "truncate 128k" $testdir/file + +# need a wrapper so the "File size limit exceeded" error can be filtered +do_rlimit_copy() +{ + $XFS_IO_PROG -f -c "copy_range -l 32k -s 0 -d 16m $testdir/file" $testdir/copy +} + +ulimit -f $((8 * 1024)) +ulimit -c 0
tests/generic/530 line 162
Quoted 10 lines
tests/generic/530 new file +10
-0+do_rlimit_copy 2>&1 | grep -o "File size limit exceeded" +ulimit -f unlimited + +echo +echo copy across devices +$XFS_IO_PROG -f -c "copy_range -l 128k $testdir/file" $SCRATCH_MNT/copy +cmp $testdir/file $SCRATCH_MNT/copy +echo "md5sums after xdev copy:" +md5sum $testdir/file $SCRATCH_MNT/copy | _filter_test_dir | _filter_scratch +
Cross-device copy test was split out to a new test, see WIP:
https://github.com/amir73il/xfstests/commits/copy_file_range
Thanks,
Amir.
Quoted 6 lines · Dave Chinner
Hi folks,
We suck at bounds testing new system calls. This is a test that exercises the expected failure cases for
copy_file_range(). It's going to fail miserably on existing kernels - I'm about to post a series of fixes to linux-fsdevel that make this test pass.
Can you please cc the xfs list on fstests changes that affect xfs? :)
(says a prolific patchbomber :P)
Quoted 29 lines
The test is also dependent on
xfs_iochanges that I posted a few hours ago. The three (not 2!) patches can be found here:https://marc.info/?l=linux-xfs&m=154378403323889&w=2
https://marc.info/?l=linux-xfs&m=154378403523890&w=2
https://marc.info/?l=linux-xfs&m=154378403323888&w=2
https://marc.info/?l=linux-xfs&m=154379644526132&w=2Comments welcome.
-Dave
---
As an aside, one thing that I've discovered in writing this test is that certain things we do to test certain file conditions are not being tested corectly. e.g. immutable files.
When we set a file as immutable and then go to modify it with
xfs_iolike this:# xfs_io -c "chattr +i" test_file # xfs_io -c "pwrite 0 4k" test_fileWe are not actually testing whether the
pwrite()syscall detected that it can't write to an immutable file.xfs_ioactually fails when theopen(O_RDWR)syscall fails because we can't open an immutable file for writing. IOWs, it's not testingpwrite()at all.
Quoted 7 lines
Hence tests like generic/159 and generic/160 are not actually testing whether cloning/deduping files fails on immutable files.
Instead, what we need to do is open the file
O_RDWR, then set the file immutable, then perform the modification operation. i.e. this:# xfs_io -c "chattr +i" -c "pwrite 0 4k" test_file
Ok, I'll post a change to 159/160 to fix that. I don't think btrfs got this right (inode_permission vs. security_file_permission) either, which is why the hoisted code and tests (mis)behave the way they do.
Quoted 8 lines
Will exercise the
pwrite()syscall hitting an immutable file. A similar thing happens with trying to write/modify to read only files
- theopen()call fails, not the call that we want to test. That's why I added the "chmod" operation toxfs_io, to allow us to open a file for write, then turn it read only while we still have a writeable fd open. This then exercises trying to write/modify a read-only file.
Quoted 4 lines
I'm sure there's lots of tests that have these problems. I don't have time to audit them right now, but I'm bringing it up so that people are aware of the issue and at least catch problems like this in new tests....
--D