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.
