Re: [PATCH v3 1/3] btrfs: test btrfs specific fsverity corruption
On Thu, Apr 08, 2021 at 11:57:49AM -0700, Boris Burkov wrote:
> There are some btrfs specific fsverity scenarios that don't map
> neatly onto the tests in generic/574 like holes, inline extents,
> and preallocated extents. Cover those in a btrfs specific test.
>
> This test relies on the btrfs implementation of fsverity in:
>
> and it relies on btrfs-corrupt-block for corruption, with the patches:
>
I assume you'll fill in the patch lists when they're merged.
>
> Signed-off-by: Boris Burkov
> ---
> common/config | 1 +
> common/verity | 7 ++
> tests/btrfs/290 | 190
> tests/btrfs/290.out | 17
> tests/btrfs/group | 1 +
> 5 files changed, 216 insertions(+)
> create mode 100755 tests/btrfs/290
> create mode 100644 tests/btrfs/290.out
>
> diff --git a/common/config b/common/config
> index a47e462c..003b2a88 100644
> --- a/common/config
> +++ b/common/config
> @@ -256,6 +256,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 38eea157..d2c1ea24 100644
> --- a/common/verity
> +++ b/common/verity
> @@ -8,6 +8,10 @@ _require_scratch_verity()
> _require_scratch
> _require_command "$FSVERITY_PROG" fsverity
>
> + if [ $FSTYP == "btrfs" ]; then
> + _require_command "$BTRFS_CORRUPT_BLOCK_PROG" btrfs_corrupt_block
> + fi
Does the fsverity function depend on btrfs-corrupt-block command? I
think btrfs-corrupt-block is only needed in this specific test, other
btrfs fsverity tests may not depend on it. So add this require rule in
the test if that's the case.
> +
> if ! _scratch_mkfs_verity &>>$seqres.full; then
> # ext4: need e2fsprogs v1.44.5 or later (but actually v1.45.2+
> # is needed for some tests to pass, due to an e2fsck bug)
> @@ -147,6 +151,9 @@ _scratch_mkfs_verity()
> ext4|f2fs)
> _scratch_mkfs -O verity
> ;;
> + btrfs)
> + _scratch_mkfs
> + ;;
> *)
> _notrun "No verity support for $FSTYP"
> ;;
> diff --git a/tests/btrfs/290 b/tests/btrfs/290
> new file mode 100755
> index ..5aff7648
> --- /dev/null
> +++ b/tests/btrfs/290
> @@ -0,0 +1,190 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2021 Facebook, Inc. All Rights Reserved.
> +#
> +# FS QA Test 290
> +#
> +# Test btrfs support for fsverity.
> +# This test extends the generic fsverity testing by corrupting inline
> extents,
> +# preallocated extents, holes, and the Merkle descriptor in a btrfs-aware
> way.
> +#
> +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 15
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/verity
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +_supported_fs btrfs
> +_require_scratch
> +_require_scratch_verity
> +
> +cleanup()
Better to name it as _cleanup(), though usually we name local functions
without the leading underscore, but _cleanup function is a bit special,
almost all tests name it with "_" and it's in template in 'check'
script. And it's easier to do a global s/_cleanup/cleanup/ if it's
consistent with other tests.
> +{
> + cd /
> + rm -f $tmp.*
> +}
> +
> +get_ino() {
> + file=$1
Declare local variables with local keyword.
> + ls -i $file | awk '{print $1}'
stat -c "%i"
> +}
> +
> +validate() {
> + f=$1
> + sz=$2
sz is not used, but xfs_io below would use $sz
> + # buffered io
> + cat $f > /dev/null
> + # direct io
> + dd if=$f iflag=direct of=/dev/null status=none
$XFS_IO_PROG -dc "pread -q 0 $sz" $f
Direct I/O is used, then we need
_require_odirect
> +}
> +
> +# corrupt the data portion of an inline extent
> +corrupt_inline() {
> + f=$SCRATCH_MNT/inl
> + head -c 42 /dev/zero | tr '\0' X > $f
$XFS_IO_PROG -fc "pwrite -q -S 0x58 0 42" $f
And all usages in other functions.
> + ino=$(get_ino $f)
> + _fsv_enable $f
> + $XFS_IO_PROG -c sync $SCRATCH_MNT
> + _scratch_unmount
> + # inline data starts at disk_bytenr
> + # overwrite the first u64 with random bogus junk
> + $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f disk_bytenr $SCRATCH_DEV >
> /dev/null
> + _scratch_mount
> + validate $f
Then all validte calls should pass file size as second param.
> +}
> +
> +# preallocate a file, then corrupt it by changing it to a regular file
> +co
Re: [PATCH v3 1/3] btrfs: test btrfs specific fsverity corruption
On Thu, Apr 08, 2021 at 11:57:49AM -0700, Boris Burkov wrote:
> +get_ino() {
> + file=$1
> + ls -i $file | awk '{print $1}'
> +}
Please use 'local' when declaring variables in shell functions.
> +# corrupt the data portion of an inline extent
> +corrupt_inline() {
> + f=$SCRATCH_MNT/inl
> + head -c 42 /dev/zero | tr '\0' X > $f
> + ino=$(get_ino $f)
> + _fsv_enable $f
> + $XFS_IO_PROG -c sync $SCRATCH_MNT
> + _scratch_unmount
Isn't the sync unnecessary because the filesystem is unmounted anyway?
Likewise for all the other corrupt_* functions.
Otherwise, at a high level this test looks good -- thanks for writing it!
- Eric
[PATCH v3 1/3] btrfs: test btrfs specific fsverity corruption
There are some btrfs specific fsverity scenarios that don't map
neatly onto the tests in generic/574 like holes, inline extents,
and preallocated extents. Cover those in a btrfs specific test.
This test relies on the btrfs implementation of fsverity in:
and it relies on btrfs-corrupt-block for corruption, with the patches:
Signed-off-by: Boris Burkov
---
common/config | 1 +
common/verity | 7 ++
tests/btrfs/290 | 190
tests/btrfs/290.out | 17
tests/btrfs/group | 1 +
5 files changed, 216 insertions(+)
create mode 100755 tests/btrfs/290
create mode 100644 tests/btrfs/290.out
diff --git a/common/config b/common/config
index a47e462c..003b2a88 100644
--- a/common/config
+++ b/common/config
@@ -256,6 +256,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 38eea157..d2c1ea24 100644
--- a/common/verity
+++ b/common/verity
@@ -8,6 +8,10 @@ _require_scratch_verity()
_require_scratch
_require_command "$FSVERITY_PROG" fsverity
+ if [ $FSTYP == "btrfs" ]; then
+ _require_command "$BTRFS_CORRUPT_BLOCK_PROG" btrfs_corrupt_block
+ fi
+
if ! _scratch_mkfs_verity &>>$seqres.full; then
# ext4: need e2fsprogs v1.44.5 or later (but actually v1.45.2+
# is needed for some tests to pass, due to an e2fsck bug)
@@ -147,6 +151,9 @@ _scratch_mkfs_verity()
ext4|f2fs)
_scratch_mkfs -O verity
;;
+ btrfs)
+ _scratch_mkfs
+ ;;
*)
_notrun "No verity support for $FSTYP"
;;
diff --git a/tests/btrfs/290 b/tests/btrfs/290
new file mode 100755
index ..5aff7648
--- /dev/null
+++ b/tests/btrfs/290
@@ -0,0 +1,190 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2021 Facebook, Inc. All Rights Reserved.
+#
+# FS QA Test 290
+#
+# Test btrfs support for fsverity.
+# This test extends the generic fsverity testing by corrupting inline extents,
+# preallocated extents, holes, and the Merkle descriptor in a btrfs-aware way.
+#
+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 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/verity
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+_supported_fs btrfs
+_require_scratch
+_require_scratch_verity
+
+cleanup()
+{
+ cd /
+ rm -f $tmp.*
+}
+
+get_ino() {
+ file=$1
+ ls -i $file | awk '{print $1}'
+}
+
+validate() {
+ f=$1
+ sz=$2
+ # buffered io
+ cat $f > /dev/null
+ # direct io
+ dd if=$f iflag=direct of=/dev/null status=none
+}
+
+# corrupt the data portion of an inline extent
+corrupt_inline() {
+ f=$SCRATCH_MNT/inl
+ head -c 42 /dev/zero | tr '\0' X > $f
+ ino=$(get_ino $f)
+ _fsv_enable $f
+ $XFS_IO_PROG -c sync $SCRATCH_MNT
+ _scratch_unmount
+ # inline data starts at disk_bytenr
+ # overwrite the first u64 with random bogus junk
+ $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f disk_bytenr $SCRATCH_DEV >
/dev/null
+ _scratch_mount
+ validate $f
+}
+
+# preallocate a file, then corrupt it by changing it to a regular file
+corrupt_prealloc_to_reg() {
+ f=$SCRATCH_MNT/prealloc
+ fallocate -l 4k $f
+ ino=$(get_ino $f)
+ _fsv_enable $f
+ $XFS_IO_PROG -c sync $SCRATCH_MNT
+ _scratch_unmount
+ # set extent type from prealloc (2) to reg (1)
+ $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f type -v 1 $SCRATCH_DEV
2>/dev/null >/dev/null
+ _scratch_mount
+ validate $f
+}
+
+# corrupt a regular file by changing the type to preallocated
+corrupt_reg_to_prealloc() {
+ f=$SCRATCH_MNT/reg
+ head -c 12k /dev/zero | tr '\0' X > $f
+ ino=$(get_ino $f)
+ _fsv_enable $f
+ $XFS_IO_PROG -c sync $SCRATCH_MNT
+ _scratch_unmount
+ # set type from reg (1) to prealloc (2)
+ $BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f type -v 2 $SCRATCH_DEV
2>/dev/null >/dev/null
+ _scratch_mount
+ validate $f
+}
+
+# corrupt a file by punching a hole
+corrupt_punch_hole() {
+ f=$SCRATCH_MNT/punch
+ head -c 12k /dev/zero | tr '\0' X > $f
+ ino=$(get_ino $f)
+ # make a new extent in the middle
+ $XFS_IO_PROG -c sync $SCRATCH_MNT
+ head -c 4k /dev/zero | tr '\0' Y | dd of=$f bs=4k count=1 seek=1
conv=notrunc 2>/d
