diff mbox series

[v7,2/5] btrfs: test btrfs specific fsverity corruption

Message ID a860be32471a885292c2f6f3136cac40bebdbf05.1647382272.git.boris@bur.io (mailing list archive)
State Superseded
Headers show
Series tests for btrfs fsverity | expand

Commit Message

Boris Burkov March 15, 2022, 10:15 p.m. UTC 
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 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       |  14 ++++
 tests/btrfs/290     | 168 ++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/290.out |  25 +++++++
 5 files changed, 213 insertions(+)
 create mode 100755 tests/btrfs/290
 create mode 100644 tests/btrfs/290.out

Comments

Eric Biggers March 16, 2022, 5:26 p.m. UTC | #1 
On Tue, Mar 15, 2022 at 03:15:58PM -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 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       |  14 ++++
>  tests/btrfs/290     | 168 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/290.out |  25 +++++++
>  5 files changed, 213 insertions(+)
>  create mode 100755 tests/btrfs/290
>  create mode 100644 tests/btrfs/290.out
> 
> 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 1afe4a82..77766fca 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
> @@ -48,6 +50,15 @@ _require_scratch_verity()
>  	FSV_BLOCK_SIZE=$(get_page_size)
>  }
>  
> +# Check for userspace tools needed to corrupt verity data or metadata.
> +_require_fsverity_corruption()
> +{
> +	_require_xfs_io_command "fiemap"
> +	if [ $FSTYP == "btrfs" ]; then
> +		_require_btrfs_corrupt_block
> +	fi
> +}

This is adding a second definition of _require_fsverity_corruption().
Probably a rebase error.

Also, is this hunk in the right patch?  This patch is for adding btrfs/290;
however, btrfs/290 doesn't use _require_fsverity_corruption() anymore.

> +
>  # Check for CONFIG_FS_VERITY_BUILTIN_SIGNATURES=y, as well as the userspace
>  # commands needed to generate certificates and add them to the kernel.
>  _require_fsverity_builtin_signatures()
> @@ -153,6 +164,9 @@ _scratch_mkfs_verity()
>  	ext4|f2fs)
>  		_scratch_mkfs -O verity
>  		;;
> +	btrfs)
> +		_scratch_mkfs
> +		;;

I think a good way to organize things would be to wire up the existing verity
tests for btrfs first, then to add the btrfs-specific tests at thet end of the
series.  That would mean the above hunk would go earlier in the series, not with
btrfs/290.  It's a little hard to review as-is, as the different hunks needed to
wire up the existing tests are mixed around in different patches.

- Eric
Boris Burkov March 16, 2022, 5:36 p.m. UTC | #2 
On Wed, Mar 16, 2022 at 05:26:59PM +0000, Eric Biggers wrote:
> On Tue, Mar 15, 2022 at 03:15:58PM -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 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       |  14 ++++
> >  tests/btrfs/290     | 168 ++++++++++++++++++++++++++++++++++++++++++++
> >  tests/btrfs/290.out |  25 +++++++
> >  5 files changed, 213 insertions(+)
> >  create mode 100755 tests/btrfs/290
> >  create mode 100644 tests/btrfs/290.out
> > 
> > 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 1afe4a82..77766fca 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
> > @@ -48,6 +50,15 @@ _require_scratch_verity()
> >  	FSV_BLOCK_SIZE=$(get_page_size)
> >  }
> >  
> > +# Check for userspace tools needed to corrupt verity data or metadata.
> > +_require_fsverity_corruption()
> > +{
> > +	_require_xfs_io_command "fiemap"
> > +	if [ $FSTYP == "btrfs" ]; then
> > +		_require_btrfs_corrupt_block
> > +	fi
> > +}
> 
> This is adding a second definition of _require_fsverity_corruption().
> Probably a rebase error.
> 
> Also, is this hunk in the right patch?  This patch is for adding btrfs/290;
> however, btrfs/290 doesn't use _require_fsverity_corruption() anymore.
> 
> > +
> >  # Check for CONFIG_FS_VERITY_BUILTIN_SIGNATURES=y, as well as the userspace
> >  # commands needed to generate certificates and add them to the kernel.
> >  _require_fsverity_builtin_signatures()
> > @@ -153,6 +164,9 @@ _scratch_mkfs_verity()
> >  	ext4|f2fs)
> >  		_scratch_mkfs -O verity
> >  		;;
> > +	btrfs)
> > +		_scratch_mkfs
> > +		;;
> 
> I think a good way to organize things would be to wire up the existing verity
> tests for btrfs first, then to add the btrfs-specific tests at thet end of the
> series.  That would mean the above hunk would go earlier in the series, not with
> btrfs/290.  It's a little hard to review as-is, as the different hunks needed to
> wire up the existing tests are mixed around in different patches.

I like that. I've definitely been struggling with getting all the hunks
in the right places.. I'll double check it all better before sending it
again. Thanks for the review,

Boris
> 
> - Eric
diff mbox series

Patch

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 1afe4a82..77766fca 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
@@ -48,6 +50,15 @@  _require_scratch_verity()
 	FSV_BLOCK_SIZE=$(get_page_size)
 }
 
+# Check for userspace tools needed to corrupt verity data or metadata.
+_require_fsverity_corruption()
+{
+	_require_xfs_io_command "fiemap"
+	if [ $FSTYP == "btrfs" ]; then
+		_require_btrfs_corrupt_block
+	fi
+}
+
 # Check for CONFIG_FS_VERITY_BUILTIN_SIGNATURES=y, as well as the userspace
 # commands needed to generate certificates and add them to the kernel.
 _require_fsverity_builtin_signatures()
@@ -153,6 +164,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..f9acd55a
--- /dev/null
+++ b/tests/btrfs/290
@@ -0,0 +1,168 @@ 
+#! /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.
+#
+. ./common/preamble
+_begin_fstest auto quick verity
+
+# Import common functions.
+. ./common/filter
+. ./common/verity
+
+# real QA test starts here
+_supported_fs btrfs
+_require_scratch_verity
+_require_scratch_nocheck
+_require_odirect
+_require_xfs_io_command "falloc"
+_require_xfs_io_command "pread"
+_require_xfs_io_command "pwrite"
+_require_btrfs_corrupt_block
+
+get_ino() {
+	local file=$1
+	stat -c "%i" $file
+}
+
+validate() {
+	local f=$1
+	local sz=$(_get_filesize $f)
+	# buffered io
+	echo $(basename $f)
+	$XFS_IO_PROG -rc "pread -q 0 $sz" $f 2>&1 | _filter_scratch
+	# direct io
+	$XFS_IO_PROG -rdc "pread -q 0 $sz" $f 2>&1 | _filter_scratch
+}
+
+# corrupt the data portion of an inline extent
+corrupt_inline() {
+	local f=$SCRATCH_MNT/inl
+	$XFS_IO_PROG -fc "pwrite -q -S 0x58 0 42" $f
+	local ino=$(get_ino $f)
+	_fsv_enable $f
+	_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 2>&1
+	_scratch_mount
+	validate $f
+}
+
+# preallocate a file, then corrupt it by changing it to a regular file
+corrupt_prealloc_to_reg() {
+	local f=$SCRATCH_MNT/prealloc
+	$XFS_IO_PROG -fc "falloc 0 12k" $f
+	local ino=$(get_ino $f)
+	_fsv_enable $f
+	_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 >/dev/null 2>&1
+	_scratch_mount
+	validate $f
+}
+
+# corrupt a regular file by changing the type to preallocated
+corrupt_reg_to_prealloc() {
+	local f=$SCRATCH_MNT/reg
+	$XFS_IO_PROG -fc "pwrite -q -S 0x58 0 12288" $f
+	local ino=$(get_ino $f)
+	_fsv_enable $f
+	_scratch_unmount
+	# set type from reg (1) to prealloc (2)
+	$BTRFS_CORRUPT_BLOCK_PROG -i $ino -x 0 -f type -v 2 $SCRATCH_DEV >/dev/null 2>&1
+	_scratch_mount
+	validate $f
+}
+
+# corrupt a file by punching a hole
+corrupt_punch_hole() {
+	local f=$SCRATCH_MNT/punch
+	$XFS_IO_PROG -fc "pwrite -q -S 0x58 0 12288" $f
+	local ino=$(get_ino $f)
+	# make a new extent in the middle, sync so the writes don't coalesce
+	$XFS_IO_PROG -c sync $SCRATCH_MNT
+	$XFS_IO_PROG -fc "pwrite -q -S 0x59 4096 4096" $f
+	_fsv_enable $f
+	_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 2>&1
+	_scratch_mount
+	validate $f
+}
+
+# plug hole
+corrupt_plug_hole() {
+	local f=$SCRATCH_MNT/plug
+	$XFS_IO_PROG -fc "pwrite -q -S 0x58 0 12288" $f
+	local ino=$(get_ino $f)
+	$XFS_IO_PROG -fc "falloc 4k 4k" $f
+	_fsv_enable $f
+	_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 2>&1
+	_scratch_mount
+	validate $f
+}
+
+# corrupt the fsverity descriptor item indiscriminately (causes EINVAL)
+corrupt_verity_descriptor() {
+	local f=$SCRATCH_MNT/desc
+	$XFS_IO_PROG -fc "pwrite -q -S 0x58 0 12288" $f
+	local ino=$(get_ino $f)
+	_fsv_enable $f
+	_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 2>&1
+	_scratch_mount
+	validate $f
+}
+
+# specifically target the root hash in the descriptor (causes EIO)
+corrupt_root_hash() {
+	local f=$SCRATCH_MNT/roothash
+	$XFS_IO_PROG -fc "pwrite -q -S 0x58 0 12288" $f
+	local ino=$(get_ino $f)
+	_fsv_enable $f
+	_scratch_unmount
+	$BTRFS_CORRUPT_BLOCK_PROG -r 5 -I $ino,36,1 -v 88 -o 16 -b 1 $SCRATCH_DEV > /dev/null 2>&1
+	_scratch_mount
+	validate $f
+}
+
+# corrupt the Merkle tree data itself
+corrupt_merkle_tree() {
+	local f=$SCRATCH_MNT/merkle
+	$XFS_IO_PROG -fc "pwrite -q -S 0x58 0 12288" $f
+	local ino=$(get_ino $f)
+	_fsv_enable $f
+	_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 2>&1
+	_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
+
+status=0
+exit
diff --git a/tests/btrfs/290.out b/tests/btrfs/290.out
new file mode 100644
index 00000000..056b114b
--- /dev/null
+++ b/tests/btrfs/290.out
@@ -0,0 +1,25 @@ 
+QA output created by 290
+inl
+pread: Input/output error
+pread: Input/output error
+prealloc
+pread: Input/output error
+pread: Input/output error
+reg
+pread: Input/output error
+pread: Input/output error
+punch
+pread: Input/output error
+pread: Input/output error
+plug
+pread: Input/output error
+pread: Input/output error
+desc
+SCRATCH_MNT/desc: Invalid argument
+SCRATCH_MNT/desc: Invalid argument
+roothash
+pread: Input/output error
+pread: Input/output error
+merkle
+pread: Input/output error
+pread: Input/output error