Re: [PATCH v3] hfsplus: fix null-ptr-deref in hfsplus_cat_write_inode

[Deepanshu Kartikey]

On Thu, Apr 16, 2026 at 3:18 AM Viacheslav Dubeyko <vdubeyko@xxxxxxxxxx> wrote:
>
> As far as I can see, we have such logic in hfsplus_fill_super() [1]:
>
> if (!sb_rdonly(sb)) {
> <skipped>
>     if (!sbi->hidden_dir) {
>         <create hidden dir>
>     }
> <skipped>
> }
>
> So, I have two questions:
> (1) If we mounted volume in READ-ONLY mode initially and we re-mount it into
> READ-WRITE mode, then we skip hidden directory creation. Should we process this
> in hfsplus_reconfigure()?
> (2) Should we create hidden directory if corrupted volume hasn't hidden
> directory?
>
> If we don't create the hidden dir, then we could have troubles in multiple
> places: hfsplus_rename_cat() [2,3], hfsplus_delete_cat() [4,5]. It looks like
> that we need to create the hidden dir. Otherwise, we could trigger the kernel
> crash in multiple places.
>
> Thanks,
> Slava.
>
> [1] https://elixir.bootlin.com/linux/v7.0/source/fs/hfsplus/super.c#L589
> [2] https://elixir.bootlin.com/linux/v7.0/source/fs/hfsplus/dir.c#L326
> [3] https://elixir.bootlin.com/linux/v7.0/source/fs/hfsplus/dir.c#L397
> [4] https://elixir.bootlin.com/linux/v7.0/source/fs/hfsplus/dir.c#L418
> [5] https://elixir.bootlin.com/linux/v7.0/source/fs/hfsplus/inode.c#L257
>

Hi Vyacheslav,

Thank you for the detailed feedback and for pointing out the deeper
design issue.

You are absolutely right on both counts.

(1) hfsplus_reconfigure() does not create the hidden directory when
remounting from read-only to read-write. This means sbi->hidden_dir
can be NULL on a writable mount even if the volume is otherwise
healthy. This needs to be fixed in hfsplus_reconfigure().

(2) The correct approach is not to add NULL checks at individual
call sites — that is a band-aid. The real fix is to ensure
sbi->hidden_dir is always valid on any read-write mount, enforcing
this invariant at mount and remount time.

My plan for v4:
- Extract the hidden directory creation logic from hfsplus_fill_super()
  into a helper function hfsplus_create_hidden_dir().
- Call it from hfsplus_fill_super() as before.
- Also call it from hfsplus_reconfigure() when switching from
  read-only to read-write and hidden_dir is NULL.

This way hidden_dir is guaranteed valid for all write operations
and the individual call sites in dir.c and inode.c are safe
without needing NULL guards.

I will send v4 shortly.

Regards,
Deeanshu Kartikey