[PATCH] hpfs: validate in-fnode EA bounds unconditionally

[William Theesfeld <william@theesfeld.net>]

The in-fnode extended-attribute region is walked by hpfs_get_ea(),
hpfs_read_ea() and hpfs_set_ea() using fnode_ea() and fnode_end_ea(),
which derive their bounds from on-disk fields (ea_offs, acl_size_s,
ea_size_s).  hpfs_map_fnode() already validates that ea_offs +
acl_size_s + ea_size_s stays within the fnode buffer and that the EA
chain terminates cleanly, but those checks live inside the
sb_chk-gated block, so a user that has disabled the "chk" mount option
loads any image without them.

A crafted image with bogus values for those fields then causes the
walk to escape the 512-byte fnode buffer.  strcmp() on ea->name reads
freed memory, producing a slab use-after-free reported by syzbot:

  hpfs: filesystem error: improperly stopped
  hpfs: You really don't want any checks? You are crazy...
  hpfs: hpfs_map_sector(): read error
  ==================================================================
  BUG: KASAN: use-after-free in strcmp+0x6b/0xc0
  Read of size 1 at addr ffff88804aa888a6 by task syz.0.17/5830
  Call Trace:
   strcmp+0x6b/0xc0
   hpfs_get_ea+0x134/0xee0
   hpfs_read_inode+0x1a6/0x1050
   hpfs_fill_super+0x123d/0x1fa0
   get_tree_bdev_flags+0x431/0x4f0
   vfs_get_tree+0x92/0x2a0
   do_new_mount+0x341/0xd30
   __se_sys_mount+0x31d/0x420
   do_syscall_64+0x15f/0x560

On-disk bounds validation belongs at parse time regardless of the
"chk" option: it is required to keep the kernel from reading past the
fnode buffer, not merely to diagnose unusual images.

Move the EA bounds check and the EA-chain walk out of the sb_chk-gated
block so they run for every fnode load.  The magic and btree
consistency checks are left as before — those diagnose
suspicious-but-not-unsafe images and remain useful only when the user
has asked for them.

Reported-by: syzbot+fa88eb476e42878f2844@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=fa88eb476e42878f2844
Signed-off-by: William Theesfeld <william@theesfeld.net>
---
 fs/hpfs/map.c | 54 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/fs/hpfs/map.c b/fs/hpfs/map.c
index be7323350..c7388abdd 100644
--- a/fs/hpfs/map.c
+++ b/fs/hpfs/map.c
@@ -168,9 +168,41 @@ 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;
+
+		/*
+		 * The in-fnode EA region is walked by hpfs_get_ea(),
+		 * hpfs_read_ea() and hpfs_set_ea() using fnode_ea() and
+		 * fnode_end_ea(), which derive their bounds from on-disk
+		 * fields.  Validate those bounds unconditionally so that a
+		 * crafted image cannot cause the walk to escape the fnode
+		 * buffer regardless of the "chk" mount option.
+		 */
+		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 (ea > ea_end) {
+				hpfs_error(s, "bad EA in fnode %08lx",
+					(unsigned long)ino);
+				goto bail;
+			}
+			ea = next_ea(ea);
+		}
+
 		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 +224,6 @@ 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));
-				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);
-			}
 		}
 	}
 	return fnode;
-- 
2.54.0