diff mbox series

[5/8] libext2fs: fix ea loop in read_xattrs_from_buffer

Message ID 20260924222944.3683556-6-sandeen@redhat.com
State New
Headers show
Series e2fsprogs: fix extended attribute iteration loop bounds checking | expand

Commit Message

Eric Sandeen Sept. 24, 2026, 10:23 p.m. UTC 
From: Andreas Gruenbacher <agruenba@redhat.com>

Fix the same problems as in check_ea_in_inode().

There is no need for recalculating the remaining space in the second
iteration.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 lib/ext2fs/ext_attr.c | 38 ++++++++++++--------------------------
 1 file changed, 12 insertions(+), 26 deletions(-)
diff mbox series

Patch

diff --git a/lib/ext2fs/ext_attr.c b/lib/ext2fs/ext_attr.c
index 914bff1d..6f6d2f4f 100644
--- a/lib/ext2fs/ext_attr.c
+++ b/lib/ext2fs/ext_attr.c
@@ -884,26 +884,23 @@  static errcode_t read_xattrs_from_buffer(struct ext2_xattr_handle *handle,
 
 	/* find the end */
 	end = entries;
-	remain = storage_size;
-	while (remain >= sizeof(struct ext2_ext_attr_entry) &&
-	       !EXT2_EXT_IS_LAST_ENTRY(end)) {
-
-		/* header eats this space */
-		remain -= sizeof(struct ext2_ext_attr_entry);
-
-		/* is attribute name valid? */
-		if (EXT2_EXT_ATTR_SIZE(end->e_name_len) > remain)
+	/* take finish entry 0UL into account */
+	remain = storage_size - sizeof(__u32);
+	while (!EXT2_EXT_IS_LAST_ENTRY(end)) {
+		/* end->e_name_len is within the first four bytes */
+		if (EXT2_EXT_ATTR_LEN(end->e_name_len) > remain)
 			return EXT2_ET_EA_BAD_NAME_LEN;
-
-		/* attribute len eats this space */
-		remain -= EXT2_EXT_ATTR_SIZE(end->e_name_len);
+		remain -= EXT2_EXT_ATTR_LEN(end->e_name_len);
+		if (!end->e_value_inum) {
+			if (EXT2_EXT_ATTR_SIZE(end->e_value_size) > remain)
+				return EXT2_ET_EA_BAD_VALUE_SIZE;
+			remain -= EXT2_EXT_ATTR_SIZE(end->e_value_size);
+		}
 		end = EXT2_EXT_ATTR_NEXT(end);
 	}
 
 	entry = entries;
-	remain = storage_size;
-	while (remain >= sizeof(struct ext2_ext_attr_entry) &&
-	       !EXT2_EXT_IS_LAST_ENTRY(entry)) {
+	while (!EXT2_EXT_IS_LAST_ENTRY(entry)) {
 
 		/* Allocate space for more attrs? */
 		if (handle->count == handle->capacity) {
@@ -914,12 +911,6 @@  static errcode_t read_xattrs_from_buffer(struct ext2_xattr_handle *handle,
 
 		x = handle->attrs + handle->count;
 
-		/* header eats this space */
-		remain -= sizeof(struct ext2_ext_attr_entry);
-
-		/* attribute len eats this space */
-		remain -= EXT2_EXT_ATTR_SIZE(entry->e_name_len);
-
 		/* Extract name */
 		prefix = find_ea_prefix(entry->e_name_index);
 		prefix_len = (prefix ? strlen(prefix) : 0);
@@ -942,9 +933,6 @@  static errcode_t read_xattrs_from_buffer(struct ext2_xattr_handle *handle,
 			return EXT2_ET_BAD_EA_BLOCK_NUM;
 
 		if (entry->e_value_inum == 0) {
-			if (entry->e_value_size > remain)
-				return EXT2_ET_EA_BAD_VALUE_SIZE;
-
 			if (entry->e_value_offs + entry->e_value_size > values_size)
 				return EXT2_ET_EA_BAD_VALUE_OFFSET;
 
@@ -953,8 +941,6 @@  static errcode_t read_xattrs_from_buffer(struct ext2_xattr_handle *handle,
 			    (char *)end + sizeof(__u32))
 				return EXT2_ET_EA_BAD_VALUE_OFFSET;
 
-			remain -= entry->e_value_size;
-
 			err = ext2fs_get_mem(entry->e_value_size, &x->value);
 			if (err)
 				return err;