| Message ID | 9c64fbf9ad37dc84a31caf91762edd64b33d59db.1647461985.git.boris@bur.io (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tests for btrfs fsverity | expand |
On Wed, Mar 16, 2022 at 01:25:12PM -0700, Boris Burkov wrote: > generic/572-579 have tests for fsverity. Now that btrfs supports > fsverity, make these tests function as well. For a majority of the tests > that pass, simply adding the case to mkfs a btrfs filesystem with no > extra options is sufficient. > > However, generic/574 has tests for corrupting the merkle tree itself. > Since btrfs uses a different scheme from ext4 and f2fs for storing this > data, the existing logic for corrupting it doesn't work out of the box. > Adapt it to properly corrupt btrfs merkle items. > > 576 does not run because btrfs does not support transparent encryption. > > This test relies on the btrfs implementation of fsverity in the patch: > btrfs: initial fsverity support > > and on btrfs-corrupt-block for corruption in the patches titled: > btrfs-progs: corrupt generic item data with btrfs-corrupt-block > btrfs-progs: expand corrupt_file_extent in btrfs-corrupt-block > > Signed-off-by: Boris Burkov <boris@bur.io> > --- > common/btrfs | 5 +++++ > common/config | 1 + > common/verity | 23 +++++++++++++++++++++++ > 3 files changed, 29 insertions(+) Reviewed-by: Eric Biggers <ebiggers@google.com> - Eric
On Thu, Mar 17, 2022 at 06:16:45PM +0000, Eric Biggers wrote: > On Wed, Mar 16, 2022 at 01:25:12PM -0700, Boris Burkov wrote: > > generic/572-579 have tests for fsverity. Now that btrfs supports > > fsverity, make these tests function as well. For a majority of the tests > > that pass, simply adding the case to mkfs a btrfs filesystem with no > > extra options is sufficient. > > > > However, generic/574 has tests for corrupting the merkle tree itself. > > Since btrfs uses a different scheme from ext4 and f2fs for storing this > > data, the existing logic for corrupting it doesn't work out of the box. > > Adapt it to properly corrupt btrfs merkle items. > > > > 576 does not run because btrfs does not support transparent encryption. > > > > This test relies on the btrfs implementation of fsverity in the patch: > > btrfs: initial fsverity support > > > > and on btrfs-corrupt-block for corruption in the patches titled: > > btrfs-progs: corrupt generic item data with btrfs-corrupt-block > > btrfs-progs: expand corrupt_file_extent in btrfs-corrupt-block > > > > Signed-off-by: Boris Burkov <boris@bur.io> > > --- > > common/btrfs | 5 +++++ > > common/config | 1 + > > common/verity | 23 +++++++++++++++++++++++ > > 3 files changed, 29 insertions(+) > > Reviewed-by: Eric Biggers <ebiggers@google.com> > > - Eric Eric, Unfortunately, I think I found a more serious problem with the compatibility of generic/574 and btrfs while working on testing the enable/disable sysctls. I realized that I had forgotten to customize the mount options for btrfs to use "nodatasum" and as a result, this test was passing for btrfs inappropriately, since we were getting EIOs for failing data checksums, not verity checks. I fixed the mount option issue only to realize that some of the test cases make assumptions that don't apply to btrfs. For example: "corruption_test 130999 131000 72" Btrfs zeros pages past EOF in readpage before they make it to the user via read or mmap, and the fsverity check is done at that point, so corrupting the disk past EOF does not cause a verity failure (or get leaked to the user) but it does cause csum failures since those are done on bios, like verity checks in ext4. I verified that removing that zeroing in readpage makes the test case pass. Do you have a preference for how I might fix this? My first thought is to try to factor out any such test cases into a new test with a new requires clause that btrfs fails but ext4/f2fs pass, kind of like what we did for the EFBIG test for future FSes that might not logically address the Merkle tree in the past-EOF space. Thanks, Boris
On Fri, Apr 22, 2022 at 04:27:29PM -0700, Boris Burkov wrote: > On Thu, Mar 17, 2022 at 06:16:45PM +0000, Eric Biggers wrote: > > On Wed, Mar 16, 2022 at 01:25:12PM -0700, Boris Burkov wrote: > > > generic/572-579 have tests for fsverity. Now that btrfs supports > > > fsverity, make these tests function as well. For a majority of the tests > > > that pass, simply adding the case to mkfs a btrfs filesystem with no > > > extra options is sufficient. > > > > > > However, generic/574 has tests for corrupting the merkle tree itself. > > > Since btrfs uses a different scheme from ext4 and f2fs for storing this > > > data, the existing logic for corrupting it doesn't work out of the box. > > > Adapt it to properly corrupt btrfs merkle items. > > > > > > 576 does not run because btrfs does not support transparent encryption. > > > > > > This test relies on the btrfs implementation of fsverity in the patch: > > > btrfs: initial fsverity support > > > > > > and on btrfs-corrupt-block for corruption in the patches titled: > > > btrfs-progs: corrupt generic item data with btrfs-corrupt-block > > > btrfs-progs: expand corrupt_file_extent in btrfs-corrupt-block > > > > > > Signed-off-by: Boris Burkov <boris@bur.io> > > > --- > > > common/btrfs | 5 +++++ > > > common/config | 1 + > > > common/verity | 23 +++++++++++++++++++++++ > > > 3 files changed, 29 insertions(+) > > > > Reviewed-by: Eric Biggers <ebiggers@google.com> > > > > - Eric > > Eric, > > Unfortunately, I think I found a more serious problem with the > compatibility of generic/574 and btrfs while working on testing the > enable/disable sysctls. > > I realized that I had forgotten to customize the mount options for btrfs > to use "nodatasum" and as a result, this test was passing for btrfs > inappropriately, since we were getting EIOs for failing data checksums, > not verity checks. > > I fixed the mount option issue only to realize that some of the test > cases make assumptions that don't apply to btrfs. For example: > "corruption_test 130999 131000 72" > > Btrfs zeros pages past EOF in readpage before they make it to the user > via read or mmap, and the fsverity check is done at that point, so > corrupting the disk past EOF does not cause a verity failure (or get > leaked to the user) but it does cause csum failures since those are done > on bios, like verity checks in ext4. I verified that removing that > zeroing in readpage makes the test case pass. > > Do you have a preference for how I might fix this? My first thought is > to try to factor out any such test cases into a new test with a new > requires clause that btrfs fails but ext4/f2fs pass, kind of like what > we did for the EFBIG test for future FSes that might not logically > address the Merkle tree in the past-EOF space. The reason that an error is expected for corruption past EOF in the block containing EOF, is to ensure that the bad data isn't visible via mmap reads. If that's not a problem for btrfs due to it always zeroing the part past EOF, I think it would be fine to change that test case (specifically the one with parameters "corruption_test 130999 131000 72") to just try a mmap read of the whole EOF block, and check that it either fails with an error *or* returns zeroes. I don't think that an entirely new test script would be warranted. - Eric
diff --git a/common/btrfs b/common/btrfs index 670d9d1f..c3a7dc6e 100644 --- a/common/btrfs +++ b/common/btrfs @@ -511,3 +511,8 @@ _btrfs_metadump() $BTRFS_IMAGE_PROG "$device" "$dumpfile" [ -n "$DUMP_COMPRESSOR" ] && $DUMP_COMPRESSOR -f "$dumpfile" &> /dev/null } + +_require_btrfs_corrupt_block() +{ + _require_command "$BTRFS_CORRUPT_BLOCK_PROG" btrfs-corrupt-block +} diff --git a/common/config b/common/config index 479e50d1..67bdf912 100644 --- a/common/config +++ b/common/config @@ -296,6 +296,7 @@ export BTRFS_UTIL_PROG=$(type -P btrfs) export BTRFS_SHOW_SUPER_PROG=$(type -P btrfs-show-super) export BTRFS_CONVERT_PROG=$(type -P btrfs-convert) export BTRFS_TUNE_PROG=$(type -P btrfstune) +export BTRFS_CORRUPT_BLOCK_PROG=$(type -P btrfs-corrupt-block) export XFS_FSR_PROG=$(type -P xfs_fsr) export MKFS_NFS_PROG="false" export MKFS_CIFS_PROG="false" diff --git a/common/verity b/common/verity index d58cad90..c6a47013 100644 --- a/common/verity +++ b/common/verity @@ -3,6 +3,8 @@ # # Functions for setting up and testing fs-verity +. common/btrfs + _require_scratch_verity() { _require_scratch @@ -145,6 +147,9 @@ _require_fsverity_dump_metadata() _require_fsverity_corruption() { _require_xfs_io_command "fiemap" + if [ $FSTYP == "btrfs" ]; then + _require_btrfs_corrupt_block + fi } _scratch_mkfs_verity() @@ -153,6 +158,9 @@ _scratch_mkfs_verity() ext4|f2fs) _scratch_mkfs -O verity ;; + btrfs) + _scratch_mkfs + ;; *) _notrun "No verity support for $FSTYP" ;; @@ -314,6 +322,21 @@ _fsv_scratch_corrupt_merkle_tree() (( offset += ($(_get_filesize $file) + 65535) & ~65535 )) _fsv_scratch_corrupt_bytes $file $offset ;; + btrfs) + local ino=$(stat -c '%i' $file) + _scratch_unmount + local byte="" + while read -n 1 byte; do + local ascii=$(printf "%d" "'$byte'") + # This command will find a Merkle tree item for the inode (-I $ino,37,0) + # in the default filesystem tree (-r 5) and corrupt one byte (-b 1) at + # $offset (-o $offset) with the ascii representation of the byte we read + # (-v $ascii) + $BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,37,0 -v $ascii -o $offset -b 1 $SCRATCH_DEV + (( offset += 1 )) + done + _scratch_mount + ;; *) _fail "_fsv_scratch_corrupt_merkle_tree() unimplemented on $FSTYP" ;;
generic/572-579 have tests for fsverity. Now that btrfs supports fsverity, make these tests function as well. For a majority of the tests that pass, simply adding the case to mkfs a btrfs filesystem with no extra options is sufficient. However, generic/574 has tests for corrupting the merkle tree itself. Since btrfs uses a different scheme from ext4 and f2fs for storing this data, the existing logic for corrupting it doesn't work out of the box. Adapt it to properly corrupt btrfs merkle items. 576 does not run because btrfs does not support transparent encryption. This test relies on the btrfs implementation of fsverity in the patch: btrfs: initial fsverity support and on btrfs-corrupt-block for corruption in the patches titled: btrfs-progs: corrupt generic item data with btrfs-corrupt-block btrfs-progs: expand corrupt_file_extent in btrfs-corrupt-block Signed-off-by: Boris Burkov <boris@bur.io> --- common/btrfs | 5 +++++ common/config | 1 + common/verity | 23 +++++++++++++++++++++++ 3 files changed, 29 insertions(+)