| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <784a100e-c848-3a9c-74ef-439fa12df53c@redhat.com>
Date: Mon, 21 Jul 2025 21:51:22 +0200 (CEST)
From: Mikulas Patocka <mpatocka@...hat.com>
To: Antoni Pokusinski <apokusinski01@...il.com>
cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
syzbot+fa88eb476e42878f2844@...kaller.appspotmail.com
Subject: Re: [PATCH] hpfs: add checks for ea addresses
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@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=fa88eb476e42878f2844
> Tested-by: syzbot+fa88eb476e42878f2844@...kaller.appspotmail.com
> Signed-off-by: Antoni Pokusinski <apokusinski01@...il.com>
>
> ---
> 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
>
Powered by blists - more mailing lists