[PATCH] hpfs: add checks for ea addresses

Antoni Pokusinski posted 1 patch 2 months, 2 weeks ago
There is a newer version of this series
fs/hpfs/anode.c   | 2 +-
fs/hpfs/ea.c      | 6 +++---
fs/hpfs/hpfs_fn.h | 5 +++++
fs/hpfs/map.c     | 2 +-
4 files changed, 10 insertions(+), 5 deletions(-)
[PATCH] hpfs: add checks for ea addresses
Posted by Antoni Pokusinski 2 months, 2 weeks ago
The addresses of the extended attributes are computed using the
fnode_ea() and next_ea() functions which refer to the fields residing in
a given fnode. There are no sanity checks for the returned values, so in
the case of corrupted data in the fnode, the ea addresses are invalid.

Fix the bug by adding ea_valid_addr() function which checks if a given
extended attribute resides within the range of the ea array of a given
fnode.

Reported-by: syzbot+fa88eb476e42878f2844@syzkaller.appspotmail.com
Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>

#syz test
---
 fs/hpfs/anode.c   | 2 +-
 fs/hpfs/ea.c      | 6 +++---
 fs/hpfs/hpfs_fn.h | 5 +++++
 fs/hpfs/map.c     | 2 +-
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/hpfs/anode.c b/fs/hpfs/anode.c
index c14c9a035ee0..f347cdd94a5c 100644
--- a/fs/hpfs/anode.c
+++ b/fs/hpfs/anode.c
@@ -488,7 +488,7 @@ void hpfs_remove_fnode(struct super_block *s, fnode_secno fno)
 	if (!fnode_is_dir(fnode)) hpfs_remove_btree(s, &fnode->btree);
 	else hpfs_remove_dtree(s, le32_to_cpu(fnode->u.external[0].disk_secno));
 	ea_end = fnode_end_ea(fnode);
-	for (ea = fnode_ea(fnode); ea < ea_end; ea = next_ea(ea))
+	for (ea = fnode_ea(fnode); ea < ea_end && ea_valid_addr(fnode, ea); ea = next_ea(ea))
 		if (ea_indirect(ea))
 			hpfs_ea_remove(s, ea_sec(ea), ea_in_anode(ea), ea_len(ea));
 	hpfs_ea_ext_remove(s, le32_to_cpu(fnode->ea_secno), fnode_in_anode(fnode), le32_to_cpu(fnode->ea_size_l));
diff --git a/fs/hpfs/ea.c b/fs/hpfs/ea.c
index 102ba18e561f..d7ada7f5a7ae 100644
--- a/fs/hpfs/ea.c
+++ b/fs/hpfs/ea.c
@@ -80,7 +80,7 @@ int hpfs_read_ea(struct super_block *s, struct fnode *fnode, char *key,
 	char ex[4 + 255 + 1 + 8];
 	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))
+	for (ea = fnode_ea(fnode); ea < ea_end  && ea_valid_addr(fnode, ea); ea = next_ea(ea))
 		if (!strcmp(ea->name, key)) {
 			if (ea_indirect(ea))
 				goto indirect;
@@ -135,7 +135,7 @@ 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))
+	for (ea = fnode_ea(fnode); ea < ea_end && ea_valid_addr(fnode, ea); ea = next_ea(ea))
 		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));
@@ -198,7 +198,7 @@ void hpfs_set_ea(struct inode *inode, struct fnode *fnode, const char *key,
 	unsigned char h[4];
 	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))
+	for (ea = fnode_ea(fnode); ea < ea_end && ea_valid_addr(fnode, ea); ea = next_ea(ea))
 		if (!strcmp(ea->name, key)) {
 			if (ea_indirect(ea)) {
 				if (ea_len(ea) == size)
diff --git a/fs/hpfs/hpfs_fn.h b/fs/hpfs/hpfs_fn.h
index 237c1c23e855..c65ce60d7d9a 100644
--- a/fs/hpfs/hpfs_fn.h
+++ b/fs/hpfs/hpfs_fn.h
@@ -152,6 +152,11 @@ static inline struct extended_attribute *next_ea(struct extended_attribute *ea)
 	return (struct extended_attribute *)((char *)ea + 5 + ea->namelen + ea_valuelen(ea));
 }
 
+static inline bool ea_valid_addr(struct fnode *fnode, struct extended_attribute *ea)
+{
+	return ((char *)ea >= (char *)&fnode->ea) && ((char *)ea < (char *)&fnode->ea + sizeof(fnode->ea));
+}
+
 static inline secno ea_sec(struct extended_attribute *ea)
 {
 	return le32_to_cpu(get_unaligned((__le32 *)((char *)ea + 9 + ea->namelen)));
diff --git a/fs/hpfs/map.c b/fs/hpfs/map.c
index ecd9fccd1663..0016dcbf1b1f 100644
--- a/fs/hpfs/map.c
+++ b/fs/hpfs/map.c
@@ -202,7 +202,7 @@ struct fnode *hpfs_map_fnode(struct super_block *s, ino_t ino, struct buffer_hea
 			}
 			ea = fnode_ea(fnode);
 			ea_end = fnode_end_ea(fnode);
-			while (ea != ea_end) {
+			while (ea != ea_end && ea_valid_addr(fnode, ea)) {
 				if (ea > ea_end) {
 					hpfs_error(s, "bad EA in fnode %08lx",
 						(unsigned long)ino);
-- 
2.25.1
Re: [PATCH] hpfs: add checks for ea addresses
Posted by Al Viro 2 months, 2 weeks ago
On Sun, Jul 20, 2025 at 02:34:27PM +0200, Antoni Pokusinski wrote:

>  	ea_end = fnode_end_ea(fnode);
> -	for (ea = fnode_ea(fnode); ea < ea_end; ea = next_ea(ea))
> +	for (ea = fnode_ea(fnode); ea < ea_end && ea_valid_addr(fnode, ea); ea = next_ea(ea))

That looks really fishy.  If nothing else, your ea_valid_addr() is a range check;
are there any situations where we would want to allow fnode_ea() and fnode_end_ea()
be out of that range?
Re: [syzbot] [fs?] KASAN: use-after-free Read in hpfs_get_ea
Posted by syzbot 2 months, 2 weeks ago
Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+fa88eb476e42878f2844@syzkaller.appspotmail.com
Tested-by: syzbot+fa88eb476e42878f2844@syzkaller.appspotmail.com

Tested on:

commit:         f4a40a42 Merge tag 'efi-fixes-for-v6.16-2' of git://gi..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12314b82580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=f09d04131ef56b22
dashboard link: https://syzkaller.appspot.com/bug?extid=fa88eb476e42878f2844
compiler:       Debian clang version 20.1.7 (++20250616065708+6146a88f6049-1~exp1~20250616065826.132), Debian LLD 20.1.7
patch:          https://syzkaller.appspot.com/x/patch.diff?x=17c0538c580000

Note: testing is done by a robot and is best-effort only.