Re: [PATCH v6 1/4] btrfs: test btrfs specific fsverity corruption

[Boris Burkov <boris@bur.io>]

On Mon, Mar 14, 2022 at 11:58:35PM +0000, Eric Biggers wrote:
> On Mon, Feb 14, 2022 at 04:09:55PM -0800, Boris Burkov wrote:
> > diff --git a/common/verity b/common/verity
> > index 38eea157..eec8ae72 100644
> > --- a/common/verity
> > +++ b/common/verity
> > @@ -3,6 +3,8 @@
> >  #
> >  # Functions for setting up and testing fs-verity
> >  
> > +. common/btrfs
> 
> Why does common/btrfs need to be included here?

To get _require_btrfs_corrupt_block. I can move that in here, as it
doesn't have any non-verity use case in btrfs tests yet. But it should
probably stay a named function rather than get rolled into
require_fsverity_corruption if I make btrfs/290 use it directly (which I
think is a good suggestion).

> 
> > +# 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
> > +}
> 
> Which function(s), specifically, is this function a prerequisite for?  Based on
> the name, it sounds like it would be a prerequisite for calling
> _fsv_scratch_corrupt_bytes() and _fsv_scratch_corrupt_merkle_tree().  But that's
> apparently not the case, seeing as generic/576 calls
> _fsv_scratch_corrupt_bytes() but doesn't call _require_fsverity_corruption(),
> and your new test btrfs/290 calls _require_fsverity_corruption() but doesn't
> call either _fsv_scratch_corrupt_bytes() or _fsv_scratch_corrupt_merkle_tree().
> 
> So, the purpose of this function is unclear.
> 
> Perhaps it should be a prerequisite for all _fsv_scratch_corrupt*() functions,
> and btrfs/290 should check for btrfs-corrupt-block directly?

My intent was for it to be a requirement for _fsv_scratch_corrupt_bytes
and _fsv_scratch_corrupt_merkle_tree, but I missed 576, since I was
testing with btrfs and that doesn't have transparent encryption. Apologies
for missing that test. I'll make _require_fsverity_corruption 1:1 with
_fsv_scratch_corrupt_bytes. However, I believe I still need
btrfs_corrupt_block here, since that is part of the "generic" corruption
for btrfs.

> 
> - Eric