[PATCH v2 0/3] tests for btrfs fsverity

[PATCH v2 0/3] tests for btrfs fsverity

[Boris Burkov]

This patchset provides tests for fsverity support in btrfs.

It includes modifications for generic tests to pass with btrfs as well
as new btrfs specific tests.

Boris Burkov (3):
  btrfs: test btrfs specific fsverity corruption
  generic/574: corrupt btrfs merkle tree data
  btrfs: test verity orphans with dmlogwrites

 common/config       |   1 +
 common/verity       |  22 ++++-
 tests/btrfs/290     | 190 ++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/290.out |  17 ++++
 tests/btrfs/291     | 156 ++++++++++++++++++++++++++++++++++++
 tests/btrfs/291.out |   2 +
 tests/btrfs/group   |   2 +
 7 files changed, 388 insertions(+), 2 deletions(-)
 create mode 100755 tests/btrfs/290
 create mode 100644 tests/btrfs/290.out
 create mode 100755 tests/btrfs/291
 create mode 100644 tests/btrfs/291.out

-- 
2.30.2

[PATCH v2 1/3] btrfs: test btrfs specific fsverity corruption

[Boris Burkov]

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:
btrfs: add compat_flags to btrfs_inode_item
btrfs: initial fsverity support
btrfs: check verity for reads of inline extents and holes
btrfs: fallback to buffered io for verity files

and it relies on btrfs-corrupt-block for corruption, with the
following btrfs-progs patches:
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/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 d83dfb28..80d5ab66 100644
--- a/common/config
+++ b/common/config
@@ -254,6 +254,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 a8d3de06..c0b0c55d 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)
@@ -91,6 +95,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 00000000..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>/dev/null
+	_fsv_enable $f
+	$XFS_IO_PROG -c sync $SCRATCH_MNT
+	_scratch_unmount
+	# change disk_bytenr to 0, representing a hole
+	$BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 4096 -f disk_bytenr -v 0 $SCRATCH_DEV > /dev/null
+	_scratch_mount
+	validate $f
+}
+
+# plug hole
+corrupt_plug_hole() {
+	f=$SCRATCH_MNT/plug
+	head -c 12k /dev/zero | tr '\0' X > $f
+	ino=$(get_ino $f)
+	fallocate -p -o 4k -l 4k $f
+	_fsv_enable $f
+	$XFS_IO_PROG -c sync $SCRATCH_MNT
+	_scratch_unmount
+	# change disk_bytenr to some value, plugging the hole
+	$BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 4096 -f disk_bytenr -v 13639680 $SCRATCH_DEV > /dev/null
+	_scratch_mount
+	validate $f
+}
+
+# corrupt the fsverity descriptor item indiscriminately (causes EINVAL)
+corrupt_verity_descriptor() {
+	f=$SCRATCH_MNT/desc
+	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
+	# key for the descriptor item is <inode, BTRFS_VERITY_DESC_ITEM_KEY, 1>,
+	# 88 is X. So we write 5 Xs to the start of the descriptor
+	$BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,36,1 -v 88 -o 0 -b 5 $SCRATCH_DEV > /dev/null
+	_scratch_mount
+	validate $f
+}
+
+# specifically target the root hash in the descriptor (causes EIO)
+corrupt_root_hash() {
+	f=$SCRATCH_MNT/roothash
+	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
+	$BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,36,1 -v 88 -o 16 -b 1 $SCRATCH_DEV >> $seqres.full
+	#$BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,36,0 -v 88 -o 120 -b 5 $SCRATCH_DEV > /dev/null
+	_scratch_mount
+	validate $f
+}
+
+# corrupt the Merkle tree data itself
+corrupt_merkle_tree() {
+	f=$SCRATCH_MNT/merkle
+	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
+	# key for the descriptor item is <inode, BTRFS_VERITY_MERKLE_ITEM_KEY, 0>,
+	# 88 is X. So we write 5 Xs to somewhere in the middle of the first
+	# merkle item
+	$BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,37,0 -v 88 -o 100 -b 5 $SCRATCH_DEV > /dev/null
+	_scratch_mount
+	validate $f
+}
+
+# real QA test starts here
+_scratch_mkfs >/dev/null
+_scratch_mount
+
+corrupt_inline
+corrupt_prealloc_to_reg
+corrupt_reg_to_prealloc
+corrupt_punch_hole
+corrupt_plug_hole
+corrupt_verity_descriptor
+corrupt_root_hash
+corrupt_merkle_tree
+
+# we intentionally corrupted, re-mkfs to avoid tripping the corrupted fs error
+_scratch_unmount
+_scratch_mkfs >/dev/null
+
+status=0
+exit
diff --git a/tests/btrfs/290.out b/tests/btrfs/290.out
new file mode 100644
index 00000000..4da61246
--- /dev/null
+++ b/tests/btrfs/290.out
@@ -0,0 +1,17 @@
+QA output created by 290
+cat: /mnt/scratch/inl: Input/output error
+dd: error reading '/mnt/scratch/inl': Input/output error
+cat: /mnt/scratch/prealloc: Input/output error
+dd: error reading '/mnt/scratch/prealloc': Input/output error
+cat: /mnt/scratch/reg: Input/output error
+dd: error reading '/mnt/scratch/reg': Input/output error
+cat: /mnt/scratch/punch: Input/output error
+dd: error reading '/mnt/scratch/punch': Input/output error
+cat: /mnt/scratch/plug: Input/output error
+dd: error reading '/mnt/scratch/plug': Input/output error
+cat: /mnt/scratch/desc: Invalid argument
+dd: failed to open '/mnt/scratch/desc': Invalid argument
+cat: /mnt/scratch/roothash: Input/output error
+dd: error reading '/mnt/scratch/roothash': Input/output error
+cat: /mnt/scratch/merkle: Input/output error
+dd: error reading '/mnt/scratch/merkle': Input/output error
diff --git a/tests/btrfs/group b/tests/btrfs/group
index a7c65983..58943c85 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -233,3 +233,4 @@
 228 auto quick volume
 229 auto quick send clone
 230 auto quick qgroup limit
+290 auto quick verity
-- 
2.30.2

[PATCH v2 2/3] generic/574: corrupt btrfs merkle tree data

[Boris Burkov]

generic/574 has tests for corrupting the merkle tree data stored by the
filesystem. Since btrfs uses a different scheme 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.

Note that there is a bit of a kludge here: since btrfs_corrupt_block
doesn't handle streaming corruption bytes from stdin (I could change
that, but it feels like overkill for this purpose), I just read the
first corruption byte and duplicate it for the desired length. That is
how the test is using the interface in practice, anyway.

This test relies on the btrfs implementation of fsverity in:
btrfs: add compat_flags to btrfs_inode_item
btrfs: initial fsverity support
btrfs: check verity for reads of inline extents and holes
btrfs: fallback to buffered io for verity files

It also relies on the btrfs fiemap fix in:
btrfs: return whole extents in fiemap

and it relies on btrfs-corrupt-block for corruption, with the
following btrfs-progs patches:
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/verity | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/common/verity b/common/verity
index c0b0c55d..333526ac 100644
--- a/common/verity
+++ b/common/verity
@@ -3,8 +3,7 @@
 #
 # Functions for setting up and testing fs-verity
 
-_require_scratch_verity()
-{
+_require_scratch_verity() {
 	_require_scratch
 	_require_command "$FSVERITY_PROG" fsverity
 
@@ -244,6 +243,18 @@ _fsv_scratch_corrupt_merkle_tree()
 		(( offset += ($(_get_filesize $file) + 65535) & ~65535 ))
 		_fsv_scratch_corrupt_bytes $file $offset
 		;;
+	btrfs)
+		ino=$(ls -i $file | awk '{print $1}')
+		sync
+		cat > $tmp.bytes
+		sz=$(_get_filesize $tmp.bytes)
+		read -n 1 byte < $tmp.bytes
+		ascii=$(printf "%d" "'$byte'")
+		_scratch_unmount
+		$BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,37,0 -v $ascii -o $offset -b $sz $SCRATCH_DEV
+		sync
+		_scratch_mount
+		;;
 	*)
 		_fail "_fsv_scratch_corrupt_merkle_tree() unimplemented on $FSTYP"
 		;;
-- 
2.30.2

[PATCH v2 3/3] btrfs: test verity orphans with dmlogwrites

[Boris Burkov]

The behavior of orphans is most interesting across mounts, interrupted
at arbitrary points during fsverity enable. To cover as many such cases
as possible, use dmlogwrites and dmsnapshot as in
log-writes/replay-individual.sh. At each log entry, we want to assert a
somewhat complicated invariant:

If verity has not yet started: an orphan indicates that verity has
started.
If verity has started: mount should handle the orphan and blow away
verity data: expect 0 merkle items after mounting the snapshot dev. If
we can measure the file, verity has finished.
If verity has finished: the orphan should be gone, so mount should not
blow away merkle items. Expect the same number of merkle items before
and after mounting the snapshot dev.

Note that this relies on grepping btrfs inspect-internal dump-tree.
Until btrfs-progs has the ability to print the new Merkle items, they
will show up as UNKNOWN.36/37.

This test relies on the btrfs implementation of fsverity in:
btrfs: add compat_flags to btrfs_inode_item
btrfs: initial fsverity support
btrfs: check verity for reads of inline extents and holes
btrfs: fallback to buffered io for verity files
btrfs: verity metadata orphan items

and it relies on btrfs-corrupt-block for corruption, with the
following btrfs-progs patches:
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>
---
 tests/btrfs/291     | 156 ++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/291.out |   2 +
 tests/btrfs/group   |   1 +
 3 files changed, 159 insertions(+)
 create mode 100755 tests/btrfs/291
 create mode 100644 tests/btrfs/291.out

diff --git a/tests/btrfs/291 b/tests/btrfs/291
new file mode 100755
index 00000000..61f36426
--- /dev/null
+++ b/tests/btrfs/291
@@ -0,0 +1,156 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2021 Facebook, Inc. All Rights Reserved.
+#
+# FS QA Test 291
+#
+# Test btrfs consistency after each FUA while enabling verity on a file
+# This test works by following the pattern in log-writes/replay-individual.sh:
+# 1. run a workload (verity + sync) while logging to the log device
+# 2. replay an entry to the replay device
+# 3. snapshot the replay device to the snapshot device
+# 4. run destructive tests on the snapshot device (e.g. mount with orphans)
+# 5. goto 2
+#
+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
+
+_cleanup()
+{
+	cd /
+	_log_writes_cleanup &> /dev/null
+	rm -f $tmp.*
+	$LVM_PROG vgremove -f -y $vgname >>$seqres.full 2>&1
+	losetup -d $loop_dev >>$seqres.full 2>&1
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+. ./common/dmlogwrites
+. ./common/verity
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+_supported_fs btrfs
+
+_require_scratch
+_require_log_writes
+_require_dm_target snapshot
+_require_command $LVM_PROG lvm
+_require_scratch_verity
+
+sync_loop() {
+	i=$1
+	[ -z "$i" ] && _fail "sync loop needs a number of iterations"
+	while [ $i -gt 0 ]
+	do
+		$XFS_IO_PROG -c sync $SCRATCH_MNT
+		let i-=1
+	done
+}
+
+dump_tree() {
+	dev=$1
+	$BTRFS_UTIL_PROG inspect-internal dump-tree $dev
+}
+
+count_item() {
+	dev=$1
+	item=$2
+	dump_tree $dev | grep -c $item
+}
+
+_log_writes_init $SCRATCH_DEV
+_log_writes_mkfs
+_log_writes_mount
+
+f=$SCRATCH_MNT/fsv
+dd if=/dev/zero of=$f bs=1M count=10 >>$seqres.full 2>&1
+$XFS_IO_PROG -c sync $SCRATCH_MNT
+sync_loop 10 &
+_fsv_enable $f
+$XFS_IO_PROG -c sync $SCRATCH_MNT
+
+_log_writes_unmount
+_log_writes_remove
+
+dd if=/dev/zero of=$tmp.loop-file bs=1M count=1 seek=8192 >>$seqres.full 2>&1
+loop_dev=$(losetup -f --show $tmp.loop-file)
+vgname=vg_replay
+lvname=lv_replay
+replay_dev=/dev/mapper/vg_replay-lv_replay
+snapname=lv_snap
+snap_dev=/dev/mapper/vg_replay-$snapname
+
+$LVM_PROG vgcreate -f $vgname $loop_dev >>$seqres.full 2>&1 || _fail "failed to vgcreate $vgname"
+$LVM_PROG lvcreate -L 4G -n $lvname $vgname -y >>$seqres.full 2>&1 || \
+	_fail "failed to lvcreate $lvname"
+$UDEV_SETTLE_PROG >>$seqres.full 2>&1
+
+replay_log_prog=$here/src/log-writes/replay-log
+num_entries=$($replay_log_prog --log $LOGWRITES_DEV --num-entries)
+entry=$($replay_log_prog --log $LOGWRITES_DEV --replay $replay_dev --find --end-mark mkfs | cut -d@ -f1)
+$replay_log_prog --log $LOGWRITES_DEV --replay $replay_dev --limit $entry || \
+	_fail "failed to replay to start entry $entry"
+let entry+=1
+
+# state = 0: verity hasn't started
+# state = 1: verity underway
+# state = 2: verity done
+state=0
+while [ $entry -lt $num_entries ];
+do
+	$replay_log_prog --limit 1 --log $LOGWRITES_DEV --replay $replay_dev --start $entry || \
+		_fail "failed to take replay step at entry: $entry"
+
+	$LVM_PROG lvcreate -s -L 4M -n $snapname $vgname/$lvname >>$seqres.full 2>&1 || \
+		_fail "Failed to create snapshot"
+	$UDEV_SETTLE_PROG >>$seqres.full 2>&1
+
+	orphan=$(count_item $snap_dev ORPHAN)
+	if [ $state -eq 0 ]; then
+		[ $orphan -gt 0 ] && state=1
+	fi
+
+	pre_mount=$(count_item $snap_dev UNKNOWN.3[67])
+	_mount $snap_dev $SCRATCH_MNT || _fail "mount failed at entry $entry"
+	fsverity measure $SCRATCH_MNT/fsv >>$seqres.full 2>&1
+	measured=$?
+	umount $SCRATCH_MNT
+	[ $state -eq 1 ] && [ $measured -eq 0 ] && state=2
+	[ $state -eq 2 ] && ([ $measured -eq 0 ] || _fail "verity done, but measurement failed at entry $entry")
+	post_mount=$(count_item $snap_dev UNKNOWN.3[67])
+
+	echo "entry: $entry, state: $state, orphan: $orphan, pre_mount: $pre_mount, post_mount: $post_mount" >> $seqres.full
+
+	if [ $state -eq 1 ]; then
+		[ $post_mount -eq 0 ] || \
+			_fail "mount failed to clear under-construction merkle items pre: $pre_mount, post: $post_mount at entry $entry";
+	fi
+	if [ $state -eq 2 ]; then
+		[ $pre_mount -gt 0 ] || \
+			_fail "expected to have verity items before mount at entry $entry"
+		[ $pre_mount -eq $post_mount ] || \
+			_fail "mount cleared merkle items after verity was enabled $pre_mount vs $post_mount at entry $entry";
+	fi
+
+	let entry+=1
+	$LVM_PROG lvremove $vgname/$snapname -y >>$seqres.full
+done
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/291.out b/tests/btrfs/291.out
new file mode 100644
index 00000000..04605c70
--- /dev/null
+++ b/tests/btrfs/291.out
@@ -0,0 +1,2 @@
+QA output created by 291
+Silence is golden
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 58943c85..72e3ec38 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -234,3 +234,4 @@
 229 auto quick send clone
 230 auto quick qgroup limit
 290 auto quick verity
+291 auto verity
-- 
2.30.2

Re: [PATCH v2 0/3] tests for btrfs fsverity

[Eric Biggers]

On Thu, Apr 08, 2021 at 11:30:10AM -0700, Boris Burkov wrote:
> This patchset provides tests for fsverity support in btrfs.
> 
> It includes modifications for generic tests to pass with btrfs as well
> as new btrfs specific tests.

Which commit does this apply to?  It doesn't apply to the latest xfstests master
branch.

- Eric

Re: [PATCH v2 2/3] generic/574: corrupt btrfs merkle tree data

[Eric Biggers]

On Thu, Apr 08, 2021 at 11:30:12AM -0700, Boris Burkov wrote:
> 
> Note that there is a bit of a kludge here: since btrfs_corrupt_block
> doesn't handle streaming corruption bytes from stdin (I could change
> that, but it feels like overkill for this purpose), I just read the
> first corruption byte and duplicate it for the desired length. That is
> how the test is using the interface in practice, anyway.

If that's the problem, couldn't you just write the data to a temporary file?

- Eric

Re: [PATCH v2 0/3] tests for btrfs fsverity

[Boris Burkov]

On Thu, Apr 08, 2021 at 11:36:30AM -0700, Eric Biggers wrote:
> On Thu, Apr 08, 2021 at 11:30:10AM -0700, Boris Burkov wrote:
> > This patchset provides tests for fsverity support in btrfs.
> > 
> > It includes modifications for generic tests to pass with btrfs as well
> > as new btrfs specific tests.
> 
> Which commit does this apply to?  It doesn't apply to the latest xfstests master
> branch.
> 

Oh, I'm quite sorry about that.
This is written on top of the btrfs group's xfstests staging branch.
for reference, that's:
901c7750a6add
from https://github.com/btrfs/fstests/tree/staging

I'll rebase on the xfstests master branch and re-send it.

> - Eric

Re: [PATCH v2 2/3] generic/574: corrupt btrfs merkle tree data

[Boris Burkov]

On Thu, Apr 08, 2021 at 11:41:42AM -0700, Eric Biggers wrote:
> On Thu, Apr 08, 2021 at 11:30:12AM -0700, Boris Burkov wrote:
> > 
> > Note that there is a bit of a kludge here: since btrfs_corrupt_block
> > doesn't handle streaming corruption bytes from stdin (I could change
> > that, but it feels like overkill for this purpose), I just read the
> > first corruption byte and duplicate it for the desired length. That is
> > how the test is using the interface in practice, anyway.
> 
> If that's the problem, couldn't you just write the data to a temporary file?

Sorry, I was a bit too vague. It doesn't have a file or stdin interface,
as far as I know.

btrfs-corrupt-block has your typical "kitchen sink of flags" interface and
doesn't currently read input from streams/files. I extended that
interface in the simplest way to support arbitrary corruption, which
didn't fit with the stream based corruption this test does.

my options seem to be:
shoehorn the "byte, length" interface into this test or
shoehorn the "stream corruption input in" interface into
btrfs-corrupt-block.

I have no problem with either, the former was just less work because I
already wrote it that way. If the junk I did here is a deal-breaker, I
don't mind modifying btrfs-corrupt-block.

> 
> - Eric

Thanks for the quick review,
Boris

Re: [PATCH v2 2/3] generic/574: corrupt btrfs merkle tree data

[Eric Biggers]

On Thu, Apr 08, 2021 at 11:49:37AM -0700, Boris Burkov wrote:
> On Thu, Apr 08, 2021 at 11:41:42AM -0700, Eric Biggers wrote:
> > On Thu, Apr 08, 2021 at 11:30:12AM -0700, Boris Burkov wrote:
> > > 
> > > Note that there is a bit of a kludge here: since btrfs_corrupt_block
> > > doesn't handle streaming corruption bytes from stdin (I could change
> > > that, but it feels like overkill for this purpose), I just read the
> > > first corruption byte and duplicate it for the desired length. That is
> > > how the test is using the interface in practice, anyway.
> > 
> > If that's the problem, couldn't you just write the data to a temporary file?
> 
> Sorry, I was a bit too vague. It doesn't have a file or stdin interface,
> as far as I know.
> 
> btrfs-corrupt-block has your typical "kitchen sink of flags" interface and
> doesn't currently read input from streams/files. I extended that
> interface in the simplest way to support arbitrary corruption, which
> didn't fit with the stream based corruption this test does.
> 
> my options seem to be:
> shoehorn the "byte, length" interface into this test or
> shoehorn the "stream corruption input in" interface into
> btrfs-corrupt-block.
> 
> I have no problem with either, the former was just less work because I
> already wrote it that way. If the junk I did here is a deal-breaker, I
> don't mind modifying btrfs-corrupt-block.
> 

If it's a lot of trouble to handle arbitrary data, then I think you should
change _fsv_scratch_corrupt_merkle_tree() to actually take a (byte, length) pair
instead of data on stdin.  Otherwise, _fsv_scratch_corrupt_merkle_tree() would
claim to do one thing but actually would do a different thing on one specific
filesystem.

- Eric

Re: [PATCH v2 2/3] generic/574: corrupt btrfs merkle tree data

[Boris Burkov]

On Thu, Apr 08, 2021 at 04:16:28PM -0700, Eric Biggers wrote:
> On Thu, Apr 08, 2021 at 11:49:37AM -0700, Boris Burkov wrote:
> > On Thu, Apr 08, 2021 at 11:41:42AM -0700, Eric Biggers wrote:
> > > On Thu, Apr 08, 2021 at 11:30:12AM -0700, Boris Burkov wrote:
> > > > 
> > > > Note that there is a bit of a kludge here: since btrfs_corrupt_block
> > > > doesn't handle streaming corruption bytes from stdin (I could change
> > > > that, but it feels like overkill for this purpose), I just read the
> > > > first corruption byte and duplicate it for the desired length. That is
> > > > how the test is using the interface in practice, anyway.
> > > 
> > > If that's the problem, couldn't you just write the data to a temporary file?
> > 
> > Sorry, I was a bit too vague. It doesn't have a file or stdin interface,
> > as far as I know.
> > 
> > btrfs-corrupt-block has your typical "kitchen sink of flags" interface and
> > doesn't currently read input from streams/files. I extended that
> > interface in the simplest way to support arbitrary corruption, which
> > didn't fit with the stream based corruption this test does.
> > 
> > my options seem to be:
> > shoehorn the "byte, length" interface into this test or
> > shoehorn the "stream corruption input in" interface into
> > btrfs-corrupt-block.
> > 
> > I have no problem with either, the former was just less work because I
> > already wrote it that way. If the junk I did here is a deal-breaker, I
> > don't mind modifying btrfs-corrupt-block.
> > 
> 
> If it's a lot of trouble to handle arbitrary data, then I think you should
> change _fsv_scratch_corrupt_merkle_tree() to actually take a (byte, length) pair
> instead of data on stdin.  Otherwise, _fsv_scratch_corrupt_merkle_tree() would
> claim to do one thing but actually would do a different thing on one specific
> filesystem.
> 
> - Eric

It's no trouble. I think reading in corruption bytes will be a better
interface for btrfs-corrupt-block so I'll go ahead and do it.