[PATCH] ext2: reject xattr value overlapping the entry area

[PATCH] ext2: reject xattr value overlapping the entry area

[Guanglei Zhu]

ext2_xattr_entry_valid() checks that a value ends within the block but
not that it starts after the entry area, so a corrupted block can place
a value inside the entries.  ext2_xattr_set() then underflows the free
space calculation:

	free = min_offs - ((char*)last - (char*)header) - sizeof(__u32);

and writes the new value before the start of the block:

	val = (char *)header + min_offs - size;

Check that the value area does not overlap the entry area and refuse
such a block instead.

Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
A crafted ext2 image with an xattr entry whose e_value_offs lies inside
the entry area triggers, on a KASAN build, a use-after-free report in
ext2_xattr_set().  With this patch, setxattr() on such an image returns
-EIO instead.

 fs/ext2/xattr.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 9b68c490a..2b89b758e 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -485,6 +485,14 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
 		if (not_found > 0)
 			here = last;
 
+		/*
+		 * The value area must not overlap the entry area, otherwise
+		 * the space calculation below underflows and the new value
+		 * would be written before the start of the block.
+		 */
+		if (min_offs < (size_t)((char *)last - (char *)header) + sizeof(__u32))
+			goto bad_block;
+
 		/* Check whether we have enough space left. */
 		free = min_offs - ((char*)last - (char*)header) - sizeof(__u32);
 	} else {
-- 
2.43.0

Re: [PATCH] ext2: reject xattr value overlapping the entry area

[sashiko-bot]

> ext2_xattr_entry_valid() checks that a value ends within the block but
> not that it starts after the entry area, so a corrupted block can place
> a value inside the entries.  ext2_xattr_set() then underflows the free
> space calculation:
> 
> 	free = min_offs - ((char*)last - (char*)header) - sizeof(__u32);
> 
> and writes the new value before the start of the block:
> 
> 	val = (char *)header + min_offs - size;
> 
> Check that the value area does not overlap the entry area and refuse
> such a block instead.
> 
> Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915064453.249419-1-zhugl3@xiaopeng.com?part=1