[PATCH] ext4: xattr: fix out-of-bounds access in ext4_xattr_set_entry

[ZhengYuan Huang <gality369@gmail.com>]

[BUG]
KASAN reports show out-of-bounds and use-after-free memory accesses when
ext4_xattr_set_entry() processes corrupted on-disk xattr entries:

  BUG: KASAN: slab-out-of-bounds in ext4_xattr_set_entry+0xfc2/0x1f40 fs/ext4/xattr.c:1735
  Write of size 12 at addr ffff88801a249af4 [slab OOB]
  Call Trace:
    ...
    ext4_xattr_set_entry+0xfc2/0x1f40 fs/ext4/xattr.c:1735
    ext4_xattr_ibody_set+0x396/0x5a0 fs/ext4/xattr.c:2268
    ext4_destroy_inline_data_nolock+0x25e/0x560 fs/ext4/inline.c:463
    ext4_convert_inline_data_nolock+0x186/0xa80 fs/ext4/inline.c:1105
    ext4_try_add_inline_entry+0x58e/0x960 fs/ext4/inline.c:1224
    ext4_add_entry+0x6d2/0xce0 fs/ext4/namei.c:2389
    ext4_rename+0x133c/0x2490 fs/ext4/namei.c:3929
    ext4_rename2+0x1de/0x2c0 fs/ext4/namei.c:4208
    vfs_rename+0xd42/0x1d50 fs/namei.c:5216
    do_renameat2+0x715/0xb60 fs/namei.c:5364
    ...

  BUG: KASAN: use-after-free in ext4_xattr_set_entry+0xfd3/0x1f40 fs/ext4/xattr.c:1736
  Write of size 65796 at addr ffff88802feb6ee8 [UAF across page boundary]

[CAUSE]
During inode load, xattr_check_inode() validates the ibody xattr entries
found in the inode at that time, and ext4_read_inode_extra() sets
EXT4_STATE_XATTR after the validation succeeds.

Later, when updating an ibody xattr, ext4_xattr_ibody_find() does not rely
on those already validated contents. It calls ext4_get_inode_loc() and
reads the inode table block again, so the entry eventually passed to
ext4_xattr_set_entry() comes from a new on-disk read. xattr_find_entry()
may return that entry based on its name, but does not revalidate its
e_value_offs and e_value_size before they are dereferenced.

Therefore, if the inode table block is modified between inode load and the
later xattr update, the code ends up validating one version of the xattr
data and using another. ext4_xattr_set_entry() may then consume corrupted
e_value_offs/e_value_size fields from the newly read entry, which can cause
out-of-bounds accesses, size_t underflow, and use-after-free.

[FIX]
Fix this by validating the target entry's value offset and size in
ext4_xattr_set_entry() before using them. Reject invalid entries
with -EFSCORRUPTED, consistent with the checks already enforced by
check_xattrs() for ibody xattrs.

Fixes: dec214d00e0d7 ("ext4: xattr inode deduplication")
Cc: stable@vger.kernel.org
Signed-off-by: ZhengYuan Huang <gality369@gmail.com>
---
 fs/ext4/xattr.c | 58 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index ce7253b3f549..3ebfe2dfcae9 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1638,6 +1638,48 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
 			EXT4_XATTR_SIZE(le32_to_cpu(here->e_value_size)) : 0;
 	new_size = (i->value && !in_inode) ? EXT4_XATTR_SIZE(i->value_len) : 0;
 
+	/* Compute min_offs and last. */
+	last = s->first;
+	for (; !IS_LAST_ENTRY(last); last = next) {
+		next = EXT4_XATTR_NEXT(last);
+		if ((void *)next >= s->end) {
+			EXT4_ERROR_INODE(inode, "corrupted xattr entries");
+			ret = -EFSCORRUPTED;
+			goto out;
+		}
+		if (!last->e_value_inum && last->e_value_size) {
+			size_t offs = le16_to_cpu(last->e_value_offs);
+
+			if (offs < min_offs)
+				min_offs = offs;
+		}
+	}
+
+	/*
+	 * Validate the value range before dereferencing e_value_offs / e_value_size.
+	 * This mirrors check_xattrs() for the entry we are about to touch.
+	 */
+	if (!s->not_found && !here->e_value_inum && here->e_value_size) {
+		u16 offs = le16_to_cpu(here->e_value_offs);
+		size_t size = le32_to_cpu(here->e_value_size);
+		void *value;
+
+		if (offs > s->end - s->base) {
+			EXT4_ERROR_INODE(inode, "corrupted xattr entry: invalid value offset");
+			ret = -EFSCORRUPTED;
+			goto out;
+		}
+
+		value = s->base + offs;
+		if (value < (void *)last + sizeof(__u32) ||
+		    size > s->end - value ||
+		    EXT4_XATTR_SIZE(size) > s->end - value) {
+			EXT4_ERROR_INODE(inode, "corrupted xattr entry: invalid value range");
+			ret = -EFSCORRUPTED;
+			goto out;
+		}
+	}
+
 	/*
 	 * Optimization for the simple case when old and new values have the
 	 * same padded sizes. Not applicable if external inodes are involved.
@@ -1657,22 +1699,6 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
 		goto update_hash;
 	}
 
-	/* Compute min_offs and last. */
-	last = s->first;
-	for (; !IS_LAST_ENTRY(last); last = next) {
-		next = EXT4_XATTR_NEXT(last);
-		if ((void *)next >= s->end) {
-			EXT4_ERROR_INODE(inode, "corrupted xattr entries");
-			ret = -EFSCORRUPTED;
-			goto out;
-		}
-		if (!last->e_value_inum && last->e_value_size) {
-			size_t offs = le16_to_cpu(last->e_value_offs);
-			if (offs < min_offs)
-				min_offs = offs;
-		}
-	}
-
 	/* Check whether we have enough space. */
 	if (i->value) {
 		size_t free;
-- 
2.43.0