| Message ID | ca320cd0c8427458828cc36d5d5168bbe6b6bab2.1617906318.git.boris@bur.io (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tests for btrfs fsverity | expand |
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
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
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
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.
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" ;;
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(-)