From 973700943721d50952ab0712725e50ce09c34429 Mon Sep 17 00:00:00 2001 From: Auke Kok Date: Wed, 8 Apr 2026 11:37:06 -0700 Subject: [PATCH 1/2] Fix indx delete using wrong xid, leaving orphans. During inode deletion, scoutfs_xattr_drop forgot to set the xid of the xattr after calling parse_indx_key, which hardcodes xid=0, and it is the callers' responsibility. delete_force then deletes the wrong key, and returns no errors on nonexistant keys. So now there is a pending deletion for a non-existant indx and an orphan indx entry in the tree. Subsequent calls to `scoutfs read-xattr-index` will thus return entries for deleted inodes. Signed-off-by: Auke Kok --- kmod/src/xattr.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kmod/src/xattr.c b/kmod/src/xattr.c index 7805b364..d8c468e3 100644 --- a/kmod/src/xattr.c +++ b/kmod/src/xattr.c @@ -1265,6 +1265,7 @@ int scoutfs_xattr_drop(struct super_block *sb, u64 ino, ret = parse_indx_key(&tag_key, xat->name, xat->name_len, ino); if (ret < 0) goto out; + scoutfs_xattr_set_indx_key_xid(&tag_key, le64_to_cpu(key.skx_id)); } if ((tgs.totl || tgs.indx) && locked_zone != tag_key.sk_zone) { From b9c49629a2e486306d667daa5b5f93d5d383494f Mon Sep 17 00:00:00 2001 From: Auke Kok Date: Wed, 8 Apr 2026 11:27:15 -0700 Subject: [PATCH 2/2] Add basic-xattr-indx tests. We had no basic testing for `scoutfs read-xattr-index` whatsoever. This adds your basic negative argument tests, lifecycle tests, the deduplicated reads, and partial removal. This exposes a bug in deletion where the indx entry isn't cleaned up on inode delete. Signed-off-by: Auke Kok --- tests/golden/basic-xattr-indx | 54 ++++++++++++ tests/sequence | 1 + tests/tests/basic-xattr-indx.sh | 143 ++++++++++++++++++++++++++++++++ 3 files changed, 198 insertions(+) create mode 100644 tests/golden/basic-xattr-indx create mode 100644 tests/tests/basic-xattr-indx.sh diff --git a/tests/golden/basic-xattr-indx b/tests/golden/basic-xattr-indx new file mode 100644 index 00000000..59ae7bfe --- /dev/null +++ b/tests/golden/basic-xattr-indx @@ -0,0 +1,54 @@ +== testing invalid read-xattr-index arguments +bad index position entry argument 'bad', it must be in the form "a.b.ino" where each value can be prefixed by '0' for octal or '0x' for hex +scoutfs: read-xattr-index failed: Invalid argument (22) +bad index position entry argument '1.2', it must be in the form "a.b.ino" where each value can be prefixed by '0' for octal or '0x' for hex +scoutfs: read-xattr-index failed: Invalid argument (22) +initial major index position '256' must be between 0 and 255, inclusive. +scoutfs: read-xattr-index failed: Invalid argument (22) +first index position 1.2.3 must be less than last index position 0.0.0 +scoutfs: read-xattr-index failed: Invalid argument (22) +first index position 1.2.0 must be less than last index position 1.1.2 +scoutfs: read-xattr-index failed: Invalid argument (22) +first index position 2.2.2 must be less than last index position 2.2.1 +scoutfs: read-xattr-index failed: Invalid argument (22) +== testing invalid names +setfattr: /mnt/test/test/basic-xattr-indx/invalid: Invalid argument +setfattr: /mnt/test/test/basic-xattr-indx/invalid: Invalid argument +setfattr: /mnt/test/test/basic-xattr-indx/invalid: Invalid argument +setfattr: /mnt/test/test/basic-xattr-indx/invalid: Invalid argument +setfattr: /mnt/test/test/basic-xattr-indx/invalid: Invalid argument +setfattr: /mnt/test/test/basic-xattr-indx/invalid: Invalid argument +setfattr: /mnt/test/test/basic-xattr-indx/invalid: Invalid argument +setfattr: /mnt/test/test/basic-xattr-indx/invalid: Invalid argument +setfattr: /mnt/test/test/basic-xattr-indx/invalid: Invalid argument +setfattr: /mnt/test/test/basic-xattr-indx/invalid: Invalid argument +setfattr: /mnt/test/test/basic-xattr-indx/invalid: Invalid argument +setfattr: /mnt/test/test/basic-xattr-indx/invalid: Numerical result out of range +== testing boundary values +0.0 found +255.max found +== indx xattr must have no value +setfattr: /mnt/test/test/basic-xattr-indx/noval: Invalid argument +setfattr: /mnt/test/test/basic-xattr-indx/noval: Invalid argument +== set indx xattr and verify index entry +found +== setting same indx xattr again is a no-op +found +== removing non-existent indx xattr succeeds +setfattr: /mnt/test/test/basic-xattr-indx/file: No such attribute +still found +== explicit xattr removal cleans up index entry +== file deletion cleans up index entry +found before delete +== multiple indx xattrs on one file cleaned up by deletion +entries before delete: 2 +entries after delete: 0 +== partial removal leaves other entries +300 found +== multiple files at same index position +files at same position: 2 +surviving file found +== cross-mount visibility +found on mount 1 +== duplicate position deduplication +entries for same position: 1 diff --git a/tests/sequence b/tests/sequence index e296553a..0719a05f 100644 --- a/tests/sequence +++ b/tests/sequence @@ -26,6 +26,7 @@ srch-basic-functionality.sh simple-xattr-unit.sh retention-basic.sh totl-xattr-tag.sh +basic-xattr-indx.sh quota.sh lock-refleak.sh lock-shrink-consistency.sh diff --git a/tests/tests/basic-xattr-indx.sh b/tests/tests/basic-xattr-indx.sh new file mode 100644 index 00000000..993147db --- /dev/null +++ b/tests/tests/basic-xattr-indx.sh @@ -0,0 +1,143 @@ +# +# Test basic .indx. xattr tag functionality and index entry lifecycle +# + +t_require_commands touch rm setfattr scoutfs stat +t_require_mounts 2 + +# query index from a specific mount, default mount 0 +read_xattr_index() +{ + local nr="${1:-0}" + local mnt="$(eval echo \$T_M$nr)" + shift + + sync + echo 1 > $(t_debugfs_path $nr)/drop_weak_item_cache + scoutfs read-xattr-index -p "$mnt" "$@" +} + +MAJOR=5 +MINOR=100 + +echo "== testing invalid read-xattr-index arguments" +scoutfs read-xattr-index -p "$T_M0" bad 2>&1 +scoutfs read-xattr-index -p "$T_M0" 1.2 2>&1 +scoutfs read-xattr-index -p "$T_M0" 1.2.3 256.0.0 2>&1 +scoutfs read-xattr-index -p "$T_M0" 1.2.3 0.0.0 2>&1 +scoutfs read-xattr-index -p "$T_M0" 1.2.0 1.1.2 2>&1 +scoutfs read-xattr-index -p "$T_M0" 2.2.2 2.2.1 2>&1 + +echo "== testing invalid names" +touch "$T_D0/invalid" +setfattr -n scoutfs.hide.indx.test.$MAJOR "$T_D0/invalid" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.test "$T_D0/invalid" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.test.. "$T_D0/invalid" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.test..$MINOR "$T_D0/invalid" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.test.$MAJOR. "$T_D0/invalid" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.test.256.$MINOR "$T_D0/invalid" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.test.abc.$MINOR "$T_D0/invalid" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.test.$MAJOR.abc "$T_D0/invalid" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.test.-1.$MINOR "$T_D0/invalid" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.test.$MAJOR.-1 "$T_D0/invalid" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.test.18446744073709551616.$MINOR "$T_D0/invalid" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.$(printf 'x%.0s' $(seq 1 240)).$MAJOR.$MINOR "$T_D0/invalid" 2>&1 | t_filter_fs +rm -f "$T_D0/invalid" + +echo "== testing boundary values" +touch "$T_D0/boundary" +INO=$(stat -c "%i" "$T_D0/boundary") +setfattr -n scoutfs.hide.indx.test.0.0 "$T_D0/boundary" +read_xattr_index 0 0.0.0 0.0.-1 | awk '($3 == "'$INO'") {print "0.0 found"}' +setfattr -x scoutfs.hide.indx.test.0.0 "$T_D0/boundary" +setfattr -n scoutfs.hide.indx.test.255.18446744073709551615 "$T_D0/boundary" +read_xattr_index 0 255.0.0 255.-1.-1 | awk '($3 == "'$INO'") {print "255.max found"}' +setfattr -x scoutfs.hide.indx.test.255.18446744073709551615 "$T_D0/boundary" +rm -f "$T_D0/boundary" + +echo "== indx xattr must have no value" +touch "$T_D0/noval" +setfattr -n scoutfs.hide.indx.test.$MAJOR.$MINOR -v "" "$T_D0/noval" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.test.$MAJOR.$MINOR -v 0 "$T_D0/noval" 2>&1 | t_filter_fs +setfattr -n scoutfs.hide.indx.test.$MAJOR.$MINOR -v 1 "$T_D0/noval" 2>&1 | t_filter_fs +rm -f "$T_D0/noval" + +echo "== set indx xattr and verify index entry" +touch "$T_D0/file" +INO=$(stat -c "%i" "$T_D0/file") +setfattr -n scoutfs.hide.indx.test.$MAJOR.$MINOR "$T_D0/file" +read_xattr_index 0 $MAJOR.0.0 $MAJOR.-1.-1 | awk '($3 == "'$INO'") {print "found"}' + +echo "== setting same indx xattr again is a no-op" +setfattr -n scoutfs.hide.indx.test.$MAJOR.$MINOR "$T_D0/file" +read_xattr_index 0 $MAJOR.0.0 $MAJOR.-1.-1 | awk '($3 == "'$INO'") {print "found"}' + +echo "== removing non-existent indx xattr succeeds" +setfattr -x scoutfs.hide.indx.nonexistent.$MAJOR.999 "$T_D0/file" 2>&1 | t_filter_fs +read_xattr_index 0 $MAJOR.0.0 $MAJOR.-1.-1 | awk '($3 == "'$INO'") {print "still found"}' + +echo "== explicit xattr removal cleans up index entry" +setfattr -x scoutfs.hide.indx.test.$MAJOR.$MINOR "$T_D0/file" +read_xattr_index 0 $MAJOR.0.0 $MAJOR.-1.-1 | awk '($3 == "'$INO'") {print "found orphan"}' +rm -f "$T_D0/file" + +echo "== file deletion cleans up index entry" +touch "$T_D0/file2" +INO=$(stat -c "%i" "$T_D0/file2") +setfattr -n scoutfs.hide.indx.test.$MAJOR.$MINOR "$T_D0/file2" +read_xattr_index 0 $MAJOR.0.0 $MAJOR.-1.-1 | awk '($3 == "'$INO'") {print "found before delete"}' +rm -f "$T_D0/file2" +read_xattr_index 0 $MAJOR.0.0 $MAJOR.-1.-1 | awk '($3 == "'$INO'") {print "found orphan after delete"}' + +echo "== multiple indx xattrs on one file cleaned up by deletion" +touch "$T_D0/file3" +INO=$(stat -c "%i" "$T_D0/file3") +setfattr -n scoutfs.hide.indx.a.$MAJOR.200 "$T_D0/file3" +setfattr -n scoutfs.hide.indx.b.$MAJOR.300 "$T_D0/file3" +BEFORE=$(read_xattr_index 0 $MAJOR.0.0 $MAJOR.-1.-1 | awk '($3 == "'$INO'")' | wc -l) +echo "entries before delete: $BEFORE" +rm -f "$T_D0/file3" +AFTER=$(read_xattr_index 0 $MAJOR.0.0 $MAJOR.-1.-1 | awk '($3 == "'$INO'")' | wc -l) +echo "entries after delete: $AFTER" + +echo "== partial removal leaves other entries" +touch "$T_D0/partial" +INO=$(stat -c "%i" "$T_D0/partial") +setfattr -n scoutfs.hide.indx.a.$MAJOR.200 "$T_D0/partial" +setfattr -n scoutfs.hide.indx.b.$MAJOR.300 "$T_D0/partial" +setfattr -x scoutfs.hide.indx.a.$MAJOR.200 "$T_D0/partial" +read_xattr_index 0 $MAJOR.200.0 $MAJOR.200.-1 | awk '($3 == "'$INO'") {print "200 found"}' +read_xattr_index 0 $MAJOR.300.0 $MAJOR.300.-1 | awk '($3 == "'$INO'") {print "300 found"}' +rm -f "$T_D0/partial" + +echo "== multiple files at same index position" +touch "$T_D0/multi_a" "$T_D0/multi_b" +INO_A=$(stat -c "%i" "$T_D0/multi_a") +INO_B=$(stat -c "%i" "$T_D0/multi_b") +setfattr -n scoutfs.hide.indx.test.$MAJOR.$MINOR "$T_D0/multi_a" +setfattr -n scoutfs.hide.indx.test.$MAJOR.$MINOR "$T_D0/multi_b" +COUNT=$(read_xattr_index 0 $MAJOR.$MINOR.0 $MAJOR.$MINOR.-1 | wc -l) +echo "files at same position: $COUNT" +rm -f "$T_D0/multi_a" +read_xattr_index 0 $MAJOR.$MINOR.0 $MAJOR.$MINOR.-1 | awk '($3 == "'$INO_A'") {print "deleted file still found"}' +read_xattr_index 0 $MAJOR.$MINOR.0 $MAJOR.$MINOR.-1 | awk '($3 == "'$INO_B'") {print "surviving file found"}' +rm -f "$T_D0/multi_b" + +echo "== cross-mount visibility" +touch "$T_D0/file4" +INO=$(stat -c "%i" "$T_D0/file4") +setfattr -n scoutfs.hide.indx.test.$MAJOR.$MINOR "$T_D0/file4" +read_xattr_index 1 $MAJOR.0.0 $MAJOR.-1.-1 | awk '($3 == "'$INO'") {print "found on mount 1"}' +rm -f "$T_D0/file4" +read_xattr_index 1 $MAJOR.0.0 $MAJOR.-1.-1 | awk '($3 == "'$INO'") {print "found orphan on mount 1"}' + +echo "== duplicate position deduplication" +touch "$T_D0/file5" +INO=$(stat -c "%i" "$T_D0/file5") +setfattr -n scoutfs.hide.indx.aa.$MAJOR.$MINOR "$T_D0/file5" +setfattr -n scoutfs.hide.indx.bb.$MAJOR.$MINOR "$T_D0/file5" +COUNT=$(read_xattr_index 0 $MAJOR.0.0 $MAJOR.-1.-1 | awk '($3 == "'$INO'")' | wc -l) +echo "entries for same position: $COUNT" +rm -f "$T_D0/file5" + +t_pass