Hi
I've got an email that shows these syslog lines:
hpfs: filesystem error: warning: spare dnodes used, try chkdsk
hpfs: You really don't want any checks? You are crazy...
hpfs: hpfs_map_sector(): read error
hpfs: code page support is disabled
==================================================================
BUG: KASAN: use-after-free in strcmp+0x6f/0xc0 lib/string.c:283
Read of size 1 at addr ffff8880116728a6 by task syz-executor411/6741
It seems that you deliberately turned off checking by using the parameter
check=none.
The HPFS driver will not check metadata for corruption if "check=none" is
used, you should use the default "check=normal" or enable extra
time-consuming checks using "check=strict".
The code that checks extended attributes in the fnode is in the function
hpfs_map_fnode, the branch "if ((fnode = hpfs_map_sector(s, ino, bhp,
FNODE_RD_AHEAD))) { if (hpfs_sb(s)->sb_chk) {" - fixes for checking
extended attributes should go there.
If you get a KASAN warning when using "check=normal" or "check=strict",
report it and I will fix it; with "check=none" it is not supposed to work.
Mikulas
On Sun, 20 Jul 2025, Antoni Pokusinski wrote:
> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=fa88eb476e42878f2844
> Tested-by: syzbot+fa88eb476e42878f2844@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Antoni Pokusinski <apokusinski01@xxxxxxxxx>
>
> ---
> 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
>
