[PATCH] hpfs: fix extended attribute bounds checks and get_indirect_ea() overflow

Hui Peng posted 1 patch 4 days, 22 hours ago
[PATCH] hpfs: fix extended attribute bounds checks and get_indirect_ea() overflow
Posted by Hui Peng 4 days, 22 hours ago
Fix extended attribute parsing bugs in fs/hpfs/ea.c and fs/hpfs/map.c:

1. In get_indirect_ea(), reject negative or wrapping size values (such
   as size == 0xffffffff where size + 1 wraps to 0 and kmalloc(0)
   returns ZERO_SIZE_PTR).
2. In hpfs_ea_ext_remove(), hpfs_get_ea(), and hpfs_map_fnode(),
   validate EA entry sizes (`ea->namelen` and `ea_size(ea)`) against the
   buffer end and ensure `ea_size(ea) > 0` so malformed extended
   attributes cannot cause out-of-bounds reads or infinite loops.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
diff --git a/fs/hpfs/ea.c b/fs/hpfs/ea.c
index 4664f9ab06ee..a3b3e6c7800e 100644
--- a/fs/hpfs/ea.c
+++ b/fs/hpfs/ea.c
@@ -51,6 +51,10 @@ void hpfs_ea_ext_remove(struct super_block *s, secno a, int ano, unsigned len)
 static char *get_indirect_ea(struct super_block *s, int ano, secno a, int size)
 {
 	char *ret;
+	if (size < 0 || size > 0xffff) {
+		hpfs_error(s, "bad indirect EA size %d", size);
+		return NULL;
+	}
 	if (!(ret = kmalloc(size + 1, GFP_NOFS))) {
 		pr_err("out of memory for EA\n");
 		return NULL;
@@ -135,10 +139,22 @@ char *hpfs_get_ea(struct super_block *s, struct fnode *fnode, char *key, int *si
 	secno a;
 	struct extended_attribute *ea;
 	struct extended_attribute *ea_end = fnode_end_ea(fnode);
-	for (ea = fnode_ea(fnode); ea < ea_end; ea = next_ea(ea))
+	if (le16_to_cpu(fnode->ea_size_s) &&
+	    (le16_to_cpu(fnode->ea_offs) < 0xc4 ||
+	     le16_to_cpu(fnode->ea_offs) + le16_to_cpu(fnode->acl_size_s) +
+	     le16_to_cpu(fnode->ea_size_s) > 0x200))
+		return NULL;
+	for (ea = fnode_ea(fnode); ea < ea_end; ea = next_ea(ea)) {
+		if ((char *)ea + 5 > (char *)ea_end ||
+		    next_ea(ea) > ea_end ||
+		    ea->name[ea->namelen] != 0)
+			return NULL;
 		if (!strcmp(ea->name, key)) {
-			if (ea_indirect(ea))
+			if (ea_indirect(ea)) {
+				if (ea_valuelen(ea) < 8)
+					return NULL;
 				return get_indirect_ea(s, ea_in_anode(ea), ea_sec(ea), *size = ea_len(ea));
+			}
 			if (!(ret = kmalloc((*size = ea_valuelen(ea)) + 1, GFP_NOFS))) {
 				pr_err("out of memory for EA\n");
 				return NULL;
@@ -147,6 +163,7 @@ char *hpfs_get_ea(struct super_block *s, struct fnode *fnode, char *key, int *si
 			ret[ea_valuelen(ea)] = 0;
 			return ret;
 		}
+	}
 	a = le32_to_cpu(fnode->ea_secno);
 	len = le32_to_cpu(fnode->ea_size_l);
 	ano = fnode_in_anode(fnode);
@@ -162,6 +179,7 @@ char *hpfs_get_ea(struct super_block *s, struct fnode *fnode, char *key, int *si
 		if (hpfs_ea_read(s, a, ano, pos, 4, ex)) return NULL;
 		if (hpfs_ea_read(s, a, ano, pos + 4, ea->namelen + 1 + (ea_indirect(ea) ? 8 : 0), ex + 4))
 			return NULL;
+		ea->name[ea->namelen] = 0;
 		if (!strcmp(ea->name, key)) {
 			if (ea_indirect(ea))
 				return get_indirect_ea(s, ea_in_anode(ea), ea_sec(ea), *size = ea_len(ea));
diff --git a/fs/hpfs/map.c b/fs/hpfs/map.c
index be73233502f8..b2c571e71a60 100644
--- a/fs/hpfs/map.c
+++ b/fs/hpfs/map.c
@@ -168,9 +168,9 @@ struct fnode *hpfs_map_fnode(struct super_block *s, ino_t ino, struct buffer_hea
 		return NULL;
 	}
 	if ((fnode = hpfs_map_sector(s, ino, bhp, FNODE_RD_AHEAD))) {
+		struct extended_attribute *ea;
+		struct extended_attribute *ea_end;
 		if (hpfs_sb(s)->sb_chk) {
-			struct extended_attribute *ea;
-			struct extended_attribute *ea_end;
 			if (le32_to_cpu(fnode->magic) != FNODE_MAGIC) {
 				hpfs_error(s, "bad magic on fnode %08lx",
 					(unsigned long)ino);
@@ -192,24 +192,31 @@ struct fnode *hpfs_map_fnode(struct super_block *s, ino_t ino, struct buffer_hea
 					goto bail;
 				}
 			}
-			if (le16_to_cpu(fnode->ea_size_s) && (le16_to_cpu(fnode->ea_offs) < 0xc4 ||
-			   le16_to_cpu(fnode->ea_offs) + le16_to_cpu(fnode->acl_size_s) + le16_to_cpu(fnode->ea_size_s) > 0x200)) {
-				hpfs_error(s,
-					"bad EA info in fnode %08lx: ea_offs == %04x ea_size_s == %04x",
-					(unsigned long)ino,
-					le16_to_cpu(fnode->ea_offs), le16_to_cpu(fnode->ea_size_s));
+		}
+		if (le16_to_cpu(fnode->ea_size_s) &&
+		    (le16_to_cpu(fnode->ea_offs) < 0xc4 ||
+		     le16_to_cpu(fnode->ea_offs) +
+		     le16_to_cpu(fnode->acl_size_s) +
+		     le16_to_cpu(fnode->ea_size_s) > 0x200)) {
+			hpfs_error(s,
+				"bad EA info in fnode %08lx: ea_offs == %04x ea_size_s == %04x",
+				(unsigned long)ino,
+				le16_to_cpu(fnode->ea_offs),
+				le16_to_cpu(fnode->ea_size_s));
+			goto bail;
+		}
+		ea = fnode_ea(fnode);
+		ea_end = fnode_end_ea(fnode);
+		while (ea != ea_end) {
+			if ((char *)ea + 5 > (char *)ea_end ||
+			    next_ea(ea) > ea_end ||
+			    ea->name[ea->namelen] != 0 ||
+			    (ea_indirect(ea) && ea_valuelen(ea) != 8)) {
+				hpfs_error(s, "bad EA in fnode %08lx",
+					(unsigned long)ino);
 				goto bail;
 			}
-			ea = fnode_ea(fnode);
-			ea_end = fnode_end_ea(fnode);
-			while (ea != ea_end) {
-				if (ea > ea_end) {
-					hpfs_error(s, "bad EA in fnode %08lx",
-						(unsigned long)ino);
-					goto bail;
-				}
-				ea = next_ea(ea);
-			}
+			ea = next_ea(ea);
 		}
 	}
 	return fnode;