Re: [syzbot] [hfs?] WARNING in hfs_write_inode

From: Michael Schmitz
Date: Thu Jan 05 2023 - 18:46:25 EST


Hi Linus,

Am 06.01.2023 um 10:53 schrieb Linus Torvalds:
On Thu, Jan 5, 2023 at 1:35 PM Michael Schmitz <schmitzmic@xxxxxxxxx> 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