Re: [PATCH v2] hfsplus: fix ignored error return in hfsplus_delete_cat

[Deepanshu Kartikey]

On Wed, Apr 15, 2026 at 12:59 AM Viacheslav Dubeyko <vdubeyko@xxxxxxxxxx> wrote:
>
> Frankly speaking, I don't quite follow how your fix relates to the initial
> problem?
>
> vfs_unlink+0x272/0x6d0 fs/namei.c:5476
>
> error = dir->i_op->unlink(dir, dentry);
>
> hfsplus_unlink+0x57a/0x930 fs/hfsplus/dir.c:435
>
> res = hfsplus_cat_write_inode(sbi->hidden_dir);
>
> RIP: 0010:hfsplus_cat_write_inode+0xbe/0x8f0 fs/hfsplus/inode.c:637
>
> struct hfs_btree *tree = HFSPLUS_SB(inode->i_sb)->cat_tree;
>
> It looks like that sbi->hidden_dir is NULL and this is why crash here.
>
> But how have it happened at first place?
>

Hi Vyacheslav,

Thank you for the review and for pushing back on the commit message.

Let me explain my initial approach. When I saw "hfsplus: xattr search
failed" in the crash log, I traced it to hfsplus_delete_all_attrs()
in hfsplus_delete_cat(). I noticed the return value was completely
ignored and thought that was the root cause — that the ignored error
was allowing execution to continue into hfsplus_cat_write_inode()
with the filesystem in a bad state.

However after your feedback I looked more carefully at the actual
crash site:

    hfsplus_unlink+0x57a/0x930 fs/hfsplus/dir.c:435
    res = hfsplus_cat_write_inode(sbi->hidden_dir);

You are right — sbi->hidden_dir is NULL here. When a corrupt image
is mounted where the hidden directory is absent.

hfsplus_link() and hfsplus_unlink() then call
hfsplus_cat_write_inode(sbi->hidden_dir) unconditionally without
checking for NULL. Other call sites in dir.c already guard against
this; these two were simply missed.

I will send v3 with the correct fix in dir.c.

I also want to be transparent — I used Claude AI as a learning tool
while working through this bug, and used it to help generate the
commit message. I should have verified the root cause more carefully
before sending. I apologize for the noise.

Regards,
Deeanshu Kartikey