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
|