linux-kernel - Re: [syzbot] [hfs?] WARNING in hfs_write

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1a3d07bf-16f5-71a8-6500-7d37802dbadd@gmail.com>
Date:   Fri, 6 Jan 2023 12:46:04 +1300
From:   Michael Schmitz <schmitzmic@...il.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Arnd Bergmann <arnd@...db.de>,
        syzbot <syzbot+7bb7cd3595533513a9e7@...kaller.appspotmail.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        christian.brauner@...ntu.com,
        Damien Le Moal <damien.lemoal@...nsource.wdc.com>,
        jlayton@...nel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com,
        Matthew Wilcox <willy@...radead.org>,
        ZhangPeng <zhangpeng362@...wei.com>,
        Viacheslav Dubeyko <slava@...eyko.com>,
        linux-m68k@...ts.linux-m68k.org, flar@...andria.com
Subject: Re: [syzbot] [hfs?] WARNING in hfs_write_inode

Hi Linus,

Am 06.01.2023 um 10:53 schrieb Linus Torvalds:
> On Thu, Jan 5, 2023 at 1:35 PM Michael Schmitz <schmitzmic@...il.com> wrote:
>>
>> Looking at Linus' patch, I wonder whether the missing fd.entrylength
>> size test in the HFS_IS_RSRC(inode) case was due to the fact that a
>> file's resource fork may be empty?
>
> But if that is the case, then the subsequent hfs_bnode_read would
> return garbage, no? And then writing it back after the update would be
> even worse.
>
> So adding that
>
> +               if (fd.entrylength < sizeof(struct hfs_cat_file))
> +                       goto out;
>
> would seem to be the right thing anyway. No?

Yes, it would seem to be the right thing (in order to avoid further 
corrupting HFS data structures). Returning -EIO might cause a regression 
though.

> But I really don't know the code, so this is all from just looking at
> it and going "that makes no sense". Maybe it _does_ make sense to
> people who have more background on it.

What had me wondering is that the 'panic?' comment was only present in 
the directory and regular file data cased but not in the resource fork 
case.

But I don't really understand the code too well either. I'll have to see 
for myself whether or not your patch does cause a regression on HFS 
filesystems such as the OF bootstrap partition used on PowerPC Macs.

Cheers,

	Michael


>
>              Linus
>