When a corrupt HFS+ image is mounted where the hidden directory is
absent, hfsplus_fill_super() handles -ENOENT from
hfsplus_get_hidden_dir_entry() silently and continues with
sbi->hidden_dir left as NULL.

hfsplus_link() and hfsplus_unlink() call
hfsplus_cat_write_inode(sbi->hidden_dir) unconditionally without
checking whether hidden_dir is NULL, triggering a general protection
fault caught by KASAN as a null-ptr-deref at offset 0x28.

Other call sites in dir.c already guard against this by checking
hidden_dir for NULL before use. Apply the same guard to the two
unprotected call sites in hfsplus_link() and hfsplus_unlink().

Changes in v3:
  - Correct fix location: guard sbi->hidden_dir before calling
    hfsplus_cat_write_inode() in hfsplus_link() and hfsplus_unlink()
    in dir.c, as suggested by Vyacheslav Dubeyko.

Changes in v2:
  - Fixed commit message: hfsplus_delete_cat() is called from
    multiple callers, not just hfsplus_unlink() as incorrectly
    stated in v1.

Reported-by: syzbot+c0ba772a362e70937dfb@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=c0ba772a362e70937dfb
Tested-by: syzbot+c0ba772a362e70937dfb@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 fs/hfsplus/dir.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index d559bf8625f8..a7feef53d8cb 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -362,9 +362,11 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir,
 	if (res)
 		goto out;
 
-	res = hfsplus_cat_write_inode(sbi->hidden_dir);
-	if (res)
-		goto out;
+	if (sbi->hidden_dir) {
+		res = hfsplus_cat_write_inode(sbi->hidden_dir);
+		if (res)
+			goto out;
+	}
 
 	res = hfsplus_cat_write_inode(inode);
 
@@ -431,11 +433,10 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry)
 out:
 	if (!res) {
 		res = hfsplus_cat_write_inode(dir);
-		if (!res) {
+		if (!res && sbi->hidden_dir)
 			res = hfsplus_cat_write_inode(sbi->hidden_dir);
-			if (!res)
-				res = hfsplus_cat_write_inode(inode);
-		}
+		if (!res)
+			res = hfsplus_cat_write_inode(inode);
 	}
 
 	mutex_unlock(&sbi->vh_mutex);
-- 
2.43.0

On Wed, 2026-04-15 at 07:13 +0530, Deepanshu Kartikey wrote:
> When a corrupt HFS+ image is mounted where the hidden directory is
> absent, hfsplus_fill_super() handles -ENOENT from
> hfsplus_get_hidden_dir_entry() silently and continues with
> sbi->hidden_dir left as NULL.
> 
> hfsplus_link() and hfsplus_unlink() call
> hfsplus_cat_write_inode(sbi->hidden_dir) unconditionally without
> checking whether hidden_dir is NULL, triggering a general protection
> fault caught by KASAN as a null-ptr-deref at offset 0x28.
> 
> Other call sites in dir.c already guard against this by checking
> hidden_dir for NULL before use. Apply the same guard to the two
> unprotected call sites in hfsplus_link() and hfsplus_unlink().
> 
> Changes in v3:
>   - Correct fix location: guard sbi->hidden_dir before calling
>     hfsplus_cat_write_inode() in hfsplus_link() and hfsplus_unlink()
>     in dir.c, as suggested by Vyacheslav Dubeyko.
> 
> Changes in v2:
>   - Fixed commit message: hfsplus_delete_cat() is called from
>     multiple callers, not just hfsplus_unlink() as incorrectly
>     stated in v1.
> 
> Reported-by: syzbot+c0ba772a362e70937dfb@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=c0ba772a362e70937dfb
> Tested-by: syzbot+c0ba772a362e70937dfb@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
>  fs/hfsplus/dir.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
> index d559bf8625f8..a7feef53d8cb 100644
> --- a/fs/hfsplus/dir.c
> +++ b/fs/hfsplus/dir.c
> @@ -362,9 +362,11 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir,
>  	if (res)
>  		goto out;
>  
> -	res = hfsplus_cat_write_inode(sbi->hidden_dir);
> -	if (res)
> -		goto out;
> +	if (sbi->hidden_dir) {
> +		res = hfsplus_cat_write_inode(sbi->hidden_dir);
> +		if (res)
> +			goto out;
> +	}
>  
>  	res = hfsplus_cat_write_inode(inode);
>  
> @@ -431,11 +433,10 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry)
>  out:
>  	if (!res) {
>  		res = hfsplus_cat_write_inode(dir);
> -		if (!res) {
> +		if (!res && sbi->hidden_dir)
>  			res = hfsplus_cat_write_inode(sbi->hidden_dir);
> -			if (!res)
> -				res = hfsplus_cat_write_inode(inode);
> -		}
> +		if (!res)
> +			res = hfsplus_cat_write_inode(inode);
>  	}
>  
>  	mutex_unlock(&sbi->vh_mutex);


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

On Thu, Apr 16, 2026 at 3:18 AM Viacheslav Dubeyko <vdubeyko@redhat.com> 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

On Tue, 2026-04-21 at 07:14 +0530, Deepanshu Kartikey wrote:
> On Thu, Apr 16, 2026 at 3:18 AM Viacheslav Dubeyko <vdubeyko@redhat.com> 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.
> 

Makes sense.

> I will send v4 shortly.
> 
> 

Sounds good.

Thanks,
Slava.