Re: [xfstests PATCH] generic/574: test corruption at more offsets

[Eric Biggers <ebiggers@kernel.org>]

On Wed, Jun 19, 2024 at 02:49:23PM +0200, Andrey Albershteyn wrote:
> On 2024-06-11 20:53:34, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Expand generic/574 to test for corruption in more different parts of the
> > file to try to exercise any hashing optimizations that might be used.
> > 
> > There is no existing bug that this finds.  This is just to prevent
> > future bugs, considering optimizations along the lines of
> > https://lore.kernel.org/fsverity/20240611034822.36603-7-ebiggers@kernel.org/
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> >  tests/generic/574 | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/tests/generic/574 b/tests/generic/574
> > index cb42baaa..72440d49 100755
> > --- a/tests/generic/574
> > +++ b/tests/generic/574
> > @@ -194,10 +194,25 @@ test_block_size()
> >  	corruption_test $block_size 131072 0 5
> >  	corruption_test $block_size 131072 4091 5
> >  	corruption_test $block_size 131072 65536 65536
> >  	corruption_test $block_size 131072 131067 5
> >  
> > +	# Test corrupting a block in files of length 1..4 blocks, and test
> > +	# corrupting each block of a 4-block file.  This ensures that all code
> > +	# paths that might exist due to multi-block hashing optimizations the
> > +	# fsverity implementation may use get covered, assuming no more than 4
> > +	# blocks are hashed at once.  E.g., consider an fsverity implementation
> > +	# that verifies sets of blocks but has a bug when given a single block,
> > +	# or that has a bug that makes it not verify all the blocks of each set.
> > +	local i
> > +	for i in $(seq 1 4); do
> > +		corruption_test $block_size $((i*block_size)) $((block_size/2)) 5
> > +	done
> > +	for i in $(seq 0 3); do
> > +		corruption_test $block_size $((4*block_size)) $((i*block_size)) 5
> > +	done
> > +
> >  	corrupt_eof_block_test $block_size 130999 72
> >  
> >  	# Merkle tree corruption.
> >  	corruption_test $block_size 200000 100 10 true
> >  
> > 
> > base-commit: e46fa3a7dae4a65fd80128bf381dba4fd5036ebb
> > -- 
> > 2.45.2
> > 
> > 
> 
> The addition looks fine to me, why is it 4 blocks though, isn't 2
> enough?
> 
> Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>
> 

For the currently proposed fsverity patch, 2 is enough.  The same approach could
be extended to more blocks later, though.  So using something a bit higher in
the test from the beginning makes sense.

- Eric