Re: [PATCH] hfsplus: set attributes inode dirty at correct position

Viacheslav Dubeyko
2026-04-15T22:11:18Z

On Wed, 2026-04-15 at 16:45 +0800, Edward Adam Davis wrote:
> Syzbot reported a null-ptr-deref in [1].
> If the attributes file is not loaded during system mount, a trigger
> occurs [1] when setxattr is executed in userspace.
>
> Move the mark inode dirty operation to a point after the attr_tree has
> been successfully acquired.
>
> [1]
> KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
> Call Trace:
> hfsplus_setxattr+0x124/0x340 fs/hfsplus/xattr.c:555
> hfsplus_trusted_setxattr+0x40/0x60 fs/hfsplus/xattr_trusted.c:30
> __vfs_setxattr+0x43c/0x480 fs/xattr.c:218
> __vfs_setxattr_noperm+0x12d/0x660 fs/xattr.c:252
> vfs_setxattr+0x163/0x360 fs/xattr.c:339
> do_setxattr fs/xattr.c:654 [inline]
>
> Reported-by: syzbot+bc70a12e438dadba4fb4@syzkaller.appspotmail.com
> Fixes: ee8422d00b7c ("hfsplus: fix potential Allocation File corruption after fsync")
> Closes: https://syzkaller.appspot.com/bug?extid=bc70a12e438dadba4fb4
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> fs/hfsplus/xattr.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
> index 452a1f9becb2..3e6f45b3259d 100644
> --- a/fs/hfsplus/xattr.c
> +++ b/fs/hfsplus/xattr.c
> @@ -317,12 +317,14 @@ static int hfsplus_create_attributes_file(struct super_block *sb)
> next_node++;
> }
>
> - hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(sb), HFSPLUS_I_ATTR_DIRTY);

It's really strange that xfstests didn't catch the issue. Probably, we need to
have the specialized HFS+ tests.

> hfsplus_mark_inode_dirty(attr_file, HFSPLUS_I_ATTR_DIRTY);
>
> sbi->attr_tree = hfs_btree_open(sb, HFSPLUS_ATTR_CNID);
> if (!sbi->attr_tree)
> pr_err("failed to load attributes file\n");
> + else
> + hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(sb),
> + HFSPLUS_I_ATTR_DIRTY);

As far as I can see, HFSPLUS_ATTR_TREE_I(sb) and attr_file are the same
entities. Am I right here? :) So, we can simply remove the first
hfsplus_mark_inode_dirty(). Does it make sense?

Thanks,
Slava.

>
> failed_header_node_init:
> kfree(buf);