[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250721224228.nzt7l7knum5hupgl@antoni-VivoBook-ASUSLaptop-X512FAY-K512FA>
Date: Tue, 22 Jul 2025 00:42:28 +0200
From: Antoni Pokusinski <apokusinski01@...il.com>
To: Mikulas Patocka <mpatocka@...hat.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
Hello,
Thanks for the feedback.
On Mon, Jul 21, 2025 at 09:51:22PM +0200, Mikulas Patocka wrote:
> 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".
>
Yes, that's right. If we had "check!=none", then the issue would not come
up in syzkaller due to the checks performed on the extended attribues in the fnode.
I've just tried to confim that by using the "check=normal" and I did not get
the KASAN warning, as expected.
> 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
>
I'm just wondering what should be the expected kernel behaviour in the situation where
"check=none" and the "ea_offs", "acl_size_s", "ea_size_s" fields of fnode are corrupt?
If we assume that in such case running into some undefined behavior (which is accessing
an unknown memory area) is alright, then the code does not need any changes.
But if we'd like to prevent it, then I think we should always check the extended
attribute address regardless of the "check" parameter, as demonstrated
in the patch.
Kind regards,
Antoni
Powered by blists - more mailing lists