Merge pull request #56 from versity/zab/xattr_shrink_bad_items

Fix xattr update out of bounds access

kmod/src/xattr.c

@@ -368,7 +368,7 @@ static int change_xattr_items(struct inode *inode, u64 id,
 	}
 
 	/* update dirtied overlapping existing items, last partial first */
-	for (i = old_parts - 1; i >= 0; i--) {
+	for (i = min(old_parts, new_parts) - 1; i >= 0; i--) {
 		off = i * SCOUTFS_XATTR_MAX_PART_SIZE;
 		bytes = min_t(unsigned int, new_bytes - off,
 			      SCOUTFS_XATTR_MAX_PART_SIZE);

tests/golden/simple-xattr-unit

@@ -16,3 +16,4 @@ setfattr: /mnt/test/test/simple-xattr-unit/file: Numerical result out of range
 setfattr: /mnt/test/test/simple-xattr-unit/file: Argument list too long
 === good length boundaries
 === 500 random lengths
+=== alternate val size between interesting sizes

tests/tests/simple-xattr-unit.sh

@@ -46,6 +46,35 @@ print_and_run() {
 	"$@" || echo "returned nonzero status: $?"
 }
 
+# fill a buffer with strings that identify their byte offset
+offs=""
+for o in $(seq 0 7 $((65535 - 7))); do
+	offs+="$(printf "[%5u]" $o)"
+done
+
+change_val_sizes() {
+	local name="$1"
+	local file="$2"
+	local from="$3"
+	local to="$4"
+
+	while : ; do
+		setfattr -x "$name" "$file" > /dev/null 2>&1
+		setfattr -n "$name" -v "${offs:0:$from}" "$file"
+		setfattr -n "$name" -v "${offs:0:$to}" "$file"
+		if ! diff -u <(echo -n "${offs:0:$to}") <(getfattr --absolute-names --only-values -n "$name" $file) ; then
+			echo "setting $name from $from to $to failed"
+		fi
+
+		if [ $from == $3 ]; then
+			from=$4
+			to=$3
+		else
+			break
+		fi
+	done
+}
+
 echo "=== XATTR_ flag combinations"
 touch "$FILE"
 print_and_run dumb_setxattr -p "$FILE" -n user.test -v val -c -r
@@ -80,4 +109,17 @@ for i in $(seq 1 $NR); do
 	test_xattr_lengths $name_len $val_len
 done
 
+echo "=== alternate val size between interesting sizes"
+name="user.test"
+ITEM=896
+HDR=$((8 + 9))
+# one full item apart
+change_val_sizes $name "$FILE" $(((ITEM * 2) - HDR)) $(((ITEM * 3) - HDR))
+# multiple full items apart
+change_val_sizes $name "$FILE" $(((ITEM * 6) - HDR)) $(((ITEM * 9) - HDR))
+# item boundary fence posts
+change_val_sizes $name "$FILE" $(((ITEM * 5) - HDR - 1)) $(((ITEM * 13) - HDR + 1))
+# min and max
+change_val_sizes $name "$FILE" 1 65535
+
 t_pass