linux-kernel - Re: [PATCH] hfsplus: don't use BUG_ON() in hfsplus_create_attributes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b6da38b0-dc7e-4fdc-b99c-f4fbd2a20168@I-love.SAKURA.ne.jp>
Date: Sat, 12 Jul 2025 20:22:20 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Viacheslav Dubeyko <Slava.Dubeyko@....com>,
        "frank.li@...o.com" <frank.li@...o.com>,
        "glaubitz@...sik.fu-berlin.de" <glaubitz@...sik.fu-berlin.de>,
        "slava@...eyko.com" <slava@...eyko.com>,
        "brauner@...nel.org" <brauner@...nel.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>
Cc: "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] hfsplus: don't use BUG_ON() in
 hfsplus_create_attributes_file()

On 2025/07/12 2:21, Viacheslav Dubeyko wrote:
> Frankly speaking, I still don't see the whole picture here. If we have created
> the Attribute File during mount operation, then why should we try to create the
> Attributes File during __hfsplus_setxattr() call? If we didn't create the
> Attributes File during the mount time and HFSPLUS_SB(inode->i_sb)->attr_tree is
> NULL, then how i_size_read(attr_file) != 0? Even if we are checking vhdr-
>> attr_file.total_blocks, then it doesn't provide guarantee that
> i_size_read(attr_file) is zero too. Something is wrong in this situation and
> more stricter mount time validation cannot guarantee against the situation that
> you are trying to solve in the issue. We are missing something here.

I still don't see what you are missing.

When hfsplus_iget(sb, HFSPLUS_ATTR_CNID) is called from hfsplus_create_attributes_file(sb),
hfsplus_system_read_inode(inode) from hfsplus_iget(HFSPLUS_ATTR_CNID) calls
hfsplus_inode_read_fork(inode, &vhdr->attr_file). Since hfsplus_inode_read_fork() calls
inode_set_bytes(), it is natural that i_size_read(attr_file) != 0 when returning from
hfsplus_iget(sb, HFSPLUS_ATTR_CNID).

At this point, the only question should be why hfsplus_inode_read_fork() from
hfsplus_system_read_inode(inode) from hfsplus_iget() is not called from hfsplus_fill_super()
when the Attributes File already exists and its size is not 0. And the reason is that
hfsplus_iget(sb, HFSPLUS_ATTR_CNID) from hfs_btree_open(sb, HFSPLUS_ATTR_CNID) is called
only when vhdr->attr_file.total_blocks != 0.

That is, when "vhdr" contains erroneous values (in the reproducer, vhdr->attr_file.total_blocks
is 0) that do not reflect the actual state of the filesystem (in the reproducer, inode_set_bytes()
sets non-zero value despite vhdr->attr_file.total_blocks is 0), hfsplus_fill_super() fails to call
hfs_btree_open(sb, HFSPLUS_ATTR_CNID) at mount time.

You can easily reproduce this problem by compiling and running the reproducer
at https://syzkaller.appspot.com/text?tag=ReproC&x=15f6b9d4580000 after you run
"losetup -f" which creates /dev/loop0 needed by the reproducer.

I noticed that the reason fsck.hfsplus could not detect errors is that the filesystem
image in the reproducer was compressed. If I run fsck.hfsplus on uncompressed image,
fsck.hfsplus generated the following messages.

# fsck.hfsplus hfsplus.img
** hfsplus.img
   Executing fsck_hfs (version 540.1-Linux).
** Checking non-journaled HFS Plus Volume.
   The volume name is untitled
** Checking extents overflow file.
** Checking catalog file.
   Invalid extent entry
(4, 1)
** Checking multi-linked files.
** Checking catalog hierarchy.
** Checking extended attributes file.
** Checking volume bitmap.
** Checking volume information.
** Repairing volume.
   Look for links to corrupt files in DamagedFiles directory.
** Rechecking volume.
** Checking non-journaled HFS Plus Volume.
   The volume name is untitled
** Checking extents overflow file.
** Checking catalog file.
** Checking multi-linked files.
** Checking catalog hierarchy.
** Checking extended attributes file.
** Checking volume bitmap.
   Volume bitmap needs minor repair for under-allocation
** Checking volume information.
   Invalid volume free block count
   (It should be 179 instead of 180)
** Repairing volume.
** Rechecking volume.
** Checking non-journaled HFS Plus Volume.
   The volume name is untitled
** Checking extents overflow file.
** Checking catalog file.
** Checking multi-linked files.
** Checking catalog hierarchy.
** Checking extended attributes file.
** Checking volume bitmap.
** Checking volume information.
** The volume untitled was repaired successfully.