WARNING in hfs_write_inode

The issue was bisected to:

commit 55d1cbbbb29e6656c662ee8f73ba1fc4777532eb

Author: Arnd Bergmann <[email protected]>
Date:   Tue Nov 9 02:35:04 2021 +0000

hfs/hfsplus: use WARN_ON for sanity check

My patch was a mechanical conversion from '/* panic? */' to 'WARN_ON()' to work around a compiler warning, and the previous code had been in there since the 2004 HFS rewrite by Roman Zippel.

I know nothing about what this function actually does, so my best answer is that we could revert my patch and use pr_debug() instead of WARN_ON() for all of these.

I suspect this code is basically all dead. From what I can tell, hfs only gets updates for

(a) syzbot reports

(b) vfs interface changes

and the last real changes seem to have been by Ernesto A. Fernández back in 2018.

Hmm. Looking at that code, we have another bug in there, introduced by an earlier fix for a similar issue: commit 8d824e69d9f3 ("hfs: fix OOB Read in __hfs_brec_find") added

(quoted diff) +2 -0

+       if (HFS_I(main_inode)->cat_key.CName.len > HFS_NAMELEN)
+               return -EIO;

but it's after hfs_find_init(), so it should actually have done a hfs_find_exit() to not leak memory.

So we should probably fix that too.

Something like this ENTIRELY UNTESTED patch?

Do we have anybody who looks at hfs?

Adding Viacheslav Dubeyko to Cc, he's at least been reviewing patches for HFS and HFS+ somewhat recently. The linux-m68k list may have some users dual-booting old MacOS.

[1] https://salsa.debian.org/installer-team/grub-installer/-/blob/master/grub-installer#L918

Quoted 7 lines · Linus Torvalds

I suspect this code is basically all dead. From what I can tell, hfs only gets updates for

(a) syzbot reports

(b) vfs interface changes

There is clearly no new work going into it, and most data exchange with MacOS would use HFS+, but I think there are still some users.

Quoted 17 lines

and the last real changes seem to have been by Ernesto A. Fernández back in 2018.

Hmm. Looking at that code, we have another bug in there, introduced by an earlier fix for a similar issue: commit 8d824e69d9f3 ("hfs: fix OOB Read in __hfs_brec_find") added

(quoted diff) +2 -0

+       if (HFS_I(main_inode)->cat_key.CName.len > HFS_NAMELEN)
+               return -EIO;

but it's after hfs_find_init(), so it should actually have done a hfs_find_exit() to not leak memory.

So we should probably fix that too.

Something like this ENTIRELY UNTESTED patch?

Quoted 1 line

Do we have anybody who looks at hfs?

Adding Viacheslav Dubeyko to Cc, he's at least been reviewing patches for HFS and HFS+ somewhat recently. The linux-m68k list may have some users dual-booting old MacOS.

Viacheslav, see the start of the thread at
https://lore.kernel.org/lkml/[email protected]/

Hi Arnd,

Quoted 1 line · Arnd Bergmann

Quoted 9 lines

Quoted 4 lines

Something like this ENTIRELY UNTESTED patch?

Do we have anybody who looks at hfs?

Adding Viacheslav Dubeyko to Cc, he's at least been reviewing patches for HFS and HFS+ somewhat recently. The linux-m68k list may have some users dual-booting old MacOS.

Quoted 3 lines

Viacheslav, see the start of the thread at
https://lore.kernel.org/lkml/[email protected]/

Let me take a look into the issue.

Also, as far as I can see, available volume in report (mount_0.gz) somehow corrupted already:

Quoted 1 line · Viacheslav Dubeyko

Also, as far as I can see, available volume in report (mount_0.gz) somehow corrupted already:

Syzbot generates deliberately-corrupted (aka fuzzed) filesystem images. So basically, you can't trust anything you read from the disc.

Quoted 6 lines · Viacheslav Dubeyko

Quoted 1 line

Also, as far as I can see, available volume in report (mount_0.gz) somehow corrupted already:

Syzbot generates deliberately-corrupted (aka fuzzed) filesystem images. So basically, you can't trust anything you read from the disc.

If the volume has been deliberately corrupted, then no guarantee that file system driver will behave nicely. Technically speaking, inode write operation should never happened for corrupted volume because the corruption should be detected during b-tree node initialization time. If we would like to achieve such nice state of HFS/HFS+ drivers, then it requires a lot of refactoring/implementation efforts. I am not sure that it is worth to do because not so many guys really use HFS/HFS+ as the main file system under Linux.

Quoted 7 lines · Viacheslav Dubeyko

Quoted 6 lines · Viacheslav Dubeyko

Quoted 1 line

Also, as far as I can see, available volume in report (mount_0.gz) somehow corrupted already:

Syzbot generates deliberately-corrupted (aka fuzzed) filesystem images. So basically, you can't trust anything you read from the disc.

Quoted 7 lines

If the volume has been deliberately corrupted, then no guarantee that file system driver will behave nicely. Technically speaking, inode write operation should never happened for corrupted volume because the corruption should be detected during b-tree node initialization time. If we would like to achieve such nice state of HFS/HFS+ drivers, then it requires a lot of refactoring/implementation efforts. I am not sure that it is worth to do because not so many guys really use HFS/HFS+ as the main file system under Linux.

Most popular distros will happily auto-mount HFS/HFS+ from anything inserted into USB (e.g. what one may think is a charger). This creates interesting security consequences for most Linux users.
An image may also be corrupted non-deliberately, which will lead to random memory corruptions if the kernel trusts it blindly.

Quoted 17 lines · Dmitry Vyukov

Quoted 7 lines · Viacheslav Dubeyko

Quoted 6 lines · Viacheslav Dubeyko

Quoted 1 line

Also, as far as I can see, available volume in report (mount_0.gz) somehow corrupted already:

Syzbot generates deliberately-corrupted (aka fuzzed) filesystem images. So basically, you can't trust anything you read from the disc.

Quoted 7 lines

If the volume has been deliberately corrupted, then no guarantee that file system driver will behave nicely. Technically speaking, inode write operation should never happened for corrupted volume because the corruption should be detected during b-tree node initialization time. If we would like to achieve such nice state of HFS/HFS+ drivers, then it requires a lot of refactoring/implementation efforts. I am not sure that it is worth to do because not so many guys really use HFS/HFS+ as the main file system under Linux.

Quoted 5 lines

Most popular distros will happily auto-mount HFS/HFS+ from anything inserted into USB (e.g. what one may think is a charger). This creates interesting security consequences for most Linux users.
An image may also be corrupted non-deliberately, which will lead to random memory corruptions if the kernel trusts it blindly.

Then we should delete the HFS/HFS+ filesystems. They're orphaned in MAINTAINERS and if distros are going to do such a damnfool thing, then we must stop them.

Quoted 3 lines

Then we should delete the HFS/HFS+ filesystems. They're orphaned in MAINTAINERS and if distros are going to do such a damnfool thing, then we must stop them.

Both HFS and HFS+ work perfectly fine. And if distributions or users are so sensitive about security, it's up to them to blacklist individual features in the kernel.

Both HFS and HFS+ have been the default filesystem on MacOS for 30 years and I don't think it's justified to introduce such a hard compatibility breakage just because some people are worried about theoretical evil maid attacks.

HFS/HFS+ mandatory if you want to boot Linux on a classic Mac or PowerMac and I don't think it's okay to break all these systems running Linux.

Quoted 13 lines · John Paul Adrian Glaubitz

Quoted 3 lines

Then we should delete the HFS/HFS+ filesystems. They're orphaned in MAINTAINERS and if distros are going to do such a damnfool thing, then we must stop them.

Both HFS and HFS+ work perfectly fine. And if distributions or users are so sensitive about security, it's up to them to blacklist individual features in the kernel.

Both HFS and HFS+ have been the default filesystem on MacOS for 30 years and I don't think it's justified to introduce such a hard compatibility breakage just because some people are worried about theoretical evil maid attacks.

Quoted 2 lines

HFS/HFS+ mandatory if you want to boot Linux on a classic Mac or PowerMac and I don't think it's okay to break all these systems running Linux.

If they're so popular, then it should be no trouble to find somebody to volunteer to maintain those filesystems. Except they've been marked as orphaned since 2011 and effectively were orphaned several years before that (the last contribution I see from Roman Zippel is in 2008, and his last contribution to hfs was in 2006).

Quoted 14 lines · Matthew Wilcox

Quoted 13 lines · John Paul Adrian Glaubitz

Quoted 3 lines

Then we should delete the HFS/HFS+ filesystems. They're orphaned in MAINTAINERS and if distros are going to do such a damnfool thing, then we must stop them.

Both HFS and HFS+ work perfectly fine. And if distributions or users are so sensitive about security, it's up to them to blacklist individual features in the kernel.

Both HFS and HFS+ have been the default filesystem on MacOS for 30 years and I don't think it's justified to introduce such a hard compatibility breakage just because some people are worried about theoretical evil maid attacks.

Quoted 8 lines

Quoted 2 lines

HFS/HFS+ mandatory if you want to boot Linux on a classic Mac or PowerMac and I don't think it's okay to break all these systems running Linux.

If they're so popular, then it should be no trouble to find somebody to volunteer to maintain those filesystems. Except they've been marked as orphaned since 2011 and effectively were orphaned several years before that (the last contribution I see from Roman Zippel is in 2008, and his last contribution to hfs was in 2006).

I suspect that this is one of those catch-22 situations: distros are going to enable every feature under the sun. That doesn't mean that anyone is actually _using_ them these days.

Is "staging" still a thing? Maybe we should move these drivers into the staging directory and pick a release where we'll sunset it, and then see who comes out of the woodwork?

Quoted 3 lines

I suspect that this is one of those catch-22 situations: distros are going to enable every feature under the sun. That doesn't mean that anyone is actually _using_ them these days.

We need to much more proactive about dropping support for unmaintained filesystems that nobody is ever fixing despite the constant stream of corruption- and deadlock- related bugs reported against them.

Quoted 3 lines · Dave Chinner

Quoted 3 lines

I suspect that this is one of those catch-22 situations: distros are going to enable every feature under the sun. That doesn't mean that anyone is actually _using_ them these days.

I think the value of filesystem code is not just a question of how often it gets executed -- it's also about retaining access to the data collected in archives, museums, galleries etc. that is inevitably held in old formats.

Quoted 3 lines

We need to much more proactive about dropping support for unmaintained filesystems that nobody is ever fixing despite the constant stream of corruption- and deadlock- related bugs reported against them.

IMO, a stream of bug reports is not a reason to remove code (it's a reason to revert some commits).

Anyway, that stream of bugs presumably flows from the unstable kernel API, which is inherently high-maintenance. It seems that a stable API could be more appropriate for any filesystem for which the on-disk format is fixed (by old media, by unmaintained FLOSS implementations or abandoned proprietary implementations).

You've misunderstood. Google have decided to subject the entire kernel (including obsolete unmaintained filesystems) to stress tests that it's never had before. IOW these bugs have been there since the code was merged. There's nothing to back out. There's no API change to blame. It's always been buggy and it's never mattered before.

It wouldn't be so bad if Google had also decided to fund people to fix those bugs, but no, they've decided to dump them on public mailing lists and berate developers into fixing them.

Quoted 6 lines · Finn Thain

Quoted 3 lines · Dave Chinner

Quoted 3 lines

I suspect that this is one of those catch-22 situations: distros are going to enable every feature under the sun. That doesn't mean that anyone is actually _using_ them these days.

Quoted 4 lines

I think the value of filesystem code is not just a question of how often it gets executed -- it's also about retaining access to the data collected in archives, museums, galleries etc. that is inevitably held in old formats.

That's an argument for adding support to tar, not for maintaining read/write support.

Quoted 5 lines

Quoted 4 lines

We need to much more proactive about dropping support for unmaintained filesystems that nobody is ever fixing despite the constant stream of corruption- and deadlock- related bugs reported against them.

Quoted 8 lines

IMO, a stream of bug reports is not a reason to remove code (it's a reason to revert some commits).

Anyway, that stream of bugs presumably flows from the unstable kernel API, which is inherently high-maintenance. It seems that a stable API could be more appropriate for any filesystem for which the on-disk format is fixed (by old media, by unmaintained FLOSS implementations or abandoned proprietary implementations).

You've misunderstood. Google have decided to subject the entire kernel (including obsolete unmaintained filesystems) to stress tests that it's never had before. IOW these bugs have been there since the code was merged. There's nothing to back out. There's no API change to blame. It's always been buggy and it's never mattered before.

It wouldn't be so bad if Google had also decided to fund people to fix those bugs, but no, they've decided to dump them on public mailing lists and berate developers into fixing them.

Quoted 10 lines · Finn Thain

Quoted 3 lines · Dave Chinner

Quoted 3 lines

I suspect that this is one of those catch-22 situations: distros are going to enable every feature under the sun. That doesn't mean that anyone is actually _using_ them these days.

I think the value of filesystem code is not just a question of how often it gets executed -- it's also about retaining access to the data collected in archives, museums, galleries etc. that is inevitably held in old formats.

That's an argument for adding support to tar, not for maintaining read/write support.

Quoted 4 lines

Quoted 3 lines

We need to much more proactive about dropping support for unmaintained filesystems that nobody is ever fixing despite the constant stream of corruption- and deadlock- related bugs reported against them.

Quoted 8 lines

IMO, a stream of bug reports is not a reason to remove code (it's a reason to revert some commits).

Anyway, that stream of bugs presumably flows from the unstable kernel API, which is inherently high-maintenance. It seems that a stable API could be more appropriate for any filesystem for which the on-disk format is fixed (by old media, by unmaintained FLOSS implementations or abandoned proprietary implementations).

You've misunderstood. Google have decided to subject the entire kernel (including obsolete unmaintained filesystems) to stress tests that it's never had before. IOW these bugs have been there since the code was merged. There's nothing to back out. There's no API change to blame. It's always been buggy and it's never mattered before.

It wouldn't be so bad if Google had also decided to fund people to fix those bugs, but no, they've decided to dump them on public mailing lists and berate developers into fixing them.

You've misunderstood. Google have decided to subject the entire kernel (including obsolete unmaintained filesystems) to stress tests that it's never had before. IOW these bugs have been there since the code was merged. There's nothing to back out. There's no API change to blame. It's always been buggy and it's never mattered before.

I'm not blaming the unstable API for the bugs, I'm blaming it for the workload. A stable API (like a userspace API) decreases the likelihood that overloaded maintainers have to orphan a filesystem implementation.

Quoted 13 lines · John Paul Adrian Glaubitz

Quoted 3 lines

Then we should delete the HFS/HFS+ filesystems. They're orphaned in MAINTAINERS and if distros are going to do such a damnfool thing, then we must stop them.

Both HFS and HFS+ work perfectly fine. And if distributions or users are so sensitive about security, it's up to them to blacklist individual features in the kernel.

Both HFS and HFS+ have been the default filesystem on MacOS for 30 years and I don't think it's justified to introduce such a hard compatibility breakage just because some people are worried about theoretical evil maid attacks.

Quoted 2 lines

HFS/HFS+ mandatory if you want to boot Linux on a classic Mac or PowerMac and I don't think it's okay to break all these systems running Linux.

If they're so popular, then it should be no trouble to find somebody to volunteer to maintain those filesystems. Except they've been marked as orphaned since 2011 and effectively were orphaned several years before that (the last contribution I see from Roman Zippel is in 2008, and his last contribution to hfs was in 2006).

Quoted 15 lines · Matthew Wilcox

Quoted 13 lines · John Paul Adrian Glaubitz

Quoted 3 lines

Then we should delete the HFS/HFS+ filesystems. They're orphaned in MAINTAINERS and if distros are going to do such a damnfool thing, then we must stop them.

Both HFS and HFS+ work perfectly fine. And if distributions or users are so sensitive about security, it's up to them to blacklist individual features in the kernel.

Both HFS and HFS+ have been the default filesystem on MacOS for 30 years and I don't think it's justified to introduce such a hard compatibility breakage just because some people are worried about theoretical evil maid attacks.

Quoted 8 lines

Quoted 2 lines

HFS/HFS+ mandatory if you want to boot Linux on a classic Mac or PowerMac and I don't think it's okay to break all these systems running Linux.

If they're so popular, then it should be no trouble to find somebody to volunteer to maintain those filesystems. Except they've been marked as orphaned since 2011 and effectively were orphaned several years before that (the last contribution I see from Roman Zippel is in 2008, and his last contribution to hfs was in 2006).

One data point may help.. I've been running Linux on an old PowerMac and an old Intel MacBook since about 2014 or 2015 or so. I have needed the HFS/HFS+ filesystem support for about 9 years now (including that "blessed" support for the Apple Boot partition).

There's never been a problem with Linux and the Apple filesystems. Maybe it speaks to the maturity/stability of the code that already exists. The code does not need a lot of attention nowadays.

Maybe the orphaned status is the wrong metric to use to determine removal. Maybe a better metric would be installation base. I.e., how many users use the filesystem.

I think you're missing the context. There are bugs in how this filesystem handles intentionally-corrupted filesystems. That's being reported as a critical bug because apparently some distributions automount HFS/HFS+ filesystems presented to them on a USB key. Nobody is being paid to fix these bugs. Nobody is volunteering to fix these bugs out of the kindness of their heart. What choice do we have but to remove the filesystem, regardless of how many happy users it has?

Quoted 8 lines · Matthew Wilcox

I think you're missing the context. There are bugs in how this filesystem handles intentionally-corrupted filesystems. That's being reported as a critical bug because apparently some distributions automount HFS/HFS+ filesystems presented to them on a USB key. Nobody is being paid to fix these bugs. Nobody is volunteering to fix these bugs out of the kindness of their heart. What choice do we have but to remove the filesystem, regardless of how many happy users it has?

You're being silly.

We have tons of sane options. The obvious one is "just don't mount untrusted media".

Now, the kernel doesn't know which media is trusted or not, since the kernel doesn't actually see things like /etc/mtab and friends. So we in the kernel can't do that, but distros should have a very easy time just fixing their crazy models.

Saying that the kernel should remove a completely fine filesystem just because some crazy use-cases that nobody cares about are broken, now *that* just crazy.

Now, would it be good to have a maintainer for hgs? Obviously. But no, we don't remove filesystems just because they don't have maintainers.

And no, we have not suddenly started saying "users don't matter".

Linus

Quoted 17 lines · Dmitry Vyukov

Quoted 7 lines · Viacheslav Dubeyko

Quoted 6 lines · Viacheslav Dubeyko

Quoted 1 line

Also, as far as I can see, available volume in report (mount_0.gz) somehow corrupted already:

Syzbot generates deliberately-corrupted (aka fuzzed) filesystem images. So basically, you can't trust anything you read from the disc.

Quoted 7 lines

If the volume has been deliberately corrupted, then no guarantee that file system driver will behave nicely. Technically speaking, inode write operation should never happened for corrupted volume because the corruption should be detected during b-tree node initialization time. If we would like to achieve such nice state of HFS/HFS+ drivers, then it requires a lot of refactoring/implementation efforts. I am not sure that it is worth to do because not so many guys really use HFS/HFS+ as the main file system under Linux.

Quoted 5 lines

Most popular distros will happily auto-mount HFS/HFS+ from anything inserted into USB (e.g. what one may think is a charger). This creates interesting security consequences for most Linux users.
An image may also be corrupted non-deliberately, which will lead to random memory corruptions if the kernel trusts it blindly.

Then we should delete the HFS/HFS+ filesystems. They're orphaned in MAINTAINERS and if distros are going to do such a damnfool thing, then we must stop them.

(Please ignore my previous mail which was CC'ed to the wrong list)

Hello!

Quoted 24 lines · Matthew Wilcox

Quoted 17 lines · Dmitry Vyukov

Quoted 7 lines · Viacheslav Dubeyko

Quoted 6 lines · Viacheslav Dubeyko

Quoted 1 line

Also, as far as I can see, available volume in report (mount_0.gz) somehow corrupted already:

Syzbot generates deliberately-corrupted (aka fuzzed) filesystem images. So basically, you can't trust anything you read from the disc.

Quoted 7 lines

If the volume has been deliberately corrupted, then no guarantee that file system driver will behave nicely. Technically speaking, inode write operation should never happened for corrupted volume because the corruption should be detected during b-tree node initialization time. If we would like to achieve such nice state of HFS/HFS+ drivers, then it requires a lot of refactoring/implementation efforts. I am not sure that it is worth to do because not so many guys really use HFS/HFS+ as the main file system under Linux.

Quoted 5 lines

Most popular distros will happily auto-mount HFS/HFS+ from anything inserted into USB (e.g. what one may think is a charger). This creates interesting security consequences for most Linux users.
An image may also be corrupted non-deliberately, which will lead to random memory corruptions if the kernel trusts it blindly.

Quoted 3 lines

Then we should delete the HFS/HFS+ filesystems. They're orphaned in MAINTAINERS and if distros are going to do such a damnfool thing, then we must stop them.

Both HFS and HFS+ work perfectly fine. And if distributions or users are so sensitive about security, it's up to them to blacklist individual features in the kernel.

Both HFS and HFS+ have been the default filesystem on MacOS for 30 years and I don't think it's justified to introduce such a hard compatibility breakage just because some people are worried about theoretical evil maid attacks.

HFS/HFS+ mandatory if you want to boot Linux on a classic Mac or PowerMac and I don't think it's okay to break all these systems running Linux.

Thanks,
Adrian

Installing a new kernel to the HFS filesystem, or a boot loader like yaboot, might be another matter. But there still is an user space option like hfsutils or hfsplus.

That said, in terms of the argument about USB media with corrupt HFS filesystems presenting a security risk, I take the view that once you have physical access to a system, all bets are off. Doubly so if auto-mounting USB media is enabled.

Quoted 7 lines · Viacheslav Dubeyko

Quoted 6 lines · Viacheslav Dubeyko

Quoted 1 line

Also, as far as I can see, available volume in report (mount_0.gz) somehow corrupted already:

Syzbot generates deliberately-corrupted (aka fuzzed) filesystem images. So basically, you can't trust anything you read from the disc.

Quoted 7 lines

If the volume has been deliberately corrupted, then no guarantee that file system driver will behave nicely. Technically speaking, inode write operation should never happened for corrupted volume because the corruption should be detected during b-tree node initialization time. If we would like to achieve such nice state of HFS/HFS+ drivers, then it requires a lot of refactoring/implementation efforts. I am not sure that it is worth to do because not so many guys really use HFS/HFS+ as the main file system under Linux.

Most popular distros will happily auto-mount HFS/HFS+ from anything inserted into USB (e.g. what one may think is a charger). This creates interesting security consequences for most Linux users.
An image may also be corrupted non-deliberately, which will lead to random memory corruptions if the kernel trusts it blindly.

Quoted 6 lines

I suspect this code is basically all dead. From what I can tell, hfs only gets updates for

(a) syzbot reports

(b) vfs interface changes

There is clearly no new work going into it, and most data exchange with MacOS would use HFS+, but I think there are still some users.

Quoted 11 lines

and the last real changes seem to have been by Ernesto A. Fernández back in 2018.

Hmm. Looking at that code, we have another bug in there, introduced by an earlier fix for a similar issue: commit 8d824e69d9f3 ("hfs: fix OOB Read in __hfs_brec_find") added

(quoted diff) +2 -0

+       if (HFS_I(main_inode)->cat_key.CName.len > HFS_NAMELEN)
+               return -EIO;

Quoted 6 lines

but it's after hfs_find_init(), so it should actually have done a hfs_find_exit() to not leak memory.

So we should probably fix that too.

Something like this ENTIRELY UNTESTED patch?

Quoted 2 lines

Do we have anybody who looks at hfs?

Adding Viacheslav Dubeyko to Cc, he's at least been reviewing patches for HFS and HFS+ somewhat recently. The linux-m68k list may have some users dual-booting old MacOS.

Viacheslav, see the start of the thread at
https://lore.kernel.org/lkml/[email protected]/

Arnd

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?

Quoted 4 lines

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

(quoted diff) +2 -0

+               if (fd.entrylength < sizeof(struct hfs_cat_file))
+                       goto out;

would seem to be the right thing anyway. No?

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.

Linus

Hi Linus,

Am 06.01.2023 um 10:53 schrieb Linus Torvalds:

Quoted 13 lines

Quoted 4 lines · Michael Schmitz

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

Quoted 4 lines

(quoted diff) +2 -0

+               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.