[PATCHSET v5.6] fstests: fs-verity support for XFS

[PATCHSET v5.6] fstests: fs-verity support for XFS

[Darrick J. Wong]

Hi all,

This patchset adds support for fsverity to XFS.  In keeping with
Andrey's original design, XFS stores all fsverity metadata in the
extended attribute data.  However, I've made a few changes to the code:
First, it now caches merkle tree blocks directly instead of abusing the
buffer cache.  This reduces lookup overhead quite a bit, at a cost of
needing a new shrinker for cached merkle tree blocks.

To reduce the ondisk footprint further, I also made the verity
enablement code detect trailing zeroes whenever fsverity tells us to
write a buffer, and elide storing the zeroes.  To further reduce the
footprint of sparse files, I also skip writing merkle tree blocks if the
block contents are entirely hashes of zeroes.

Next, I implemented more of the tooling around verity, such as debugger
support, as much fsck support as I can manage without knowing the
internal format of the fsverity information; and added support for
xfs_scrub to read fsverity files to validate the consistency of the data
against the merkle tree.

Finally, I add the ability for administrators to turn off fsverity,
which might help recovering damaged data from an inconsistent file.

From Andrey Albershteyn:

Here's v5 of my patchset of adding fs-verity support to XFS.

This implementation uses extended attributes to store fs-verity
metadata. The Merkle tree blocks are stored in the remote extended
attributes. The names are offsets into the tree.
From Darrick J. Wong:

This v5.3 patchset builds upon v5.2 of Andrey's patchset to implement
fsverity for XFS.

The biggest thing that I didn't like in the v5 patchset is the abuse of
the data device's buffer cache to store the incore version of the merkle
tree blocks.  Not only do verity state flags end up in xfs_buf, but the
double-alloc flag wastes memory and doesn't remain internally consistent
if the xattrs shift around.

I replaced all of that with a per-inode xarray that indexes incore
merkle tree blocks.  For cache hits, this dramatically reduces the
amount of work that xfs has to do to feed fsverity.  The per-block
overhead is much lower (8 bytes instead of ~300 for xfs_bufs), and we no
longer have to entertain layering violations in the buffer cache.  I
also added a per-filesystem shrinker so that reclaim can cull cached
merkle tree blocks, starting with the leaf tree nodes.

I've also rolled in some changes recommended by the fsverity maintainer,
fixed some organization and naming problems in the xfs code, fixed a
collision in the xfs_inode iflags, and improved dead merkle tree cleanup
per the discussion of the v5 series.  At this point I'm happy enough
with this code to start integrating and testing it in my trees, so it's
time to send it out a coherent patchset for comments.

For v5.3, I've added bits and pieces of online and offline repair
support, reduced the size of partially filled merkle tree blocks by
removing trailing zeroes, changed the xattr hash function to better
avoid collisions between merkle tree keys, made the fsverity
invalidation bitmap unnecessary, and made it so that we can save space
on sparse verity files by not storing merkle tree blocks that hash
totally zeroed data blocks.

From Andrey Albershteyn:

Here's v5 of my patchset of adding fs-verity support to XFS.

This implementation uses extended attributes to store fs-verity
metadata. The Merkle tree blocks are stored in the remote extended
attributes. The names are offsets into the tree.

If you're going to start using this code, I strongly recommend pulling
from my git trees, which are linked below.

This has been running on the djcloud for months with no problems.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=fsverity

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=fsverity

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=fsverity
---
Commits in this patchset:
 * common/verity: enable fsverity for XFS
 * xfs/{021,122}: adapt to fsverity xattrs
 * xfs/122: adapt to fsverity
 * xfs: test xfs_scrub detection and correction of corrupt fsverity metadata
 * xfs: test disabling fsverity
 * common/populate: add verity files to populate xfs images
---
 common/populate    |   24 +++++++++
 common/verity      |   39 ++++++++++++++-
 tests/xfs/021      |    3 +
 tests/xfs/122.out  |    3 +
 tests/xfs/1880     |  135 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1880.out |   37 ++++++++++++++
 tests/xfs/1881     |  111 +++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1881.out |   28 +++++++++++
 8 files changed, 378 insertions(+), 2 deletions(-)
 create mode 100755 tests/xfs/1880
 create mode 100644 tests/xfs/1880.out
 create mode 100755 tests/xfs/1881
 create mode 100644 tests/xfs/1881.out

[PATCH 1/6] common/verity: enable fsverity for XFS

[Darrick J. Wong]

From: Andrey Albershteyn <aalbersh@redhat.com>

XFS supports verity and can be enabled for -g verity group.

Signed-off-by: Andrey Albershteyn <andrey.albershteyn@gmail.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/verity |   39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)


diff --git a/common/verity b/common/verity
index 59b67e1201..20408c8c0e 100644
--- a/common/verity
+++ b/common/verity
@@ -43,7 +43,16 @@ _require_scratch_verity()
 
 	# The filesystem may be aware of fs-verity but have it disabled by
 	# CONFIG_FS_VERITY=n.  Detect support via sysfs.
-	if [ ! -e /sys/fs/$fstyp/features/verity ]; then
+	case $FSTYP in
+	xfs)
+		_scratch_unmount
+		_check_scratch_xfs_features VERITY &>>$seqres.full
+		_scratch_mount
+	;;
+	*)
+		test -e /sys/fs/$fstyp/features/verity
+	esac
+	if [ ! $? ]; then
 		_notrun "kernel $fstyp isn't configured with verity support"
 	fi
 
@@ -201,6 +210,9 @@ _scratch_mkfs_verity()
 	ext4|f2fs)
 		_scratch_mkfs -O verity
 		;;
+	xfs)
+		_scratch_mkfs -i verity
+		;;
 	btrfs)
 		_scratch_mkfs
 		;;
@@ -334,12 +346,19 @@ _fsv_scratch_corrupt_bytes()
 	local lstart lend pstart pend
 	local dd_cmds=()
 	local cmd
+	local device=$SCRATCH_DEV
 
 	sync	# Sync to avoid unwritten extents
 
 	cat > $tmp.bytes
 	local end=$(( offset + $(_get_filesize $tmp.bytes ) ))
 
+	# If this is an xfs realtime file, switch @device to the rt device
+	if [ $FSTYP = "xfs" ]; then
+		$XFS_IO_PROG -r -c 'stat -v' "$file" | grep -q -w realtime && \
+			device=$SCRATCH_RTDEV
+	fi
+
 	# For each extent that intersects the requested range in order, add a
 	# command that writes the next part of the data to that extent.
 	while read -r lstart lend pstart pend; do
@@ -355,7 +374,7 @@ _fsv_scratch_corrupt_bytes()
 		elif (( offset < lend )); then
 			local len=$((lend - offset))
 			local seek=$((pstart + (offset - lstart)))
-			dd_cmds+=("head -c $len | dd of=$SCRATCH_DEV oflag=seek_bytes seek=$seek status=none")
+			dd_cmds+=("head -c $len | dd of=$device oflag=seek_bytes seek=$seek status=none")
 			(( offset += len ))
 		fi
 	done < <($XFS_IO_PROG -r -c "fiemap $offset $((end - offset))" "$file" \
@@ -408,6 +427,22 @@ _fsv_scratch_corrupt_merkle_tree()
 		done
 		_scratch_mount
 		;;
+	xfs)
+		local ino=$(stat -c '%i' $file)
+		local attr_offset=$(( $offset % $FSV_BLOCK_SIZE ))
+		local attr_index=$(printf "%08d" $(( offset - attr_offset )))
+		_scratch_unmount
+		# Attribute name is 8 bytes long (byte position of Merkle tree block)
+		_scratch_xfs_db -x -c "inode $ino" \
+			-c "attr_modify -f -m 8 -o $attr_offset $attr_index \"BUG\"" \
+			-c "ablock 0" -c "print" \
+			>>$seqres.full
+		# In case bsize == 4096 and merkle block size == 1024, by
+		# modifying attribute with 'attr_modify we can corrupt quota
+		# account. Let's repair it
+		_scratch_xfs_repair >> $seqres.full 2>&1
+		_scratch_mount
+		;;
 	*)
 		_fail "_fsv_scratch_corrupt_merkle_tree() unimplemented on $FSTYP"
 		;;

[PATCH 2/6] xfs/{021,122}: adapt to fsverity xattrs

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Adjust these tests to accomdate the use of xattrs to store fsverity
metadata.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/021     |    3 +++
 tests/xfs/122.out |    1 +
 2 files changed, 4 insertions(+)


diff --git a/tests/xfs/021 b/tests/xfs/021
index ef307fc064..dcecf41958 100755
--- a/tests/xfs/021
+++ b/tests/xfs/021
@@ -118,6 +118,7 @@ _scratch_xfs_db -r -c "inode $inum_1" -c "print a.sfattr"  | \
 	perl -ne '
 /\.secure/ && next;
 /\.parent/ && next;
+/\.verity/ && next;
 	print unless /^\d+:\[.*/;'
 
 echo "*** dump attributes (2)"
@@ -128,6 +129,7 @@ _scratch_xfs_db -r -c "inode $inum_2" -c "a a.bmx[0].startblock" -c print  \
 	| perl -ne '
 s/,secure//;
 s/,parent//;
+s/,verity//;
 s/info.hdr/info/;
 /hdr.info.crc/ && next;
 /hdr.info.bno/ && next;
@@ -135,6 +137,7 @@ s/info.hdr/info/;
 /hdr.info.lsn/ && next;
 /hdr.info.owner/ && next;
 /\.parent/ && next;
+/\.verity/ && next;
 s/^(hdr.info.magic =) 0x3bee/\1 0xfbee/;
 s/^(hdr.firstused =) (\d+)/\1 FIRSTUSED/;
 s/^(hdr.freemap\[0-2] = \[base,size]).*/\1 [FREEMAP..]/;
diff --git a/tests/xfs/122.out b/tests/xfs/122.out
index abd82e7142..019fe7545f 100644
--- a/tests/xfs/122.out
+++ b/tests/xfs/122.out
@@ -142,6 +142,7 @@ sizeof(struct xfs_scrub_vec) = 16
 sizeof(struct xfs_scrub_vec_head) = 40
 sizeof(struct xfs_swap_extent) = 64
 sizeof(struct xfs_unmount_log_format) = 8
+sizeof(struct xfs_verity_merkle_key) = 8
 sizeof(struct xfs_xmd_log_format) = 16
 sizeof(struct xfs_xmi_log_format) = 88
 sizeof(union xfs_rtword_raw) = 4

[PATCH 3/6] xfs/122: adapt to fsverity

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Add fields for fsverity ondisk structures.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/122.out |    2 ++
 1 file changed, 2 insertions(+)


diff --git a/tests/xfs/122.out b/tests/xfs/122.out
index 019fe7545f..22f36c0311 100644
--- a/tests/xfs/122.out
+++ b/tests/xfs/122.out
@@ -65,6 +65,7 @@ sizeof(struct xfs_agfl) = 36
 sizeof(struct xfs_attr3_leaf_hdr) = 80
 sizeof(struct xfs_attr3_leafblock) = 88
 sizeof(struct xfs_attr3_rmt_hdr) = 56
+sizeof(struct xfs_attr3_rmtverity_hdr) = 36
 sizeof(struct xfs_attr_sf_entry) = 3
 sizeof(struct xfs_attr_sf_hdr) = 4
 sizeof(struct xfs_attr_shortform) = 8
@@ -120,6 +121,7 @@ sizeof(struct xfs_log_dinode) = 176
 sizeof(struct xfs_log_legacy_timestamp) = 8
 sizeof(struct xfs_map_extent) = 32
 sizeof(struct xfs_map_freesp) = 32
+sizeof(struct xfs_merkle_key) = 8
 sizeof(struct xfs_parent_rec) = 12
 sizeof(struct xfs_phys_extent) = 16
 sizeof(struct xfs_refcount_key) = 4

[PATCH 4/6] xfs: test xfs_scrub detection and correction of corrupt fsverity metadata

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Create a basic test to ensure that xfs_scrub media scans complain about
files that don't pass fsverity validation.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/1880     |  135 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1880.out |   37 ++++++++++++++
 2 files changed, 172 insertions(+)
 create mode 100755 tests/xfs/1880
 create mode 100644 tests/xfs/1880.out


diff --git a/tests/xfs/1880 b/tests/xfs/1880
new file mode 100755
index 0000000000..a2119f04c2
--- /dev/null
+++ b/tests/xfs/1880
@@ -0,0 +1,135 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Oracle.  All Rights Reserved.
+#
+# FS QA Test 1880
+#
+# Corrupt fsverity descriptor, merkle tree blocks, and file contents.  Ensure
+# that xfs_scrub detects this and repairs whatever it can.
+#
+. ./common/preamble
+_begin_fstest auto quick verity
+
+_cleanup()
+{
+	cd /
+	_restore_fsverity_signatures
+	rm -f $tmp.*
+}
+
+. ./common/verity
+. ./common/filter
+. ./common/fuzzy
+
+_supported_fs xfs
+_require_scratch_verity
+_disable_fsverity_signatures
+_require_fsverity_corruption
+_require_scratch_nocheck	# fsck test
+
+_scratch_mkfs >> $seqres.full
+_scratch_mount
+
+_require_scratch_xfs_scrub
+_require_xfs_has_feature "$SCRATCH_MNT" verity
+VICTIM_FILE="$SCRATCH_MNT/a"
+_fsv_can_enable "$VICTIM_FILE" || _notrun "cannot enable fsverity"
+
+create_victim()
+{
+	local filesize="${1:-3}"
+
+	rm -f "$VICTIM_FILE"
+	perl -e "print 'moo' x $((filesize / 3))" > "$VICTIM_FILE"
+	fsverity enable --hash-alg=sha256 --block-size=1024 "$VICTIM_FILE"
+	fsverity measure "$VICTIM_FILE" | _filter_scratch
+}
+
+filter_scrub() {
+	awk '{
+		if ($0 ~ /fsverity metadata missing/) {
+			print("fsverity metadata missing");
+		} else if ($0 ~ /Corruption.*inode record/) {
+			print("xfs_ino corruption");
+		} else if ($0 ~ /verity error at offset/) {
+			print("fsverity read error");
+		}
+	}'
+}
+
+run_scrub() {
+	$XFS_SCRUB_PROG -b -x $* $SCRATCH_MNT &> $tmp.moo
+	filter_scrub < $tmp.moo
+	cat $tmp.moo >> $seqres.full
+}
+
+cat_victim() {
+	$XFS_IO_PROG -r -c 'pread -q 0 4096' "$VICTIM_FILE" 2>&1 | _filter_scratch
+}
+
+echo "Part 1: Delete the fsverity descriptor" | tee -a $seqres.full
+create_victim
+_scratch_unmount
+_scratch_xfs_db -x -c "path /a" -c "attr_remove -f vdesc" -c 'ablock 0' -c print >> $seqres.full
+_scratch_mount
+cat_victim
+run_scrub -n
+
+echo "Part 2: Run repair to clear XFS_DIFLAG2_VERITY" | tee -a $seqres.full
+run_scrub
+cat_victim
+run_scrub -n
+
+echo "Part 3: Corrupt the fsverity descriptor" | tee -a $seqres.full
+create_victim
+_scratch_unmount
+_scratch_xfs_db -x -c "path /a" -c 'attr_modify -f "vdesc" -o 0 "BUGSAHOY"' -c 'ablock 0' -c print >> $seqres.full
+_scratch_mount
+cat_victim
+run_scrub -n
+
+echo "Part 4: Run repair to clear XFS_DIFLAG2_VERITY" | tee -a $seqres.full
+run_scrub
+cat_victim
+run_scrub -n
+
+echo "Part 5: Corrupt the fsverity file data" | tee -a $seqres.full
+create_victim
+_scratch_unmount
+_scratch_xfs_db -x -c "path /a" -c 'dblock 0' -c 'blocktrash -3 -o 0 -x 24 -y 24 -z' -c print >> $seqres.full
+_scratch_mount
+cat_victim
+run_scrub -n
+
+echo "Part 6: Run repair which will not help" | tee -a $seqres.full
+run_scrub
+cat_victim
+run_scrub -n
+
+echo "Part 7: Corrupt a merkle tree block" | tee -a $seqres.full
+create_victim 1234 # two merkle tree blocks
+_fsv_scratch_corrupt_merkle_tree "$VICTIM_FILE" 0
+cat_victim
+run_scrub -n
+
+echo "Part 8: Run repair which will not help" | tee -a $seqres.full
+run_scrub
+cat_victim
+run_scrub -n
+
+echo "Part 9: Corrupt the fsverity salt" | tee -a $seqres.full
+create_victim
+_scratch_unmount
+_scratch_xfs_db -x -c "path /a" -c 'attr_modify -f "vdesc" -o 3 #08' -c 'attr_modify -f "vdesc" -o 80 "BUGSAHOY"' -c 'ablock 0' -c print >> $seqres.full
+_scratch_mount
+cat_victim
+run_scrub -n
+
+echo "Part 10: Run repair which will not help" | tee -a $seqres.full
+run_scrub
+cat_victim
+run_scrub -n
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/1880.out b/tests/xfs/1880.out
new file mode 100644
index 0000000000..17961ec70b
--- /dev/null
+++ b/tests/xfs/1880.out
@@ -0,0 +1,37 @@
+QA output created by 1880
+Part 1: Delete the fsverity descriptor
+sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
+SCRATCH_MNT/a: Invalid argument
+xfs_ino corruption
+fsverity metadata missing
+Part 2: Run repair to clear XFS_DIFLAG2_VERITY
+Part 3: Corrupt the fsverity descriptor
+sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
+SCRATCH_MNT/a: Invalid argument
+xfs_ino corruption
+fsverity metadata missing
+Part 4: Run repair to clear XFS_DIFLAG2_VERITY
+Part 5: Corrupt the fsverity file data
+sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
+pread: Input/output error
+fsverity read error
+Part 6: Run repair which will not help
+fsverity read error
+pread: Input/output error
+fsverity read error
+Part 7: Corrupt a merkle tree block
+sha256:c56f1115966bafa6c9d32b4717f554b304161f33923c9292c7a92a27866a853c SCRATCH_MNT/a
+pread: Input/output error
+fsverity read error
+Part 8: Run repair which will not help
+fsverity read error
+pread: Input/output error
+fsverity read error
+Part 9: Corrupt the fsverity salt
+sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
+pread: Input/output error
+fsverity read error
+Part 10: Run repair which will not help
+fsverity read error
+pread: Input/output error
+fsverity read error

[PATCH 5/6] xfs: test disabling fsverity

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Add a test to make sure that we can disable fsverity on a file that
doesn't pass fsverity validation on its contents anymore.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/1881     |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1881.out |   28 +++++++++++++
 2 files changed, 139 insertions(+)
 create mode 100755 tests/xfs/1881
 create mode 100644 tests/xfs/1881.out


diff --git a/tests/xfs/1881 b/tests/xfs/1881
new file mode 100755
index 0000000000..411802d7c7
--- /dev/null
+++ b/tests/xfs/1881
@@ -0,0 +1,111 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Oracle.  All Rights Reserved.
+#
+# FS QA Test 1881
+#
+# Corrupt fsverity descriptor, merkle tree blocks, and file contents.  Ensure
+# that we can still disable fsverity, at least for the latter cases.
+#
+. ./common/preamble
+_begin_fstest auto quick verity
+
+_cleanup()
+{
+	cd /
+	_restore_fsverity_signatures
+	rm -f $tmp.*
+}
+
+. ./common/verity
+. ./common/filter
+. ./common/fuzzy
+
+_supported_fs xfs
+_require_scratch_verity
+_disable_fsverity_signatures
+_require_fsverity_corruption
+_require_xfs_io_command noverity
+_require_scratch_nocheck	# corruption test
+
+_scratch_mkfs >> $seqres.full
+_scratch_mount
+
+_require_xfs_has_feature "$SCRATCH_MNT" verity
+VICTIM_FILE="$SCRATCH_MNT/a"
+_fsv_can_enable "$VICTIM_FILE" || _notrun "cannot enable fsverity"
+
+create_victim()
+{
+	local filesize="${1:-3}"
+
+	rm -f "$VICTIM_FILE"
+	perl -e "print 'moo' x $((filesize / 3))" > "$VICTIM_FILE"
+	fsverity enable --hash-alg=sha256 --block-size=1024 "$VICTIM_FILE"
+	fsverity measure "$VICTIM_FILE" | _filter_scratch
+}
+
+disable_verity() {
+	$XFS_IO_PROG -r -c 'noverity' "$VICTIM_FILE" 2>&1 | _filter_scratch
+}
+
+cat_victim() {
+	$XFS_IO_PROG -r -c 'pread -q 0 4096' "$VICTIM_FILE" 2>&1 | _filter_scratch
+}
+
+echo "Part 1: Delete the fsverity descriptor" | tee -a $seqres.full
+create_victim
+_scratch_unmount
+_scratch_xfs_db -x -c "path /a" -c "attr_remove -f vdesc" -c 'ablock 0' -c print >> $seqres.full
+_scratch_mount
+cat_victim
+
+echo "Part 2: Disable fsverity, which won't work" | tee -a $seqres.full
+disable_verity
+cat_victim
+
+echo "Part 3: Corrupt the fsverity descriptor" | tee -a $seqres.full
+create_victim
+_scratch_unmount
+_scratch_xfs_db -x -c "path /a" -c 'attr_modify -f "vdesc" -o 0 "BUGSAHOY"' -c 'ablock 0' -c print >> $seqres.full
+_scratch_mount
+cat_victim
+
+echo "Part 4: Disable fsverity, which won't work" | tee -a $seqres.full
+disable_verity
+cat_victim
+
+echo "Part 5: Corrupt the fsverity file data" | tee -a $seqres.full
+create_victim
+_scratch_unmount
+_scratch_xfs_db -x -c "path /a" -c 'dblock 0' -c 'blocktrash -3 -o 0 -x 24 -y 24 -z' -c print >> $seqres.full
+_scratch_mount
+cat_victim
+
+echo "Part 6: Disable fsverity, which should work" | tee -a $seqres.full
+disable_verity
+cat_victim
+
+echo "Part 7: Corrupt a merkle tree block" | tee -a $seqres.full
+create_victim 1234 # two merkle tree blocks
+_fsv_scratch_corrupt_merkle_tree "$VICTIM_FILE" 0
+cat_victim
+
+echo "Part 8: Disable fsverity, which should work" | tee -a $seqres.full
+disable_verity
+cat_victim
+
+echo "Part 9: Corrupt the fsverity salt" | tee -a $seqres.full
+create_victim
+_scratch_unmount
+_scratch_xfs_db -x -c "path /a" -c 'attr_modify -f "vdesc" -o 3 #08' -c 'attr_modify -f "vdesc" -o 80 "BUGSAHOY"' -c 'ablock 0' -c print >> $seqres.full
+_scratch_mount
+cat_victim
+
+echo "Part 10: Disable fsverity, which should work" | tee -a $seqres.full
+disable_verity
+cat_victim
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/1881.out b/tests/xfs/1881.out
new file mode 100644
index 0000000000..3e94b8001e
--- /dev/null
+++ b/tests/xfs/1881.out
@@ -0,0 +1,28 @@
+QA output created by 1881
+Part 1: Delete the fsverity descriptor
+sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
+SCRATCH_MNT/a: Invalid argument
+Part 2: Disable fsverity, which won't work
+SCRATCH_MNT/a: Invalid argument
+SCRATCH_MNT/a: Invalid argument
+Part 3: Corrupt the fsverity descriptor
+sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
+SCRATCH_MNT/a: Invalid argument
+Part 4: Disable fsverity, which won't work
+SCRATCH_MNT/a: Invalid argument
+SCRATCH_MNT/a: Invalid argument
+Part 5: Corrupt the fsverity file data
+sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
+pread: Input/output error
+Part 6: Disable fsverity, which should work
+pread: Input/output error
+Part 7: Corrupt a merkle tree block
+sha256:c56f1115966bafa6c9d32b4717f554b304161f33923c9292c7a92a27866a853c SCRATCH_MNT/a
+pread: Input/output error
+Part 8: Disable fsverity, which should work
+pread: Input/output error
+Part 9: Corrupt the fsverity salt
+sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
+pread: Input/output error
+Part 10: Disable fsverity, which should work
+pread: Input/output error

[PATCH 6/6] common/populate: add verity files to populate xfs images

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

If verity is enabled on a filesystem, we should create some sample
verity files.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/populate |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)


diff --git a/common/populate b/common/populate
index 35071f4210..ab9495e739 100644
--- a/common/populate
+++ b/common/populate
@@ -520,6 +520,30 @@ _scratch_xfs_populate() {
 		done
 	fi
 
+	# verity merkle trees
+	is_verity="$(_xfs_has_feature "$SCRATCH_MNT" verity -v)"
+	if [ $is_verity -gt 0 ]; then
+		echo "+ fsverity"
+
+		# Create a biggish file with all zeroes, because metadump
+		# won't preserve data blocks and we don't want the hashes to
+		# stop working for our sample fs.
+		for ((pos = 0, i = 88; pos < 23456789; pos += 234567, i++)); do
+			$XFS_IO_PROG -f -c "pwrite -S 0 $pos 234567" "$SCRATCH_MNT/verity"
+		done
+
+		fsverity enable "$SCRATCH_MNT/verity"
+
+		# Create a sparse file
+		$XFS_IO_PROG -f -c "pwrite -S 0 0 3" -c "pwrite -S 0 23456789 3" "$SCRATCH_MNT/sparse_verity"
+		fsverity enable "$SCRATCH_MNT/sparse_verity"
+
+		# Create a salted sparse file
+		$XFS_IO_PROG -f -c "pwrite -S 0 0 3" -c "pwrite -S 0 23456789 3" "$SCRATCH_MNT/salted_verity"
+		local salt="5846532066696e616c6c7920686173206461746120636865636b73756d732121"	# XFS finally has data checksums!!
+		fsverity enable --salt="$salt" "$SCRATCH_MNT/salted_verity"
+	fi
+
 	# Copy some real files (xfs tests, I guess...)
 	echo "+ real files"
 	test $fill -ne 0 && __populate_fill_fs "${SCRATCH_MNT}" 5

Re: [PATCH 4/6] xfs: test xfs_scrub detection and correction of corrupt fsverity metadata

[Andrey Albershteyn]

On 2024-04-29 20:41:50, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Create a basic test to ensure that xfs_scrub media scans complain about
> files that don't pass fsverity validation.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/1880     |  135 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/1880.out |   37 ++++++++++++++
>  2 files changed, 172 insertions(+)
>  create mode 100755 tests/xfs/1880
>  create mode 100644 tests/xfs/1880.out
> 
> 
> diff --git a/tests/xfs/1880 b/tests/xfs/1880
> new file mode 100755
> index 0000000000..a2119f04c2
> --- /dev/null
> +++ b/tests/xfs/1880
> @@ -0,0 +1,135 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Oracle.  All Rights Reserved.
> +#
> +# FS QA Test 1880
> +#
> +# Corrupt fsverity descriptor, merkle tree blocks, and file contents.  Ensure
> +# that xfs_scrub detects this and repairs whatever it can.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick verity
> +
> +_cleanup()
> +{
> +	cd /
> +	_restore_fsverity_signatures
> +	rm -f $tmp.*
> +}
> +
> +. ./common/verity
> +. ./common/filter
> +. ./common/fuzzy
> +
> +_supported_fs xfs
> +_require_scratch_verity
> +_disable_fsverity_signatures
> +_require_fsverity_corruption
> +_require_scratch_nocheck	# fsck test
> +
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount
> +
> +_require_scratch_xfs_scrub
> +_require_xfs_has_feature "$SCRATCH_MNT" verity
> +VICTIM_FILE="$SCRATCH_MNT/a"
> +_fsv_can_enable "$VICTIM_FILE" || _notrun "cannot enable fsverity"

I think this is not necessary, _require_scratch_verity already does
check if verity can be enabled (with more detailed errors).

Otherwise, looks good to me:
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>

-- 
- Andrey

Re: [PATCH 1/6] common/verity: enable fsverity for XFS

[Andrey Albershteyn]

On 2024-04-29 20:41:03, Darrick J. Wong wrote:
> From: Andrey Albershteyn <aalbersh@redhat.com>
> 
> XFS supports verity and can be enabled for -g verity group.
> 
> Signed-off-by: Andrey Albershteyn <andrey.albershteyn@gmail.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  common/verity |   39 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 37 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/common/verity b/common/verity
> index 59b67e1201..20408c8c0e 100644
> --- a/common/verity
> +++ b/common/verity
> @@ -43,7 +43,16 @@ _require_scratch_verity()
>  
>  	# The filesystem may be aware of fs-verity but have it disabled by
>  	# CONFIG_FS_VERITY=n.  Detect support via sysfs.
> -	if [ ! -e /sys/fs/$fstyp/features/verity ]; then
> +	case $FSTYP in
> +	xfs)
> +		_scratch_unmount
> +		_check_scratch_xfs_features VERITY &>>$seqres.full
> +		_scratch_mount
> +	;;
> +	*)
> +		test -e /sys/fs/$fstyp/features/verity
> +	esac
> +	if [ ! $? ]; then
>  		_notrun "kernel $fstyp isn't configured with verity support"
>  	fi
>  
> @@ -201,6 +210,9 @@ _scratch_mkfs_verity()
>  	ext4|f2fs)
>  		_scratch_mkfs -O verity
>  		;;
> +	xfs)
> +		_scratch_mkfs -i verity
> +		;;
>  	btrfs)
>  		_scratch_mkfs
>  		;;
> @@ -334,12 +346,19 @@ _fsv_scratch_corrupt_bytes()
>  	local lstart lend pstart pend
>  	local dd_cmds=()
>  	local cmd
> +	local device=$SCRATCH_DEV
>  
>  	sync	# Sync to avoid unwritten extents
>  
>  	cat > $tmp.bytes
>  	local end=$(( offset + $(_get_filesize $tmp.bytes ) ))
>  
> +	# If this is an xfs realtime file, switch @device to the rt device
> +	if [ $FSTYP = "xfs" ]; then
> +		$XFS_IO_PROG -r -c 'stat -v' "$file" | grep -q -w realtime && \
> +			device=$SCRATCH_RTDEV
> +	fi
> +
>  	# For each extent that intersects the requested range in order, add a
>  	# command that writes the next part of the data to that extent.
>  	while read -r lstart lend pstart pend; do
> @@ -355,7 +374,7 @@ _fsv_scratch_corrupt_bytes()
>  		elif (( offset < lend )); then
>  			local len=$((lend - offset))
>  			local seek=$((pstart + (offset - lstart)))
> -			dd_cmds+=("head -c $len | dd of=$SCRATCH_DEV oflag=seek_bytes seek=$seek status=none")
> +			dd_cmds+=("head -c $len | dd of=$device oflag=seek_bytes seek=$seek status=none")
>  			(( offset += len ))
>  		fi
>  	done < <($XFS_IO_PROG -r -c "fiemap $offset $((end - offset))" "$file" \
> @@ -408,6 +427,22 @@ _fsv_scratch_corrupt_merkle_tree()
>  		done
>  		_scratch_mount
>  		;;
> +	xfs)
> +		local ino=$(stat -c '%i' $file)

I didn't know about xfs_db's "path" command, this can be probably
replace with -c "path $file", below in _scratch_xfs_db.

> +		local attr_offset=$(( $offset % $FSV_BLOCK_SIZE ))
> +		local attr_index=$(printf "%08d" $(( offset - attr_offset )))
> +		_scratch_unmount
> +		# Attribute name is 8 bytes long (byte position of Merkle tree block)
> +		_scratch_xfs_db -x -c "inode $ino" \
                                here   ^^^^^^^^^^
> +			-c "attr_modify -f -m 8 -o $attr_offset $attr_index \"BUG\"" \
> +			-c "ablock 0" -c "print" \
> +			>>$seqres.full
> +		# In case bsize == 4096 and merkle block size == 1024, by
> +		# modifying attribute with 'attr_modify we can corrupt quota
> +		# account. Let's repair it
> +		_scratch_xfs_repair >> $seqres.full 2>&1
> +		_scratch_mount
> +		;;
>  	*)
>  		_fail "_fsv_scratch_corrupt_merkle_tree() unimplemented on $FSTYP"
>  		;;
> 
> 

Otherwise, looks good to me:
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>

-- 
- Andrey

Re: [PATCH 3/6] xfs/122: adapt to fsverity

[Andrey Albershteyn]

On 2024-04-29 20:41:34, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Add fields for fsverity ondisk structures.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/122.out |    2 ++
>  1 file changed, 2 insertions(+)
> 
> 
> diff --git a/tests/xfs/122.out b/tests/xfs/122.out
> index 019fe7545f..22f36c0311 100644
> --- a/tests/xfs/122.out
> +++ b/tests/xfs/122.out
> @@ -65,6 +65,7 @@ sizeof(struct xfs_agfl) = 36
>  sizeof(struct xfs_attr3_leaf_hdr) = 80
>  sizeof(struct xfs_attr3_leafblock) = 88
>  sizeof(struct xfs_attr3_rmt_hdr) = 56
> +sizeof(struct xfs_attr3_rmtverity_hdr) = 36
>  sizeof(struct xfs_attr_sf_entry) = 3
>  sizeof(struct xfs_attr_sf_hdr) = 4
>  sizeof(struct xfs_attr_shortform) = 8
> @@ -120,6 +121,7 @@ sizeof(struct xfs_log_dinode) = 176
>  sizeof(struct xfs_log_legacy_timestamp) = 8
>  sizeof(struct xfs_map_extent) = 32
>  sizeof(struct xfs_map_freesp) = 32
> +sizeof(struct xfs_merkle_key) = 8
>  sizeof(struct xfs_parent_rec) = 12
>  sizeof(struct xfs_phys_extent) = 16
>  sizeof(struct xfs_refcount_key) = 4
> 
> 

Shouldn't this patch be squashed with previous one?

-- 
- Andrey

Re: [PATCH 2/6] xfs/{021,122}: adapt to fsverity xattrs

[Andrey Albershteyn]

On 2024-04-29 20:41:19, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Adjust these tests to accomdate the use of xattrs to store fsverity
> metadata.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/021     |    3 +++
>  tests/xfs/122.out |    1 +
>  2 files changed, 4 insertions(+)
> 
> 
> diff --git a/tests/xfs/021 b/tests/xfs/021
> index ef307fc064..dcecf41958 100755
> --- a/tests/xfs/021
> +++ b/tests/xfs/021
> @@ -118,6 +118,7 @@ _scratch_xfs_db -r -c "inode $inum_1" -c "print a.sfattr"  | \
>  	perl -ne '
>  /\.secure/ && next;
>  /\.parent/ && next;
> +/\.verity/ && next;
>  	print unless /^\d+:\[.*/;'
>  
>  echo "*** dump attributes (2)"
> @@ -128,6 +129,7 @@ _scratch_xfs_db -r -c "inode $inum_2" -c "a a.bmx[0].startblock" -c print  \
>  	| perl -ne '
>  s/,secure//;
>  s/,parent//;
> +s/,verity//;
>  s/info.hdr/info/;
>  /hdr.info.crc/ && next;
>  /hdr.info.bno/ && next;
> @@ -135,6 +137,7 @@ s/info.hdr/info/;
>  /hdr.info.lsn/ && next;
>  /hdr.info.owner/ && next;
>  /\.parent/ && next;
> +/\.verity/ && next;
>  s/^(hdr.info.magic =) 0x3bee/\1 0xfbee/;
>  s/^(hdr.firstused =) (\d+)/\1 FIRSTUSED/;
>  s/^(hdr.freemap\[0-2] = \[base,size]).*/\1 [FREEMAP..]/;
> diff --git a/tests/xfs/122.out b/tests/xfs/122.out
> index abd82e7142..019fe7545f 100644
> --- a/tests/xfs/122.out
> +++ b/tests/xfs/122.out
> @@ -142,6 +142,7 @@ sizeof(struct xfs_scrub_vec) = 16
>  sizeof(struct xfs_scrub_vec_head) = 40
>  sizeof(struct xfs_swap_extent) = 64
>  sizeof(struct xfs_unmount_log_format) = 8
> +sizeof(struct xfs_verity_merkle_key) = 8
>  sizeof(struct xfs_xmd_log_format) = 16
>  sizeof(struct xfs_xmi_log_format) = 88
>  sizeof(union xfs_rtword_raw) = 4
> 

Looks good to me:
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>

-- 
- Andrey

Re: [PATCH 5/6] xfs: test disabling fsverity

[Andrey Albershteyn]

On 2024-04-29 20:42:05, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Add a test to make sure that we can disable fsverity on a file that
> doesn't pass fsverity validation on its contents anymore.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/1881     |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/1881.out |   28 +++++++++++++
>  2 files changed, 139 insertions(+)
>  create mode 100755 tests/xfs/1881
>  create mode 100644 tests/xfs/1881.out
> 
> 
> diff --git a/tests/xfs/1881 b/tests/xfs/1881
> new file mode 100755
> index 0000000000..411802d7c7
> --- /dev/null
> +++ b/tests/xfs/1881
> @@ -0,0 +1,111 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Oracle.  All Rights Reserved.
> +#
> +# FS QA Test 1881
> +#
> +# Corrupt fsverity descriptor, merkle tree blocks, and file contents.  Ensure
> +# that we can still disable fsverity, at least for the latter cases.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick verity
> +
> +_cleanup()
> +{
> +	cd /
> +	_restore_fsverity_signatures
> +	rm -f $tmp.*
> +}
> +
> +. ./common/verity
> +. ./common/filter
> +. ./common/fuzzy
> +
> +_supported_fs xfs
> +_require_scratch_verity
> +_disable_fsverity_signatures
> +_require_fsverity_corruption
> +_require_xfs_io_command noverity
> +_require_scratch_nocheck	# corruption test
> +
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount
> +
> +_require_xfs_has_feature "$SCRATCH_MNT" verity
> +VICTIM_FILE="$SCRATCH_MNT/a"
> +_fsv_can_enable "$VICTIM_FILE" || _notrun "cannot enable fsverity"

also here, if not needed in 1880

Looks good to me:
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>

-- 
- Andrey

Re: [PATCH 5/6] xfs: test disabling fsverity

[Andrey Albershteyn]

On 2024-04-29 20:42:05, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Add a test to make sure that we can disable fsverity on a file that
> doesn't pass fsverity validation on its contents anymore.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/1881     |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/1881.out |   28 +++++++++++++
>  2 files changed, 139 insertions(+)
>  create mode 100755 tests/xfs/1881
>  create mode 100644 tests/xfs/1881.out
> 
> 
> diff --git a/tests/xfs/1881 b/tests/xfs/1881
> new file mode 100755
> index 0000000000..411802d7c7
> --- /dev/null
> +++ b/tests/xfs/1881
> @@ -0,0 +1,111 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Oracle.  All Rights Reserved.
> +#
> +# FS QA Test 1881
> +#
> +# Corrupt fsverity descriptor, merkle tree blocks, and file contents.  Ensure
> +# that we can still disable fsverity, at least for the latter cases.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick verity
> +
> +_cleanup()
> +{
> +	cd /
> +	_restore_fsverity_signatures
> +	rm -f $tmp.*
> +}
> +
> +. ./common/verity
> +. ./common/filter
> +. ./common/fuzzy
> +
> +_supported_fs xfs
> +_require_scratch_verity
> +_disable_fsverity_signatures
> +_require_fsverity_corruption
> +_require_xfs_io_command noverity
> +_require_scratch_nocheck	# corruption test
> +
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount
> +
> +_require_xfs_has_feature "$SCRATCH_MNT" verity
> +VICTIM_FILE="$SCRATCH_MNT/a"
> +_fsv_can_enable "$VICTIM_FILE" || _notrun "cannot enable fsverity"
> +
> +create_victim()
> +{
> +	local filesize="${1:-3}"
> +
> +	rm -f "$VICTIM_FILE"
> +	perl -e "print 'moo' x $((filesize / 3))" > "$VICTIM_FILE"
> +	fsverity enable --hash-alg=sha256 --block-size=1024 "$VICTIM_FILE"
> +	fsverity measure "$VICTIM_FILE" | _filter_scratch
> +}
> +
> +disable_verity() {
> +	$XFS_IO_PROG -r -c 'noverity' "$VICTIM_FILE" 2>&1 | _filter_scratch
> +}
> +
> +cat_victim() {
> +	$XFS_IO_PROG -r -c 'pread -q 0 4096' "$VICTIM_FILE" 2>&1 | _filter_scratch
> +}
> +
> +echo "Part 1: Delete the fsverity descriptor" | tee -a $seqres.full
> +create_victim
> +_scratch_unmount
> +_scratch_xfs_db -x -c "path /a" -c "attr_remove -f vdesc" -c 'ablock 0' -c print >> $seqres.full
> +_scratch_mount
> +cat_victim
> +
> +echo "Part 2: Disable fsverity, which won't work" | tee -a $seqres.full
> +disable_verity
> +cat_victim
> +
> +echo "Part 3: Corrupt the fsverity descriptor" | tee -a $seqres.full
> +create_victim
> +_scratch_unmount
> +_scratch_xfs_db -x -c "path /a" -c 'attr_modify -f "vdesc" -o 0 "BUGSAHOY"' -c 'ablock 0' -c print >> $seqres.full
> +_scratch_mount
> +cat_victim
> +
> +echo "Part 4: Disable fsverity, which won't work" | tee -a $seqres.full
> +disable_verity
> +cat_victim
> +
> +echo "Part 5: Corrupt the fsverity file data" | tee -a $seqres.full
> +create_victim
> +_scratch_unmount
> +_scratch_xfs_db -x -c "path /a" -c 'dblock 0' -c 'blocktrash -3 -o 0 -x 24 -y 24 -z' -c print >> $seqres.full
> +_scratch_mount
> +cat_victim
> +
> +echo "Part 6: Disable fsverity, which should work" | tee -a $seqres.full
> +disable_verity
> +cat_victim
> +
> +echo "Part 7: Corrupt a merkle tree block" | tee -a $seqres.full
> +create_victim 1234 # two merkle tree blocks
> +_fsv_scratch_corrupt_merkle_tree "$VICTIM_FILE" 0

hmm, _fsv_scratch_corrupt_merkle_tree calls _scratch_xfs_repair, and
now with xfs_repair knowing about fs-verity is probably a problem. I
don't remember what was the problem with quota (why xfs_repiar is
there), I can check it.

> +cat_victim
> +
> +echo "Part 8: Disable fsverity, which should work" | tee -a $seqres.full
> +disable_verity
> +cat_victim
> +
> +echo "Part 9: Corrupt the fsverity salt" | tee -a $seqres.full
> +create_victim
> +_scratch_unmount
> +_scratch_xfs_db -x -c "path /a" -c 'attr_modify -f "vdesc" -o 3 #08' -c 'attr_modify -f "vdesc" -o 80 "BUGSAHOY"' -c 'ablock 0' -c print >> $seqres.full
> +_scratch_mount
> +cat_victim
> +
> +echo "Part 10: Disable fsverity, which should work" | tee -a $seqres.full
> +disable_verity
> +cat_victim
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/1881.out b/tests/xfs/1881.out
> new file mode 100644
> index 0000000000..3e94b8001e
> --- /dev/null
> +++ b/tests/xfs/1881.out
> @@ -0,0 +1,28 @@
> +QA output created by 1881
> +Part 1: Delete the fsverity descriptor
> +sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
> +SCRATCH_MNT/a: Invalid argument
> +Part 2: Disable fsverity, which won't work
> +SCRATCH_MNT/a: Invalid argument
> +SCRATCH_MNT/a: Invalid argument
> +Part 3: Corrupt the fsverity descriptor
> +sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
> +SCRATCH_MNT/a: Invalid argument
> +Part 4: Disable fsverity, which won't work
> +SCRATCH_MNT/a: Invalid argument
> +SCRATCH_MNT/a: Invalid argument
> +Part 5: Corrupt the fsverity file data
> +sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
> +pread: Input/output error
> +Part 6: Disable fsverity, which should work
> +pread: Input/output error
> +Part 7: Corrupt a merkle tree block
> +sha256:c56f1115966bafa6c9d32b4717f554b304161f33923c9292c7a92a27866a853c SCRATCH_MNT/a
> +pread: Input/output error
> +Part 8: Disable fsverity, which should work
> +pread: Input/output error
> +Part 9: Corrupt the fsverity salt
> +sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
> +pread: Input/output error
> +Part 10: Disable fsverity, which should work
> +pread: Input/output error
> 

-- 
- Andrey

Re: [PATCH 6/6] common/populate: add verity files to populate xfs images

[Andrey Albershteyn]

On 2024-04-29 20:42:21, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> If verity is enabled on a filesystem, we should create some sample
> verity files.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  common/populate |   24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> 
> diff --git a/common/populate b/common/populate
> index 35071f4210..ab9495e739 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -520,6 +520,30 @@ _scratch_xfs_populate() {
>  		done
>  	fi
>  
> +	# verity merkle trees
> +	is_verity="$(_xfs_has_feature "$SCRATCH_MNT" verity -v)"
> +	if [ $is_verity -gt 0 ]; then
> +		echo "+ fsverity"
> +
> +		# Create a biggish file with all zeroes, because metadump
> +		# won't preserve data blocks and we don't want the hashes to
> +		# stop working for our sample fs.

Hashes of the data blocks in the merkle tree? All zeros to use
.zero_digest in fs-verity? Not sure if got this comment right

> +		for ((pos = 0, i = 88; pos < 23456789; pos += 234567, i++)); do
> +			$XFS_IO_PROG -f -c "pwrite -S 0 $pos 234567" "$SCRATCH_MNT/verity"
> +		done
> +
> +		fsverity enable "$SCRATCH_MNT/verity"
> +
> +		# Create a sparse file
> +		$XFS_IO_PROG -f -c "pwrite -S 0 0 3" -c "pwrite -S 0 23456789 3" "$SCRATCH_MNT/sparse_verity"
> +		fsverity enable "$SCRATCH_MNT/sparse_verity"
> +
> +		# Create a salted sparse file
> +		$XFS_IO_PROG -f -c "pwrite -S 0 0 3" -c "pwrite -S 0 23456789 3" "$SCRATCH_MNT/salted_verity"
> +		local salt="5846532066696e616c6c7920686173206461746120636865636b73756d732121"	# XFS finally has data checksums!!
> +		fsverity enable --salt="$salt" "$SCRATCH_MNT/salted_verity"
> +	fi
> +
>  	# Copy some real files (xfs tests, I guess...)
>  	echo "+ real files"
>  	test $fill -ne 0 && __populate_fill_fs "${SCRATCH_MNT}" 5
> 

-- 
- Andrey

Re: [PATCH 1/6] common/verity: enable fsverity for XFS

[Darrick J. Wong]

On Tue, Apr 30, 2024 at 02:39:04PM +0200, Andrey Albershteyn wrote:
> On 2024-04-29 20:41:03, Darrick J. Wong wrote:
> > From: Andrey Albershteyn <aalbersh@redhat.com>
> > 
> > XFS supports verity and can be enabled for -g verity group.
> > 
> > Signed-off-by: Andrey Albershteyn <andrey.albershteyn@gmail.com>
> > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  common/verity |   39 +++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 37 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/common/verity b/common/verity
> > index 59b67e1201..20408c8c0e 100644
> > --- a/common/verity
> > +++ b/common/verity
> > @@ -43,7 +43,16 @@ _require_scratch_verity()
> >  
> >  	# The filesystem may be aware of fs-verity but have it disabled by
> >  	# CONFIG_FS_VERITY=n.  Detect support via sysfs.
> > -	if [ ! -e /sys/fs/$fstyp/features/verity ]; then
> > +	case $FSTYP in
> > +	xfs)
> > +		_scratch_unmount
> > +		_check_scratch_xfs_features VERITY &>>$seqres.full
> > +		_scratch_mount
> > +	;;
> > +	*)
> > +		test -e /sys/fs/$fstyp/features/verity
> > +	esac
> > +	if [ ! $? ]; then
> >  		_notrun "kernel $fstyp isn't configured with verity support"
> >  	fi
> >  
> > @@ -201,6 +210,9 @@ _scratch_mkfs_verity()
> >  	ext4|f2fs)
> >  		_scratch_mkfs -O verity
> >  		;;
> > +	xfs)
> > +		_scratch_mkfs -i verity
> > +		;;
> >  	btrfs)
> >  		_scratch_mkfs
> >  		;;
> > @@ -334,12 +346,19 @@ _fsv_scratch_corrupt_bytes()
> >  	local lstart lend pstart pend
> >  	local dd_cmds=()
> >  	local cmd
> > +	local device=$SCRATCH_DEV
> >  
> >  	sync	# Sync to avoid unwritten extents
> >  
> >  	cat > $tmp.bytes
> >  	local end=$(( offset + $(_get_filesize $tmp.bytes ) ))
> >  
> > +	# If this is an xfs realtime file, switch @device to the rt device
> > +	if [ $FSTYP = "xfs" ]; then
> > +		$XFS_IO_PROG -r -c 'stat -v' "$file" | grep -q -w realtime && \
> > +			device=$SCRATCH_RTDEV
> > +	fi
> > +
> >  	# For each extent that intersects the requested range in order, add a
> >  	# command that writes the next part of the data to that extent.
> >  	while read -r lstart lend pstart pend; do
> > @@ -355,7 +374,7 @@ _fsv_scratch_corrupt_bytes()
> >  		elif (( offset < lend )); then
> >  			local len=$((lend - offset))
> >  			local seek=$((pstart + (offset - lstart)))
> > -			dd_cmds+=("head -c $len | dd of=$SCRATCH_DEV oflag=seek_bytes seek=$seek status=none")
> > +			dd_cmds+=("head -c $len | dd of=$device oflag=seek_bytes seek=$seek status=none")
> >  			(( offset += len ))
> >  		fi
> >  	done < <($XFS_IO_PROG -r -c "fiemap $offset $((end - offset))" "$file" \
> > @@ -408,6 +427,22 @@ _fsv_scratch_corrupt_merkle_tree()
> >  		done
> >  		_scratch_mount
> >  		;;
> > +	xfs)
> > +		local ino=$(stat -c '%i' $file)
> 
> I didn't know about xfs_db's "path" command, this can be probably
> replace with -c "path $file", below in _scratch_xfs_db.

You /can/ use the xfs_db path command here, but then you have to strip
out $SCRATCH_MNT from $file since it of course doesn't know about mount
points.  Since $file is a file path, we might as well use stat to find
the inumber.

> > +		local attr_offset=$(( $offset % $FSV_BLOCK_SIZE ))
> > +		local attr_index=$(printf "%08d" $(( offset - attr_offset )))
> > +		_scratch_unmount
> > +		# Attribute name is 8 bytes long (byte position of Merkle tree block)
> > +		_scratch_xfs_db -x -c "inode $ino" \
>                                 here   ^^^^^^^^^^
> > +			-c "attr_modify -f -m 8 -o $attr_offset $attr_index \"BUG\"" \
> > +			-c "ablock 0" -c "print" \
> > +			>>$seqres.full
> > +		# In case bsize == 4096 and merkle block size == 1024, by
> > +		# modifying attribute with 'attr_modify we can corrupt quota
> > +		# account. Let's repair it
> > +		_scratch_xfs_repair >> $seqres.full 2>&1
> > +		_scratch_mount
> > +		;;
> >  	*)
> >  		_fail "_fsv_scratch_corrupt_merkle_tree() unimplemented on $FSTYP"
> >  		;;
> > 
> > 
> 
> Otherwise, looks good to me:
> Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>

<nod>

--D

> -- 
> - Andrey
> 
> 

Re: [PATCH 2/6] xfs/{021,122}: adapt to fsverity xattrs

[Darrick J. Wong]

On Tue, Apr 30, 2024 at 02:46:18PM +0200, Andrey Albershteyn wrote:
> On 2024-04-29 20:41:19, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Adjust these tests to accomdate the use of xattrs to store fsverity
> > metadata.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/xfs/021     |    3 +++
> >  tests/xfs/122.out |    1 +
> >  2 files changed, 4 insertions(+)
> > 
> > 
> > diff --git a/tests/xfs/021 b/tests/xfs/021
> > index ef307fc064..dcecf41958 100755
> > --- a/tests/xfs/021
> > +++ b/tests/xfs/021
> > @@ -118,6 +118,7 @@ _scratch_xfs_db -r -c "inode $inum_1" -c "print a.sfattr"  | \
> >  	perl -ne '
> >  /\.secure/ && next;
> >  /\.parent/ && next;
> > +/\.verity/ && next;
> >  	print unless /^\d+:\[.*/;'
> >  
> >  echo "*** dump attributes (2)"
> > @@ -128,6 +129,7 @@ _scratch_xfs_db -r -c "inode $inum_2" -c "a a.bmx[0].startblock" -c print  \
> >  	| perl -ne '
> >  s/,secure//;
> >  s/,parent//;
> > +s/,verity//;
> >  s/info.hdr/info/;
> >  /hdr.info.crc/ && next;
> >  /hdr.info.bno/ && next;
> > @@ -135,6 +137,7 @@ s/info.hdr/info/;
> >  /hdr.info.lsn/ && next;
> >  /hdr.info.owner/ && next;
> >  /\.parent/ && next;
> > +/\.verity/ && next;
> >  s/^(hdr.info.magic =) 0x3bee/\1 0xfbee/;
> >  s/^(hdr.firstused =) (\d+)/\1 FIRSTUSED/;
> >  s/^(hdr.freemap\[0-2] = \[base,size]).*/\1 [FREEMAP..]/;
> > diff --git a/tests/xfs/122.out b/tests/xfs/122.out
> > index abd82e7142..019fe7545f 100644
> > --- a/tests/xfs/122.out
> > +++ b/tests/xfs/122.out
> > @@ -142,6 +142,7 @@ sizeof(struct xfs_scrub_vec) = 16
> >  sizeof(struct xfs_scrub_vec_head) = 40
> >  sizeof(struct xfs_swap_extent) = 64
> >  sizeof(struct xfs_unmount_log_format) = 8
> > +sizeof(struct xfs_verity_merkle_key) = 8

Whoops, this change isn't needed anymore.

--D

> >  sizeof(struct xfs_xmd_log_format) = 16
> >  sizeof(struct xfs_xmi_log_format) = 88
> >  sizeof(union xfs_rtword_raw) = 4
> > 
> 
> Looks good to me:
> Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>
> 
> -- 
> - Andrey
> 
> 

Re: [PATCH 3/6] xfs/122: adapt to fsverity

[Darrick J. Wong]

On Tue, Apr 30, 2024 at 02:45:29PM +0200, Andrey Albershteyn wrote:
> On 2024-04-29 20:41:34, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Add fields for fsverity ondisk structures.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/xfs/122.out |    2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > 
> > diff --git a/tests/xfs/122.out b/tests/xfs/122.out
> > index 019fe7545f..22f36c0311 100644
> > --- a/tests/xfs/122.out
> > +++ b/tests/xfs/122.out
> > @@ -65,6 +65,7 @@ sizeof(struct xfs_agfl) = 36
> >  sizeof(struct xfs_attr3_leaf_hdr) = 80
> >  sizeof(struct xfs_attr3_leafblock) = 88
> >  sizeof(struct xfs_attr3_rmt_hdr) = 56
> > +sizeof(struct xfs_attr3_rmtverity_hdr) = 36
> >  sizeof(struct xfs_attr_sf_entry) = 3
> >  sizeof(struct xfs_attr_sf_hdr) = 4
> >  sizeof(struct xfs_attr_shortform) = 8
> > @@ -120,6 +121,7 @@ sizeof(struct xfs_log_dinode) = 176
> >  sizeof(struct xfs_log_legacy_timestamp) = 8
> >  sizeof(struct xfs_map_extent) = 32
> >  sizeof(struct xfs_map_freesp) = 32
> > +sizeof(struct xfs_merkle_key) = 8
> >  sizeof(struct xfs_parent_rec) = 12
> >  sizeof(struct xfs_phys_extent) = 16
> >  sizeof(struct xfs_refcount_key) = 4
> > 
> > 
> 
> Shouldn't this patch be squashed with previous one?

Actualy, the 122.out change in the previous patch is now wrong and can
go away.  These two changes are still relevant though.

--D

> -- 
> - Andrey
> 
> 

Re: [PATCH 4/6] xfs: test xfs_scrub detection and correction of corrupt fsverity metadata

[Darrick J. Wong]

On Tue, Apr 30, 2024 at 02:29:03PM +0200, Andrey Albershteyn wrote:
> On 2024-04-29 20:41:50, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Create a basic test to ensure that xfs_scrub media scans complain about
> > files that don't pass fsverity validation.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/xfs/1880     |  135 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/1880.out |   37 ++++++++++++++
> >  2 files changed, 172 insertions(+)
> >  create mode 100755 tests/xfs/1880
> >  create mode 100644 tests/xfs/1880.out
> > 
> > 
> > diff --git a/tests/xfs/1880 b/tests/xfs/1880
> > new file mode 100755
> > index 0000000000..a2119f04c2
> > --- /dev/null
> > +++ b/tests/xfs/1880
> > @@ -0,0 +1,135 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2024 Oracle.  All Rights Reserved.
> > +#
> > +# FS QA Test 1880
> > +#
> > +# Corrupt fsverity descriptor, merkle tree blocks, and file contents.  Ensure
> > +# that xfs_scrub detects this and repairs whatever it can.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick verity
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	_restore_fsverity_signatures
> > +	rm -f $tmp.*
> > +}
> > +
> > +. ./common/verity
> > +. ./common/filter
> > +. ./common/fuzzy
> > +
> > +_supported_fs xfs
> > +_require_scratch_verity
> > +_disable_fsverity_signatures
> > +_require_fsverity_corruption
> > +_require_scratch_nocheck	# fsck test
> > +
> > +_scratch_mkfs >> $seqres.full
> > +_scratch_mount
> > +
> > +_require_scratch_xfs_scrub
> > +_require_xfs_has_feature "$SCRATCH_MNT" verity
> > +VICTIM_FILE="$SCRATCH_MNT/a"
> > +_fsv_can_enable "$VICTIM_FILE" || _notrun "cannot enable fsverity"
> 
> I think this is not necessary, _require_scratch_verity already does
> check if verity can be enabled (with more detailed errors).

It is because _require_scratch_verity calls _scratch_mkfs_verity to
format the filesystem.  _scratch_mkfs_verity in turn forces verity on,
possibly overriding MKFS_OPTIONS to make it happen.  -iverity=1 might
not be set for a regular _scratch_mkfs call.

Therefore, this second _fsv_can_enable call checks that the test
runner's MKFS_OPTIONS set actually supports fsverity.

I'll leave a comment summarizing this:

# Check again to confirm that the caller's MKFS_OPTIONS result in a filesystem
# that supports fsverity.
_fsv_can_enable "$VICTIM_FILE" || _notrun "cannot enable fsverity"

--D

> Otherwise, looks good to me:
> Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>
> 
> -- 
> - Andrey
> 
> 

Re: [PATCH 5/6] xfs: test disabling fsverity

[Darrick J. Wong]

On Tue, Apr 30, 2024 at 03:11:11PM +0200, Andrey Albershteyn wrote:
> On 2024-04-29 20:42:05, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Add a test to make sure that we can disable fsverity on a file that
> > doesn't pass fsverity validation on its contents anymore.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/xfs/1881     |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/1881.out |   28 +++++++++++++
> >  2 files changed, 139 insertions(+)
> >  create mode 100755 tests/xfs/1881
> >  create mode 100644 tests/xfs/1881.out
> > 
> > 
> > diff --git a/tests/xfs/1881 b/tests/xfs/1881
> > new file mode 100755
> > index 0000000000..411802d7c7
> > --- /dev/null
> > +++ b/tests/xfs/1881
> > @@ -0,0 +1,111 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2024 Oracle.  All Rights Reserved.
> > +#
> > +# FS QA Test 1881
> > +#
> > +# Corrupt fsverity descriptor, merkle tree blocks, and file contents.  Ensure
> > +# that we can still disable fsverity, at least for the latter cases.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick verity
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	_restore_fsverity_signatures
> > +	rm -f $tmp.*
> > +}
> > +
> > +. ./common/verity
> > +. ./common/filter
> > +. ./common/fuzzy
> > +
> > +_supported_fs xfs
> > +_require_scratch_verity
> > +_disable_fsverity_signatures
> > +_require_fsverity_corruption
> > +_require_xfs_io_command noverity
> > +_require_scratch_nocheck	# corruption test
> > +
> > +_scratch_mkfs >> $seqres.full
> > +_scratch_mount
> > +
> > +_require_xfs_has_feature "$SCRATCH_MNT" verity
> > +VICTIM_FILE="$SCRATCH_MNT/a"
> > +_fsv_can_enable "$VICTIM_FILE" || _notrun "cannot enable fsverity"
> > +
> > +create_victim()
> > +{
> > +	local filesize="${1:-3}"
> > +
> > +	rm -f "$VICTIM_FILE"
> > +	perl -e "print 'moo' x $((filesize / 3))" > "$VICTIM_FILE"
> > +	fsverity enable --hash-alg=sha256 --block-size=1024 "$VICTIM_FILE"
> > +	fsverity measure "$VICTIM_FILE" | _filter_scratch
> > +}
> > +
> > +disable_verity() {
> > +	$XFS_IO_PROG -r -c 'noverity' "$VICTIM_FILE" 2>&1 | _filter_scratch
> > +}
> > +
> > +cat_victim() {
> > +	$XFS_IO_PROG -r -c 'pread -q 0 4096' "$VICTIM_FILE" 2>&1 | _filter_scratch
> > +}
> > +
> > +echo "Part 1: Delete the fsverity descriptor" | tee -a $seqres.full
> > +create_victim
> > +_scratch_unmount
> > +_scratch_xfs_db -x -c "path /a" -c "attr_remove -f vdesc" -c 'ablock 0' -c print >> $seqres.full
> > +_scratch_mount
> > +cat_victim
> > +
> > +echo "Part 2: Disable fsverity, which won't work" | tee -a $seqres.full
> > +disable_verity
> > +cat_victim
> > +
> > +echo "Part 3: Corrupt the fsverity descriptor" | tee -a $seqres.full
> > +create_victim
> > +_scratch_unmount
> > +_scratch_xfs_db -x -c "path /a" -c 'attr_modify -f "vdesc" -o 0 "BUGSAHOY"' -c 'ablock 0' -c print >> $seqres.full
> > +_scratch_mount
> > +cat_victim
> > +
> > +echo "Part 4: Disable fsverity, which won't work" | tee -a $seqres.full
> > +disable_verity
> > +cat_victim
> > +
> > +echo "Part 5: Corrupt the fsverity file data" | tee -a $seqres.full
> > +create_victim
> > +_scratch_unmount
> > +_scratch_xfs_db -x -c "path /a" -c 'dblock 0' -c 'blocktrash -3 -o 0 -x 24 -y 24 -z' -c print >> $seqres.full
> > +_scratch_mount
> > +cat_victim
> > +
> > +echo "Part 6: Disable fsverity, which should work" | tee -a $seqres.full
> > +disable_verity
> > +cat_victim
> > +
> > +echo "Part 7: Corrupt a merkle tree block" | tee -a $seqres.full
> > +create_victim 1234 # two merkle tree blocks
> > +_fsv_scratch_corrupt_merkle_tree "$VICTIM_FILE" 0
> 
> hmm, _fsv_scratch_corrupt_merkle_tree calls _scratch_xfs_repair, and
> now with xfs_repair knowing about fs-verity is probably a problem. I

It shouldn't be -- xfs_repair doesn't check the contents of the merkle
tree itself.

(xfs_scrub sort of does, but only by calling out to the kernel fsverity
code to get rough tree geometry and calling MADV_POPULATE_READ to
exercise the read validation.)

> don't remember what was the problem with quota (why xfs_repiar is
> there), I can check it.

If the attr_modify commandline changes the block count of the file, it
won't update the quota accounting information.  That can happen if the
dabtree changes shape, or if the new attr requires the creation of a new
attr leaf block, or if the remote value block count changes due to
changes in the size of the attr value.

--D

> > +cat_victim
> > +
> > +echo "Part 8: Disable fsverity, which should work" | tee -a $seqres.full
> > +disable_verity
> > +cat_victim
> > +
> > +echo "Part 9: Corrupt the fsverity salt" | tee -a $seqres.full
> > +create_victim
> > +_scratch_unmount
> > +_scratch_xfs_db -x -c "path /a" -c 'attr_modify -f "vdesc" -o 3 #08' -c 'attr_modify -f "vdesc" -o 80 "BUGSAHOY"' -c 'ablock 0' -c print >> $seqres.full
> > +_scratch_mount
> > +cat_victim
> > +
> > +echo "Part 10: Disable fsverity, which should work" | tee -a $seqres.full
> > +disable_verity
> > +cat_victim
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/xfs/1881.out b/tests/xfs/1881.out
> > new file mode 100644
> > index 0000000000..3e94b8001e
> > --- /dev/null
> > +++ b/tests/xfs/1881.out
> > @@ -0,0 +1,28 @@
> > +QA output created by 1881
> > +Part 1: Delete the fsverity descriptor
> > +sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
> > +SCRATCH_MNT/a: Invalid argument
> > +Part 2: Disable fsverity, which won't work
> > +SCRATCH_MNT/a: Invalid argument
> > +SCRATCH_MNT/a: Invalid argument
> > +Part 3: Corrupt the fsverity descriptor
> > +sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
> > +SCRATCH_MNT/a: Invalid argument
> > +Part 4: Disable fsverity, which won't work
> > +SCRATCH_MNT/a: Invalid argument
> > +SCRATCH_MNT/a: Invalid argument
> > +Part 5: Corrupt the fsverity file data
> > +sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
> > +pread: Input/output error
> > +Part 6: Disable fsverity, which should work
> > +pread: Input/output error
> > +Part 7: Corrupt a merkle tree block
> > +sha256:c56f1115966bafa6c9d32b4717f554b304161f33923c9292c7a92a27866a853c SCRATCH_MNT/a
> > +pread: Input/output error
> > +Part 8: Disable fsverity, which should work
> > +pread: Input/output error
> > +Part 9: Corrupt the fsverity salt
> > +sha256:bab5cfebae30d53e4318629d4ba0b4760d6aae38e03ae235741ed69a31873f1f SCRATCH_MNT/a
> > +pread: Input/output error
> > +Part 10: Disable fsverity, which should work
> > +pread: Input/output error
> > 
> 
> -- 
> - Andrey
> 
> 

Re: [PATCH 6/6] common/populate: add verity files to populate xfs images

[Darrick J. Wong]

On Tue, Apr 30, 2024 at 03:22:50PM +0200, Andrey Albershteyn wrote:
> On 2024-04-29 20:42:21, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > If verity is enabled on a filesystem, we should create some sample
> > verity files.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  common/populate |   24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > 
> > diff --git a/common/populate b/common/populate
> > index 35071f4210..ab9495e739 100644
> > --- a/common/populate
> > +++ b/common/populate
> > @@ -520,6 +520,30 @@ _scratch_xfs_populate() {
> >  		done
> >  	fi
> >  
> > +	# verity merkle trees
> > +	is_verity="$(_xfs_has_feature "$SCRATCH_MNT" verity -v)"
> > +	if [ $is_verity -gt 0 ]; then
> > +		echo "+ fsverity"
> > +
> > +		# Create a biggish file with all zeroes, because metadump
> > +		# won't preserve data blocks and we don't want the hashes to
> > +		# stop working for our sample fs.
> 
> Hashes of the data blocks in the merkle tree? All zeros to use
> .zero_digest in fs-verity? Not sure if got this comment right

Oooh, yeah, I need to go check that.  The block elision code might be
neutralizing this.

--D

> > +		for ((pos = 0, i = 88; pos < 23456789; pos += 234567, i++)); do
> > +			$XFS_IO_PROG -f -c "pwrite -S 0 $pos 234567" "$SCRATCH_MNT/verity"
> > +		done
> > +
> > +		fsverity enable "$SCRATCH_MNT/verity"
> > +
> > +		# Create a sparse file
> > +		$XFS_IO_PROG -f -c "pwrite -S 0 0 3" -c "pwrite -S 0 23456789 3" "$SCRATCH_MNT/sparse_verity"
> > +		fsverity enable "$SCRATCH_MNT/sparse_verity"
> > +
> > +		# Create a salted sparse file
> > +		$XFS_IO_PROG -f -c "pwrite -S 0 0 3" -c "pwrite -S 0 23456789 3" "$SCRATCH_MNT/salted_verity"
> > +		local salt="5846532066696e616c6c7920686173206461746120636865636b73756d732121"	# XFS finally has data checksums!!
> > +		fsverity enable --salt="$salt" "$SCRATCH_MNT/salted_verity"
> > +	fi
> > +
> >  	# Copy some real files (xfs tests, I guess...)
> >  	echo "+ real files"
> >  	test $fill -ne 0 && __populate_fill_fs "${SCRATCH_MNT}" 5
> > 
> 
> -- 
> - Andrey
> 
> 

Re: [PATCH 5/6] xfs: test disabling fsverity

[Andrey Albershteyn]

On 2024-04-30 08:48:10, Darrick J. Wong wrote:
> On Tue, Apr 30, 2024 at 03:11:11PM +0200, Andrey Albershteyn wrote:
> > On 2024-04-29 20:42:05, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > Add a test to make sure that we can disable fsverity on a file that
> > > doesn't pass fsverity validation on its contents anymore.
> > > 
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > >  tests/xfs/1881     |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/xfs/1881.out |   28 +++++++++++++
> > >  2 files changed, 139 insertions(+)
> > >  create mode 100755 tests/xfs/1881
> > >  create mode 100644 tests/xfs/1881.out
> > > 
> > > 
> > > diff --git a/tests/xfs/1881 b/tests/xfs/1881
> > > new file mode 100755
> > > index 0000000000..411802d7c7
> > > --- /dev/null
> > > +++ b/tests/xfs/1881
> > > @@ -0,0 +1,111 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2024 Oracle.  All Rights Reserved.
> > > +#
> > > +# FS QA Test 1881
> > > +#
> > > +# Corrupt fsverity descriptor, merkle tree blocks, and file contents.  Ensure
> > > +# that we can still disable fsverity, at least for the latter cases.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto quick verity
> > > +
> > > +_cleanup()
> > > +{
> > > +	cd /
> > > +	_restore_fsverity_signatures
> > > +	rm -f $tmp.*
> > > +}
> > > +
> > > +. ./common/verity
> > > +. ./common/filter
> > > +. ./common/fuzzy
> > > +
> > > +_supported_fs xfs
> > > +_require_scratch_verity
> > > +_disable_fsverity_signatures
> > > +_require_fsverity_corruption
> > > +_require_xfs_io_command noverity
> > > +_require_scratch_nocheck	# corruption test
> > > +
> > > +_scratch_mkfs >> $seqres.full
> > > +_scratch_mount
> > > +
> > > +_require_xfs_has_feature "$SCRATCH_MNT" verity
> > > +VICTIM_FILE="$SCRATCH_MNT/a"
> > > +_fsv_can_enable "$VICTIM_FILE" || _notrun "cannot enable fsverity"
> > > +
> > > +create_victim()
> > > +{
> > > +	local filesize="${1:-3}"
> > > +
> > > +	rm -f "$VICTIM_FILE"
> > > +	perl -e "print 'moo' x $((filesize / 3))" > "$VICTIM_FILE"
> > > +	fsverity enable --hash-alg=sha256 --block-size=1024 "$VICTIM_FILE"
> > > +	fsverity measure "$VICTIM_FILE" | _filter_scratch
> > > +}
> > > +
> > > +disable_verity() {
> > > +	$XFS_IO_PROG -r -c 'noverity' "$VICTIM_FILE" 2>&1 | _filter_scratch
> > > +}
> > > +
> > > +cat_victim() {
> > > +	$XFS_IO_PROG -r -c 'pread -q 0 4096' "$VICTIM_FILE" 2>&1 | _filter_scratch
> > > +}
> > > +
> > > +echo "Part 1: Delete the fsverity descriptor" | tee -a $seqres.full
> > > +create_victim
> > > +_scratch_unmount
> > > +_scratch_xfs_db -x -c "path /a" -c "attr_remove -f vdesc" -c 'ablock 0' -c print >> $seqres.full
> > > +_scratch_mount
> > > +cat_victim
> > > +
> > > +echo "Part 2: Disable fsverity, which won't work" | tee -a $seqres.full
> > > +disable_verity
> > > +cat_victim
> > > +
> > > +echo "Part 3: Corrupt the fsverity descriptor" | tee -a $seqres.full
> > > +create_victim
> > > +_scratch_unmount
> > > +_scratch_xfs_db -x -c "path /a" -c 'attr_modify -f "vdesc" -o 0 "BUGSAHOY"' -c 'ablock 0' -c print >> $seqres.full
> > > +_scratch_mount
> > > +cat_victim
> > > +
> > > +echo "Part 4: Disable fsverity, which won't work" | tee -a $seqres.full
> > > +disable_verity
> > > +cat_victim
> > > +
> > > +echo "Part 5: Corrupt the fsverity file data" | tee -a $seqres.full
> > > +create_victim
> > > +_scratch_unmount
> > > +_scratch_xfs_db -x -c "path /a" -c 'dblock 0' -c 'blocktrash -3 -o 0 -x 24 -y 24 -z' -c print >> $seqres.full
> > > +_scratch_mount
> > > +cat_victim
> > > +
> > > +echo "Part 6: Disable fsverity, which should work" | tee -a $seqres.full
> > > +disable_verity
> > > +cat_victim
> > > +
> > > +echo "Part 7: Corrupt a merkle tree block" | tee -a $seqres.full
> > > +create_victim 1234 # two merkle tree blocks
> > > +_fsv_scratch_corrupt_merkle_tree "$VICTIM_FILE" 0
> > 
> > hmm, _fsv_scratch_corrupt_merkle_tree calls _scratch_xfs_repair, and
> > now with xfs_repair knowing about fs-verity is probably a problem. I
> 
> It shouldn't be -- xfs_repair doesn't check the contents of the merkle
> tree itself.
> 
> (xfs_scrub sort of does, but only by calling out to the kernel fsverity
> code to get rough tree geometry and calling MADV_POPULATE_READ to
> exercise the read validation.)

oh right, it's xfs_scrub, I meant re-reading file validation

> 
> > don't remember what was the problem with quota (why xfs_repiar is
> > there), I can check it.
> 
> If the attr_modify commandline changes the block count of the file, it
> won't update the quota accounting information.  That can happen if the
> dabtree changes shape, or if the new attr requires the creation of a new
> attr leaf block, or if the remote value block count changes due to
> changes in the size of the attr value.

aha, yeah

-- 
- Andrey

Re: [PATCHSET v5.6] fstests: fs-verity support for XFS

[Zorro Lang]

Hi Darrick,

Due to only half of this patchset got reviewed, so I'd like to wait for your
later version. I won't pick up part of this patchset to merge this time, I
think better to merge it as an integrated patchset.

Thanks,
Zorro

On Mon, Apr 29, 2024 at 08:19:24PM -0700, Darrick J. Wong wrote:
> Hi all,
> 
> This patchset adds support for fsverity to XFS.  In keeping with
> Andrey's original design, XFS stores all fsverity metadata in the
> extended attribute data.  However, I've made a few changes to the code:
> First, it now caches merkle tree blocks directly instead of abusing the
> buffer cache.  This reduces lookup overhead quite a bit, at a cost of
> needing a new shrinker for cached merkle tree blocks.
> 
> To reduce the ondisk footprint further, I also made the verity
> enablement code detect trailing zeroes whenever fsverity tells us to
> write a buffer, and elide storing the zeroes.  To further reduce the
> footprint of sparse files, I also skip writing merkle tree blocks if the
> block contents are entirely hashes of zeroes.
> 
> Next, I implemented more of the tooling around verity, such as debugger
> support, as much fsck support as I can manage without knowing the
> internal format of the fsverity information; and added support for
> xfs_scrub to read fsverity files to validate the consistency of the data
> against the merkle tree.
> 
> Finally, I add the ability for administrators to turn off fsverity,
> which might help recovering damaged data from an inconsistent file.
> 
> From Andrey Albershteyn:
> 
> Here's v5 of my patchset of adding fs-verity support to XFS.
> 
> This implementation uses extended attributes to store fs-verity
> metadata. The Merkle tree blocks are stored in the remote extended
> attributes. The names are offsets into the tree.
> From Darrick J. Wong:
> 
> This v5.3 patchset builds upon v5.2 of Andrey's patchset to implement
> fsverity for XFS.
> 
> The biggest thing that I didn't like in the v5 patchset is the abuse of
> the data device's buffer cache to store the incore version of the merkle
> tree blocks.  Not only do verity state flags end up in xfs_buf, but the
> double-alloc flag wastes memory and doesn't remain internally consistent
> if the xattrs shift around.
> 
> I replaced all of that with a per-inode xarray that indexes incore
> merkle tree blocks.  For cache hits, this dramatically reduces the
> amount of work that xfs has to do to feed fsverity.  The per-block
> overhead is much lower (8 bytes instead of ~300 for xfs_bufs), and we no
> longer have to entertain layering violations in the buffer cache.  I
> also added a per-filesystem shrinker so that reclaim can cull cached
> merkle tree blocks, starting with the leaf tree nodes.
> 
> I've also rolled in some changes recommended by the fsverity maintainer,
> fixed some organization and naming problems in the xfs code, fixed a
> collision in the xfs_inode iflags, and improved dead merkle tree cleanup
> per the discussion of the v5 series.  At this point I'm happy enough
> with this code to start integrating and testing it in my trees, so it's
> time to send it out a coherent patchset for comments.
> 
> For v5.3, I've added bits and pieces of online and offline repair
> support, reduced the size of partially filled merkle tree blocks by
> removing trailing zeroes, changed the xattr hash function to better
> avoid collisions between merkle tree keys, made the fsverity
> invalidation bitmap unnecessary, and made it so that we can save space
> on sparse verity files by not storing merkle tree blocks that hash
> totally zeroed data blocks.
> 
> From Andrey Albershteyn:
> 
> Here's v5 of my patchset of adding fs-verity support to XFS.
> 
> This implementation uses extended attributes to store fs-verity
> metadata. The Merkle tree blocks are stored in the remote extended
> attributes. The names are offsets into the tree.
> 
> If you're going to start using this code, I strongly recommend pulling
> from my git trees, which are linked below.
> 
> This has been running on the djcloud for months with no problems.  Enjoy!
> Comments and questions are, as always, welcome.
> 
> --D
> 
> kernel git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=fsverity
> 
> xfsprogs git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=fsverity
> 
> fstests git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=fsverity
> ---
> Commits in this patchset:
>  * common/verity: enable fsverity for XFS
>  * xfs/{021,122}: adapt to fsverity xattrs
>  * xfs/122: adapt to fsverity
>  * xfs: test xfs_scrub detection and correction of corrupt fsverity metadata
>  * xfs: test disabling fsverity
>  * common/populate: add verity files to populate xfs images
> ---
>  common/populate    |   24 +++++++++
>  common/verity      |   39 ++++++++++++++-
>  tests/xfs/021      |    3 +
>  tests/xfs/122.out  |    3 +
>  tests/xfs/1880     |  135 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/1880.out |   37 ++++++++++++++
>  tests/xfs/1881     |  111 +++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/1881.out |   28 +++++++++++
>  8 files changed, 378 insertions(+), 2 deletions(-)
>  create mode 100755 tests/xfs/1880
>  create mode 100644 tests/xfs/1880.out
>  create mode 100755 tests/xfs/1881
>  create mode 100644 tests/xfs/1881.out
> 

Re: [PATCHSET v5.6] fstests: fs-verity support for XFS

[Darrick J. Wong]

On Sat, May 11, 2024 at 01:01:46PM +0800, Zorro Lang wrote:
> Hi Darrick,
> 
> Due to only half of this patchset got reviewed, so I'd like to wait for your
> later version. I won't pick up part of this patchset to merge this time, I
> think better to merge it as an integrated patchset.

Christoph and I talked about the future of this patchset at LSF and
there are some file format changes in store, so please hold off on
analyzing this patchset for now.

--D

> Thanks,
> Zorro
> 
> On Mon, Apr 29, 2024 at 08:19:24PM -0700, Darrick J. Wong wrote:
> > Hi all,
> > 
> > This patchset adds support for fsverity to XFS.  In keeping with
> > Andrey's original design, XFS stores all fsverity metadata in the
> > extended attribute data.  However, I've made a few changes to the code:
> > First, it now caches merkle tree blocks directly instead of abusing the
> > buffer cache.  This reduces lookup overhead quite a bit, at a cost of
> > needing a new shrinker for cached merkle tree blocks.
> > 
> > To reduce the ondisk footprint further, I also made the verity
> > enablement code detect trailing zeroes whenever fsverity tells us to
> > write a buffer, and elide storing the zeroes.  To further reduce the
> > footprint of sparse files, I also skip writing merkle tree blocks if the
> > block contents are entirely hashes of zeroes.
> > 
> > Next, I implemented more of the tooling around verity, such as debugger
> > support, as much fsck support as I can manage without knowing the
> > internal format of the fsverity information; and added support for
> > xfs_scrub to read fsverity files to validate the consistency of the data
> > against the merkle tree.
> > 
> > Finally, I add the ability for administrators to turn off fsverity,
> > which might help recovering damaged data from an inconsistent file.
> > 
> > From Andrey Albershteyn:
> > 
> > Here's v5 of my patchset of adding fs-verity support to XFS.
> > 
> > This implementation uses extended attributes to store fs-verity
> > metadata. The Merkle tree blocks are stored in the remote extended
> > attributes. The names are offsets into the tree.
> > From Darrick J. Wong:
> > 
> > This v5.3 patchset builds upon v5.2 of Andrey's patchset to implement
> > fsverity for XFS.
> > 
> > The biggest thing that I didn't like in the v5 patchset is the abuse of
> > the data device's buffer cache to store the incore version of the merkle
> > tree blocks.  Not only do verity state flags end up in xfs_buf, but the
> > double-alloc flag wastes memory and doesn't remain internally consistent
> > if the xattrs shift around.
> > 
> > I replaced all of that with a per-inode xarray that indexes incore
> > merkle tree blocks.  For cache hits, this dramatically reduces the
> > amount of work that xfs has to do to feed fsverity.  The per-block
> > overhead is much lower (8 bytes instead of ~300 for xfs_bufs), and we no
> > longer have to entertain layering violations in the buffer cache.  I
> > also added a per-filesystem shrinker so that reclaim can cull cached
> > merkle tree blocks, starting with the leaf tree nodes.
> > 
> > I've also rolled in some changes recommended by the fsverity maintainer,
> > fixed some organization and naming problems in the xfs code, fixed a
> > collision in the xfs_inode iflags, and improved dead merkle tree cleanup
> > per the discussion of the v5 series.  At this point I'm happy enough
> > with this code to start integrating and testing it in my trees, so it's
> > time to send it out a coherent patchset for comments.
> > 
> > For v5.3, I've added bits and pieces of online and offline repair
> > support, reduced the size of partially filled merkle tree blocks by
> > removing trailing zeroes, changed the xattr hash function to better
> > avoid collisions between merkle tree keys, made the fsverity
> > invalidation bitmap unnecessary, and made it so that we can save space
> > on sparse verity files by not storing merkle tree blocks that hash
> > totally zeroed data blocks.
> > 
> > From Andrey Albershteyn:
> > 
> > Here's v5 of my patchset of adding fs-verity support to XFS.
> > 
> > This implementation uses extended attributes to store fs-verity
> > metadata. The Merkle tree blocks are stored in the remote extended
> > attributes. The names are offsets into the tree.
> > 
> > If you're going to start using this code, I strongly recommend pulling
> > from my git trees, which are linked below.
> > 
> > This has been running on the djcloud for months with no problems.  Enjoy!
> > Comments and questions are, as always, welcome.
> > 
> > --D
> > 
> > kernel git tree:
> > https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=fsverity
> > 
> > xfsprogs git tree:
> > https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=fsverity
> > 
> > fstests git tree:
> > https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=fsverity
> > ---
> > Commits in this patchset:
> >  * common/verity: enable fsverity for XFS
> >  * xfs/{021,122}: adapt to fsverity xattrs
> >  * xfs/122: adapt to fsverity
> >  * xfs: test xfs_scrub detection and correction of corrupt fsverity metadata
> >  * xfs: test disabling fsverity
> >  * common/populate: add verity files to populate xfs images
> > ---
> >  common/populate    |   24 +++++++++
> >  common/verity      |   39 ++++++++++++++-
> >  tests/xfs/021      |    3 +
> >  tests/xfs/122.out  |    3 +
> >  tests/xfs/1880     |  135 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/1880.out |   37 ++++++++++++++
> >  tests/xfs/1881     |  111 +++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/1881.out |   28 +++++++++++
> >  8 files changed, 378 insertions(+), 2 deletions(-)
> >  create mode 100755 tests/xfs/1880
> >  create mode 100644 tests/xfs/1880.out
> >  create mode 100755 tests/xfs/1881
> >  create mode 100644 tests/xfs/1881.out
> > 
> 
>