From 05ce49a902be15dc93854cbfc20161205a9ee446 Mon Sep 17 00:00:00 2001 From: Mehdi Ben Hadj Khelifa Date: Mon, 1 Dec 2025 23:23:06 +0100 Subject: [PATCH 01/12] hfs: ensure sb->s_fs_info is always cleaned up When hfs was converted to the new mount api a bug was introduced by changing the allocation pattern of sb->s_fs_info. If setup_bdev_super() fails after a new superblock has been allocated by sget_fc(), but before hfs_fill_super() takes ownership of the filesystem-specific s_fs_info data it was leaked. Fix this by freeing sb->s_fs_info in hfs_kill_super(). Cc: stable@vger.kernel.org Fixes: ffcd06b6d13b ("hfs: convert hfs to use the new mount api") Reported-by: syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=ad45f827c88778ff7df6 Tested-by: Viacheslav Dubeyko Signed-off-by: Christian Brauner Signed-off-by: Mehdi Ben Hadj Khelifa Reviewed-by: Viacheslav Dubeyko Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20251201222843.82310-2-mehdi.benhadjkhelifa@gmail.com Signed-off-by: Viacheslav Dubeyko --- fs/hfs/mdb.c | 35 ++++++++++++++--------------------- fs/hfs/super.c | 10 +++++++++- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/fs/hfs/mdb.c b/fs/hfs/mdb.c index 53f3fae60217..f28cd24dee84 100644 --- a/fs/hfs/mdb.c +++ b/fs/hfs/mdb.c @@ -92,7 +92,7 @@ int hfs_mdb_get(struct super_block *sb) /* See if this is an HFS filesystem */ bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb); if (!bh) - goto out; + return -EIO; if (mdb->drSigWord == cpu_to_be16(HFS_SUPER_MAGIC)) break; @@ -102,13 +102,14 @@ int hfs_mdb_get(struct super_block *sb) * (should do this only for cdrom/loop though) */ if (hfs_part_find(sb, &part_start, &part_size)) - goto out; + return -EIO; } HFS_SB(sb)->alloc_blksz = size = be32_to_cpu(mdb->drAlBlkSiz); if (!size || (size & (HFS_SECTOR_SIZE - 1))) { pr_err("bad allocation block size %d\n", size); - goto out_bh; + brelse(bh); + return -EIO; } size = min(HFS_SB(sb)->alloc_blksz, (u32)PAGE_SIZE); @@ -125,14 +126,16 @@ int hfs_mdb_get(struct super_block *sb) brelse(bh); if (!sb_set_blocksize(sb, size)) { pr_err("unable to set blocksize to %u\n", size); - goto out; + return -EIO; } bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb); if (!bh) - goto out; - if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC)) - goto out_bh; + return -EIO; + if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC)) { + brelse(bh); + return -EIO; + } HFS_SB(sb)->mdb_bh = bh; HFS_SB(sb)->mdb = mdb; @@ -174,7 +177,7 @@ int hfs_mdb_get(struct super_block *sb) HFS_SB(sb)->bitmap = kzalloc(8192, GFP_KERNEL); if (!HFS_SB(sb)->bitmap) - goto out; + return -EIO; /* read in the bitmap */ block = be16_to_cpu(mdb->drVBMSt) + part_start; @@ -185,7 +188,7 @@ int hfs_mdb_get(struct super_block *sb) bh = sb_bread(sb, off >> sb->s_blocksize_bits); if (!bh) { pr_err("unable to read volume bitmap\n"); - goto out; + return -EIO; } off2 = off & (sb->s_blocksize - 1); len = min((int)sb->s_blocksize - off2, size); @@ -199,12 +202,12 @@ int hfs_mdb_get(struct super_block *sb) HFS_SB(sb)->ext_tree = hfs_btree_open(sb, HFS_EXT_CNID, hfs_ext_keycmp); if (!HFS_SB(sb)->ext_tree) { pr_err("unable to open extent tree\n"); - goto out; + return -EIO; } HFS_SB(sb)->cat_tree = hfs_btree_open(sb, HFS_CAT_CNID, hfs_cat_keycmp); if (!HFS_SB(sb)->cat_tree) { pr_err("unable to open catalog tree\n"); - goto out; + return -EIO; } attrib = mdb->drAtrb; @@ -229,12 +232,6 @@ int hfs_mdb_get(struct super_block *sb) } return 0; - -out_bh: - brelse(bh); -out: - hfs_mdb_put(sb); - return -EIO; } /* @@ -359,8 +356,6 @@ void hfs_mdb_close(struct super_block *sb) * Release the resources associated with the in-core MDB. */ void hfs_mdb_put(struct super_block *sb) { - if (!HFS_SB(sb)) - return; /* free the B-trees */ hfs_btree_close(HFS_SB(sb)->ext_tree); hfs_btree_close(HFS_SB(sb)->cat_tree); @@ -373,6 +368,4 @@ void hfs_mdb_put(struct super_block *sb) unload_nls(HFS_SB(sb)->nls_disk); kfree(HFS_SB(sb)->bitmap); - kfree(HFS_SB(sb)); - sb->s_fs_info = NULL; } diff --git a/fs/hfs/super.c b/fs/hfs/super.c index 47f50fa555a4..df289cbdd4e8 100644 --- a/fs/hfs/super.c +++ b/fs/hfs/super.c @@ -431,10 +431,18 @@ static int hfs_init_fs_context(struct fs_context *fc) return 0; } +static void hfs_kill_super(struct super_block *sb) +{ + struct hfs_sb_info *hsb = HFS_SB(sb); + + kill_block_super(sb); + kfree(hsb); +} + static struct file_system_type hfs_fs_type = { .owner = THIS_MODULE, .name = "hfs", - .kill_sb = kill_block_super, + .kill_sb = hfs_kill_super, .fs_flags = FS_REQUIRES_DEV, .init_fs_context = hfs_init_fs_context, }; From 126fb0ce99431126b44a6c360192668c818f641f Mon Sep 17 00:00:00 2001 From: Mehdi Ben Hadj Khelifa Date: Mon, 1 Dec 2025 23:23:07 +0100 Subject: [PATCH 02/12] hfsplus: ensure sb->s_fs_info is always cleaned up When hfsplus was converted to the new mount api a bug was introduced by changing the allocation pattern of sb->s_fs_info. If setup_bdev_super() fails after a new superblock has been allocated by sget_fc(), but before hfsplus_fill_super() takes ownership of the filesystem-specific s_fs_info data it was leaked. Fix this by freeing sb->s_fs_info in hfsplus_kill_super(). Cc: stable@vger.kernel.org Fixes: 432f7c78cb00 ("hfsplus: convert hfsplus to use the new mount api") Reported-by: Viacheslav Dubeyko Tested-by: Viacheslav Dubeyko Signed-off-by: Christian Brauner Signed-off-by: Mehdi Ben Hadj Khelifa Reviewed-by: Viacheslav Dubeyko Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20251201222843.82310-3-mehdi.benhadjkhelifa@gmail.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/super.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index aaffa9e060a0..829c8ac2639c 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -344,8 +344,6 @@ static void hfsplus_put_super(struct super_block *sb) hfs_btree_close(sbi->ext_tree); kfree(sbi->s_vhdr_buf); kfree(sbi->s_backup_vhdr_buf); - call_rcu(&sbi->rcu, delayed_free); - hfs_dbg("finished\n"); } @@ -650,7 +648,6 @@ out_free_vhdr: out_unload_nls: unload_nls(sbi->nls); unload_nls(nls); - kfree(sbi); return err; } @@ -709,10 +706,18 @@ static int hfsplus_init_fs_context(struct fs_context *fc) return 0; } +static void hfsplus_kill_super(struct super_block *sb) +{ + struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb); + + kill_block_super(sb); + call_rcu(&sbi->rcu, delayed_free); +} + static struct file_system_type hfsplus_fs_type = { .owner = THIS_MODULE, .name = "hfsplus", - .kill_sb = kill_block_super, + .kill_sb = hfsplus_kill_super, .fs_flags = FS_REQUIRES_DEV, .init_fs_context = hfsplus_init_fs_context, }; From bea4429eb30190c59b5ac7c8ff6c90176c7c110f Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Thu, 4 Dec 2025 16:00:55 -0800 Subject: [PATCH 03/12] hfsplus: fix volume corruption issue for generic/480 The xfstests' test-case generic/480 leaves HFS+ volume in corrupted state: sudo ./check generic/480 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.17.0-rc1+ #4 SMP PREEMPT_DYNAMIC Wed Oct 1 15:02:44 PDT 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/480 _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent (see XFSTESTS-2/xfstests-dev/results//generic/480.full for details) Ran: generic/480 Failures: generic/480 Failed 1 of 1 tests sudo fsck.hfsplus -d /dev/loop51 ** /dev/loop51 Using cacheBlockSize=32K cacheTotalBlock=1024 cacheSize=32768K. 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. ** Checking multi-linked files. CheckHardLinks: found 1 pre-Leopard file inodes. Incorrect number of file hard links ** Checking catalog hierarchy. ** Checking extended attributes file. ** Checking volume bitmap. ** Checking volume information. invalid VHB nextCatalogID Volume header needs minor repair (2, 0) Verify Status: VIStat = 0x8000, ABTStat = 0x0000 EBTStat = 0x0000 CBTStat = 0x0000 CatStat = 0x00000002 ** Repairing volume. Incorrect flags for file hard link (id = 19) (It should be 0x22 instead of 0x2) Incorrect flags for file inode (id = 18) (It should be 0x22 instead of 0x2) first link ID=0 is < 16 for fileinode=18 Error getting first link ID for inode = 18 (result=2) Invalid first link in hard link chain (id = 18) (It should be 19 instead of 0) Indirect node 18 needs link count adjustment (It should be 1 instead of 2) ** 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. The generic/480 test executes such steps on final phase: "Now remove of the links of our file and create a new file with the same name and in the same parent directory, and finally fsync this new file." unlink $SCRATCH_MNT/testdir/bar touch $SCRATCH_MNT/testdir/bar $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir/bar "Simulate a power failure and mount the filesystem to check that replaying the fsync log/journal succeeds, that is the mount operation does not fail." _flakey_drop_and_remount The key issue in HFS+ logic is that hfsplus_link(), hfsplus_unlink(), hfsplus_rmdir(), hfsplus_symlink(), and hfsplus_mknod() methods don't call hfsplus_cat_write_inode() for the case of modified inode objects. As a result, even if hfsplus_file_fsync() is trying to flush the dirty Catalog File, but because of not calling hfsplus_cat_write_inode() not all modified inodes save the new state into Catalog File's records. Finally, simulation of power failure results in inconsistent state of Catalog File and FSCK tool reports about volume corruption. This patch adds calling of hfsplus_cat_write_inode() method for modified inodes in hfsplus_link(), hfsplus_unlink(), hfsplus_rmdir(), hfsplus_symlink(), and hfsplus_mknod() methods. Also, it adds debug output in several methods. sudo ./check generic/480 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.18.0-rc1+ #18 SMP PREEMPT_DYNAMIC Thu Dec 4 12:24:45 PST 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/480 16s ... 16s Ran: generic/480 Passed all 1 tests Signed-off-by: Viacheslav Dubeyko cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Link: https://lore.kernel.org/r/20251205000054.3670326-1-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/dir.c | 46 +++++++++++++++++++++++++++++++++++++++++++++- fs/hfsplus/inode.c | 5 +++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c index cadf0b5f9342..ca5f74a140ec 100644 --- a/fs/hfsplus/dir.c +++ b/fs/hfsplus/dir.c @@ -313,6 +313,9 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir, if (!S_ISREG(inode->i_mode)) return -EPERM; + hfs_dbg("src_dir->i_ino %lu, dst_dir->i_ino %lu, inode->i_ino %lu\n", + src_dir->i_ino, dst_dir->i_ino, inode->i_ino); + mutex_lock(&sbi->vh_mutex); if (inode->i_ino == (u32)(unsigned long)src_dentry->d_fsdata) { for (;;) { @@ -332,7 +335,7 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir, cnid = sbi->next_cnid++; src_dentry->d_fsdata = (void *)(unsigned long)cnid; res = hfsplus_create_cat(cnid, src_dir, - &src_dentry->d_name, inode); + &src_dentry->d_name, inode); if (res) /* panic? */ goto out; @@ -350,6 +353,21 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir, mark_inode_dirty(inode); sbi->file_count++; hfsplus_mark_mdb_dirty(dst_dir->i_sb); + + res = hfsplus_cat_write_inode(src_dir); + if (res) + goto out; + + res = hfsplus_cat_write_inode(dst_dir); + if (res) + goto out; + + res = hfsplus_cat_write_inode(sbi->hidden_dir); + if (res) + goto out; + + res = hfsplus_cat_write_inode(inode); + out: mutex_unlock(&sbi->vh_mutex); return res; @@ -367,6 +385,9 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry) if (HFSPLUS_IS_RSRC(inode)) return -EPERM; + hfs_dbg("dir->i_ino %lu, inode->i_ino %lu\n", + dir->i_ino, inode->i_ino); + mutex_lock(&sbi->vh_mutex); cnid = (u32)(unsigned long)dentry->d_fsdata; if (inode->i_ino == cnid && @@ -408,6 +429,15 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry) inode_set_ctime_current(inode); mark_inode_dirty(inode); out: + if (!res) { + res = hfsplus_cat_write_inode(dir); + if (!res) { + res = hfsplus_cat_write_inode(sbi->hidden_dir); + if (!res) + res = hfsplus_cat_write_inode(inode); + } + } + mutex_unlock(&sbi->vh_mutex); return res; } @@ -429,6 +459,8 @@ static int hfsplus_rmdir(struct inode *dir, struct dentry *dentry) inode_set_ctime_current(inode); hfsplus_delete_inode(inode); mark_inode_dirty(inode); + + res = hfsplus_cat_write_inode(dir); out: mutex_unlock(&sbi->vh_mutex); return res; @@ -465,6 +497,12 @@ static int hfsplus_symlink(struct mnt_idmap *idmap, struct inode *dir, hfsplus_instantiate(dentry, inode, inode->i_ino); mark_inode_dirty(inode); + + res = hfsplus_cat_write_inode(dir); + if (res) + goto out; + + res = hfsplus_cat_write_inode(inode); goto out; out_err: @@ -506,6 +544,12 @@ static int hfsplus_mknod(struct mnt_idmap *idmap, struct inode *dir, hfsplus_instantiate(dentry, inode, inode->i_ino); mark_inode_dirty(inode); + + res = hfsplus_cat_write_inode(dir); + if (res) + goto out; + + res = hfsplus_cat_write_inode(inode); goto out; failed_mknod: diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c index 7ae6745ca7ae..c762bf909d1a 100644 --- a/fs/hfsplus/inode.c +++ b/fs/hfsplus/inode.c @@ -328,6 +328,9 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end, struct hfsplus_vh *vhdr = sbi->s_vhdr; int error = 0, error2; + hfs_dbg("inode->i_ino %lu, start %llu, end %llu\n", + inode->i_ino, start, end); + error = file_write_and_wait_range(file, start, end); if (error) return error; @@ -616,6 +619,8 @@ int hfsplus_cat_write_inode(struct inode *inode) hfsplus_cat_entry entry; int res = 0; + hfs_dbg("inode->i_ino %lu\n", inode->i_ino); + if (HFSPLUS_IS_RSRC(inode)) main_inode = HFSPLUS_I(inode)->rsrc_inode; From 9a8c4ad44721da4c48e1ff240ac76286c82837fe Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Sat, 6 Dec 2025 19:58:22 -0800 Subject: [PATCH 04/12] hfsplus: fix volume corruption issue for generic/498 The xfstests' test-case generic/498 leaves HFS+ volume in corrupted state: sudo ./check generic/498 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.18.0-rc1+ #18 SMP PREEMPT_DYNAMIC Thu Dec 4 12:24:45 PST 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/498 _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent (see XFSTESTS-2/xfstests-dev/results//generic/498.full for details) Ran: generic/498 Failures: generic/498 Failed 1 of 1 tests sudo fsck.hfsplus -d /dev/loop51 ** /dev/loop51 Using cacheBlockSize=32K cacheTotalBlock=1024 cacheSize=32768K. 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 leaf record count (It should be 16 instead of 2) ** Checking multi-linked files. CheckHardLinks: found 1 pre-Leopard file inodes. ** Checking catalog hierarchy. ** Checking extended attributes file. ** Checking volume bitmap. ** Checking volume information. Verify Status: VIStat = 0x0000, ABTStat = 0x0000 EBTStat = 0x0000 CBTStat = 0x8000 CatStat = 0x00000000 ** 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. CheckHardLinks: found 1 pre-Leopard file inodes. ** Checking catalog hierarchy. ** Checking extended attributes file. ** Checking volume bitmap. ** Checking volume information. ** The volume untitled was repaired successfully. The generic/498 test executes such steps on final phase: mkdir $SCRATCH_MNT/A mkdir $SCRATCH_MNT/B mkdir $SCRATCH_MNT/A/C touch $SCRATCH_MNT/B/foo $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/B/foo ln $SCRATCH_MNT/B/foo $SCRATCH_MNT/A/C/foo $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/A "Simulate a power failure and mount the filesystem to check that what we explicitly fsync'ed exists." _flakey_drop_and_remount The FSCK tool complains about "Invalid leaf record count". HFS+ b-tree header contains leaf_count field is updated by hfs_brec_insert() and hfs_brec_remove(). The hfs_brec_insert() is involved into hard link creation process. However, modified in-core leaf_count field is stored into HFS+ b-tree header by hfs_btree_write() method. But, unfortunately, hfs_btree_write() hasn't been called by hfsplus_cat_write_inode() and hfsplus_file_fsync() stores not fully consistent state of the Catalog File's b-tree. This patch adds calling hfs_btree_write() method in the hfsplus_cat_write_inode() with the goal of storing consistent state of Catalog File's b-tree. Finally, it makes FSCK tool happy. sudo ./check generic/498 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.18.0-rc1+ #22 SMP PREEMPT_DYNAMIC Sat Dec 6 17:01:31 PST 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/498 33s ... 31s Ran: generic/498 Passed all 1 tests Signed-off-by: Viacheslav Dubeyko cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Link: https://lore.kernel.org/r/20251207035821.3863657-1-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/inode.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c index c762bf909d1a..6153e5cc6eb6 100644 --- a/fs/hfsplus/inode.c +++ b/fs/hfsplus/inode.c @@ -615,6 +615,7 @@ out: int hfsplus_cat_write_inode(struct inode *inode) { struct inode *main_inode = inode; + struct hfs_btree *tree = HFSPLUS_SB(inode->i_sb)->cat_tree; struct hfs_find_data fd; hfsplus_cat_entry entry; int res = 0; @@ -627,7 +628,7 @@ int hfsplus_cat_write_inode(struct inode *inode) if (!main_inode->i_nlink) return 0; - if (hfs_find_init(HFSPLUS_SB(main_inode->i_sb)->cat_tree, &fd)) + if (hfs_find_init(tree, &fd)) /* panic? */ return -EIO; @@ -692,6 +693,15 @@ int hfsplus_cat_write_inode(struct inode *inode) set_bit(HFSPLUS_I_CAT_DIRTY, &HFSPLUS_I(inode)->flags); out: hfs_find_exit(&fd); + + if (!res) { + res = hfs_btree_write(tree); + if (res) { + pr_err("b-tree write err: %d, ino %lu\n", + res, inode->i_ino); + } + } + return res; } From 413466f3f0f84e7356da16c611afd69d2a0872e4 Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Tue, 23 Dec 2025 16:28:11 -0800 Subject: [PATCH 05/12] hfsplus: fix generic/020 xfstests failure The xfstests' test-case generic/020 fails to execute correctly: FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.15.0-rc4+ #8 SMP PREEMPT_DYNAMIC Thu May 1 16:43:22 PDT 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/020 _check_generic_filesystem: filesystem on /dev/loop50 is inconsistent (see xfstests-dev/results//generic/020.full for details) *** add lots of attributes *** check *** MAX_ATTRS attribute(s) +/mnt/test/attribute_12286: Numerical result out of range *** -1 attribute(s) *** remove lots of attributes ... (Run 'diff -u /xfstests-dev/tests/generic/020.out /xfstests-dev/results//generic/020.out.bad' to see the entire diff) The generic/020 creates more than 100 xattrs and gives its the names user.attribute_ (for example, user.attribute_101). As the next step, listxattr() is called with the goal to check the correctness of xattrs creation. However, it was issue in hfsplus_listxattr() logic. This method re-uses the fd.key->attr.key_name.unicode and strbuf buffers in the loop without re-initialization. As a result, part of the previous name could still remain in the buffers. For example, user.attribute_101 could be processed before user.attribute_54. The issue resulted in formation the name user.attribute_541 instead of user.attribute_54. This patch adds initialization of fd.key->attr.key_name.unicode and strbuf buffers before calling hfs_brec_goto() method that prepare next name in the buffer. HFS+ logic supports only inline xattrs. Such extended attributes can store values not bigger than 3802 bytes [1]. This limitation requires correction of generic/020 logic. Finally, generic/020 can be executed without any issue: sudo ./check generic/020 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.19.0-rc1+ #44 SMP PREEMPT_DYNAMIC Mon Dec 22 15:39:00 PST 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/020 31s ... 38s Ran: generic/020 Passed all 1 tests [1] https://elixir.bootlin.com/linux/v6.19-rc2/source/include/linux/hfs_common.h#L626 Signed-off-by: Viacheslav Dubeyko cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Link: https://lore.kernel.org/r/20251224002810.1137139-1-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/attributes.c | 14 +++++++-- fs/hfsplus/xattr.c | 70 ++++++++++++++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 14 deletions(-) diff --git a/fs/hfsplus/attributes.c b/fs/hfsplus/attributes.c index ba26980cc503..74b0ca9c4f17 100644 --- a/fs/hfsplus/attributes.c +++ b/fs/hfsplus/attributes.c @@ -117,8 +117,10 @@ static int hfsplus_attr_build_record(hfsplus_attr_entry *entry, int record_type, entry->inline_data.record_type = cpu_to_be32(record_type); if (size <= HFSPLUS_MAX_INLINE_DATA_SIZE) len = size; - else + else { + hfs_dbg("value size %zu is too big\n", size); return HFSPLUS_INVALID_ATTR_RECORD; + } entry->inline_data.length = cpu_to_be16(len); memcpy(entry->inline_data.raw_bytes, value, len); /* @@ -238,7 +240,11 @@ int hfsplus_create_attr(struct inode *inode, inode->i_ino, value, size); if (entry_size == HFSPLUS_INVALID_ATTR_RECORD) { - err = -EINVAL; + if (size > HFSPLUS_MAX_INLINE_DATA_SIZE) + err = -E2BIG; + else + err = -EINVAL; + hfs_dbg("unable to store value: err %d\n", err); goto failed_create_attr; } @@ -250,8 +256,10 @@ int hfsplus_create_attr(struct inode *inode, } err = hfs_brec_insert(&fd, entry_ptr, entry_size); - if (err) + if (err) { + hfs_dbg("unable to store value: err %d\n", err); goto failed_create_attr; + } hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ATTR_DIRTY); diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c index da95a9de9a65..504518a41212 100644 --- a/fs/hfsplus/xattr.c +++ b/fs/hfsplus/xattr.c @@ -345,8 +345,10 @@ int __hfsplus_setxattr(struct inode *inode, const char *name, if (err) goto end_setxattr; err = hfsplus_create_attr(inode, name, value, size); - if (err) + if (err) { + hfs_dbg("unable to store value: err %d\n", err); goto end_setxattr; + } } else { if (flags & XATTR_REPLACE) { pr_err("cannot replace xattr\n"); @@ -354,8 +356,10 @@ int __hfsplus_setxattr(struct inode *inode, const char *name, goto end_setxattr; } err = hfsplus_create_attr(inode, name, value, size); - if (err) + if (err) { + hfs_dbg("unable to store value: err %d\n", err); goto end_setxattr; + } } cat_entry_type = hfs_bnode_read_u16(cat_fd.bnode, cat_fd.entryoffset); @@ -392,9 +396,9 @@ end_setxattr: return err; } -static int name_len(const char *xattr_name, int xattr_name_len) +static size_t name_len(const char *xattr_name, size_t xattr_name_len) { - int len = xattr_name_len + 1; + size_t len = xattr_name_len + 1; if (!is_known_namespace(xattr_name)) len += XATTR_MAC_OSX_PREFIX_LEN; @@ -402,15 +406,22 @@ static int name_len(const char *xattr_name, int xattr_name_len) return len; } -static ssize_t copy_name(char *buffer, const char *xattr_name, int name_len) +static ssize_t copy_name(char *buffer, const char *xattr_name, size_t name_len) { ssize_t len; - if (!is_known_namespace(xattr_name)) + memset(buffer, 0, name_len); + + if (!is_known_namespace(xattr_name)) { len = scnprintf(buffer, name_len + XATTR_MAC_OSX_PREFIX_LEN, "%s%s", XATTR_MAC_OSX_PREFIX, xattr_name); - else + } else { len = strscpy(buffer, xattr_name, name_len + 1); + if (len < 0) { + pr_err("fail to copy name: err %zd\n", len); + len = 0; + } + } /* include NUL-byte in length for non-empty name */ if (len >= 0) @@ -423,16 +434,26 @@ int hfsplus_setxattr(struct inode *inode, const char *name, const char *prefix, size_t prefixlen) { char *xattr_name; + size_t xattr_name_len = + NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1; int res; - xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1, - GFP_KERNEL); + hfs_dbg("ino %lu, name %s, prefix %s, prefixlen %zu, " + "value %p, size %zu\n", + inode->i_ino, name ? name : NULL, + prefix ? prefix : NULL, prefixlen, + value, size); + + xattr_name = kmalloc(xattr_name_len, GFP_KERNEL); if (!xattr_name) return -ENOMEM; strcpy(xattr_name, prefix); strcpy(xattr_name + prefixlen, name); res = __hfsplus_setxattr(inode, xattr_name, value, size, flags); kfree(xattr_name); + + hfs_dbg("finished: res %d\n", res); + return res; } @@ -579,6 +600,10 @@ ssize_t hfsplus_getxattr(struct inode *inode, const char *name, int res; char *xattr_name; + hfs_dbg("ino %lu, name %s, prefix %s\n", + inode->i_ino, name ? name : NULL, + prefix ? prefix : NULL); + xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1, GFP_KERNEL); if (!xattr_name) @@ -589,6 +614,9 @@ ssize_t hfsplus_getxattr(struct inode *inode, const char *name, res = __hfsplus_getxattr(inode, xattr_name, value, size); kfree(xattr_name); + + hfs_dbg("finished: res %d\n", res); + return res; } @@ -679,8 +707,11 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size) struct hfs_find_data fd; struct hfsplus_attr_key attr_key; char *strbuf; + size_t strbuf_size; int xattr_name_len; + hfs_dbg("ino %lu\n", inode->i_ino); + if ((!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode)) || HFSPLUS_IS_RSRC(inode)) @@ -698,8 +729,9 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size) return err; } - strbuf = kzalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + - XATTR_MAC_OSX_PREFIX_LEN + 1, GFP_KERNEL); + strbuf_size = NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + + XATTR_MAC_OSX_PREFIX_LEN + 1; + strbuf = kzalloc(strbuf_size, GFP_KERNEL); if (!strbuf) { res = -ENOMEM; goto out; @@ -746,6 +778,9 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size) res += name_len(strbuf, xattr_name_len); } else if (can_list(strbuf)) { if (size < (res + name_len(strbuf, xattr_name_len))) { + pr_err("size %zu, res %zd, name_len %zu\n", + size, res, + name_len(strbuf, xattr_name_len)); res = -ERANGE; goto end_listxattr; } else @@ -753,6 +788,10 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size) strbuf, xattr_name_len); } + memset(fd.key->attr.key_name.unicode, 0, + sizeof(fd.key->attr.key_name.unicode)); + memset(strbuf, 0, strbuf_size); + if (hfs_brec_goto(&fd, 1)) goto end_listxattr; } @@ -761,6 +800,9 @@ end_listxattr: kfree(strbuf); out: hfs_find_exit(&fd); + + hfs_dbg("finished: res %zd\n", res); + return res; } @@ -773,6 +815,9 @@ static int hfsplus_removexattr(struct inode *inode, const char *name) int is_xattr_acl_deleted; int is_all_xattrs_deleted; + hfs_dbg("ino %lu, name %s\n", + inode->i_ino, name ? name : NULL); + if (!HFSPLUS_SB(inode->i_sb)->attr_tree) return -EOPNOTSUPP; @@ -833,6 +878,9 @@ static int hfsplus_removexattr(struct inode *inode, const char *name) end_removexattr: hfs_find_exit(&cat_fd); + + hfs_dbg("finished: err %d\n", err); + return err; } From b226804532a875c10276168dc55ce752944096bd Mon Sep 17 00:00:00 2001 From: Jori Koolstra Date: Sat, 20 Dec 2025 20:10:06 +0100 Subject: [PATCH 06/12] hfs: Replace BUG_ON with error handling for CNID count checks In a06ec283e125 next_id, folder_count, and file_count in the super block info were expanded to 64 bits, and BUG_ONs were added to detect overflow. This triggered an error reported by syzbot: if the MDB is corrupted, the BUG_ON is triggered. This patch replaces this mechanism with proper error handling and resolves the syzbot reported bug. Singed-off-by: Jori Koolstra Reported-by: syzbot+17cc9bb6d8d69b4139f0@syzkaller.appspotmail.com Closes: https://syzbot.org/bug?extid=17cc9bb6d8d69b4139f0 Signed-off-by: Jori Koolstra Reviewed-by: Viacheslav Dubeyko Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20251220191006.2465256-1-jkoolstra@xs4all.nl Signed-off-by: Viacheslav Dubeyko --- fs/hfs/dir.c | 15 +++++++++++---- fs/hfs/hfs_fs.h | 1 + fs/hfs/inode.c | 30 ++++++++++++++++++++++++------ fs/hfs/mdb.c | 31 +++++++++++++++++++++++++++---- fs/hfs/super.c | 3 +++ 5 files changed, 66 insertions(+), 14 deletions(-) diff --git a/fs/hfs/dir.c b/fs/hfs/dir.c index 86a6b317b474..0c615c078650 100644 --- a/fs/hfs/dir.c +++ b/fs/hfs/dir.c @@ -196,8 +196,8 @@ static int hfs_create(struct mnt_idmap *idmap, struct inode *dir, int res; inode = hfs_new_inode(dir, &dentry->d_name, mode); - if (!inode) - return -ENOMEM; + if (IS_ERR(inode)) + return PTR_ERR(inode); res = hfs_cat_create(inode->i_ino, dir, &dentry->d_name, inode); if (res) { @@ -226,8 +226,8 @@ static struct dentry *hfs_mkdir(struct mnt_idmap *idmap, struct inode *dir, int res; inode = hfs_new_inode(dir, &dentry->d_name, S_IFDIR | mode); - if (!inode) - return ERR_PTR(-ENOMEM); + if (IS_ERR(inode)) + return ERR_CAST(inode); res = hfs_cat_create(inode->i_ino, dir, &dentry->d_name, inode); if (res) { @@ -254,11 +254,18 @@ static struct dentry *hfs_mkdir(struct mnt_idmap *idmap, struct inode *dir, */ static int hfs_remove(struct inode *dir, struct dentry *dentry) { + struct super_block *sb = dir->i_sb; struct inode *inode = d_inode(dentry); int res; if (S_ISDIR(inode->i_mode) && inode->i_size != 2) return -ENOTEMPTY; + + if (unlikely(!is_hfs_cnid_counts_valid(sb))) { + pr_err("cannot remove file/folder\n"); + return -ERANGE; + } + res = hfs_cat_delete(inode->i_ino, dir, &dentry->d_name); if (res) return res; diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h index e94dbc04a1e4..ac0e83f77a0f 100644 --- a/fs/hfs/hfs_fs.h +++ b/fs/hfs/hfs_fs.h @@ -199,6 +199,7 @@ extern void hfs_delete_inode(struct inode *inode); extern const struct xattr_handler * const hfs_xattr_handlers[]; /* mdb.c */ +extern bool is_hfs_cnid_counts_valid(struct super_block *sb); extern int hfs_mdb_get(struct super_block *sb); extern void hfs_mdb_commit(struct super_block *sb); extern void hfs_mdb_close(struct super_block *sb); diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c index 524db1389737..878535db64d6 100644 --- a/fs/hfs/inode.c +++ b/fs/hfs/inode.c @@ -187,16 +187,23 @@ struct inode *hfs_new_inode(struct inode *dir, const struct qstr *name, umode_t s64 next_id; s64 file_count; s64 folder_count; + int err = -ENOMEM; if (!inode) - return NULL; + goto out_err; + + err = -ERANGE; mutex_init(&HFS_I(inode)->extents_lock); INIT_LIST_HEAD(&HFS_I(inode)->open_dir_list); spin_lock_init(&HFS_I(inode)->open_dir_lock); hfs_cat_build_key(sb, (btree_key *)&HFS_I(inode)->cat_key, dir->i_ino, name); next_id = atomic64_inc_return(&HFS_SB(sb)->next_id); - BUG_ON(next_id > U32_MAX); + if (next_id > U32_MAX) { + atomic64_dec(&HFS_SB(sb)->next_id); + pr_err("cannot create new inode: next CNID exceeds limit\n"); + goto out_discard; + } inode->i_ino = (u32)next_id; inode->i_mode = mode; inode->i_uid = current_fsuid(); @@ -210,7 +217,11 @@ struct inode *hfs_new_inode(struct inode *dir, const struct qstr *name, umode_t if (S_ISDIR(mode)) { inode->i_size = 2; folder_count = atomic64_inc_return(&HFS_SB(sb)->folder_count); - BUG_ON(folder_count > U32_MAX); + if (folder_count> U32_MAX) { + atomic64_dec(&HFS_SB(sb)->folder_count); + pr_err("cannot create new inode: folder count exceeds limit\n"); + goto out_discard; + } if (dir->i_ino == HFS_ROOT_CNID) HFS_SB(sb)->root_dirs++; inode->i_op = &hfs_dir_inode_operations; @@ -220,7 +231,11 @@ struct inode *hfs_new_inode(struct inode *dir, const struct qstr *name, umode_t } else if (S_ISREG(mode)) { HFS_I(inode)->clump_blocks = HFS_SB(sb)->clumpablks; file_count = atomic64_inc_return(&HFS_SB(sb)->file_count); - BUG_ON(file_count > U32_MAX); + if (file_count > U32_MAX) { + atomic64_dec(&HFS_SB(sb)->file_count); + pr_err("cannot create new inode: file count exceeds limit\n"); + goto out_discard; + } if (dir->i_ino == HFS_ROOT_CNID) HFS_SB(sb)->root_files++; inode->i_op = &hfs_file_inode_operations; @@ -244,6 +259,11 @@ struct inode *hfs_new_inode(struct inode *dir, const struct qstr *name, umode_t hfs_mark_mdb_dirty(sb); return inode; + + out_discard: + iput(inode); + out_err: + return ERR_PTR(err); } void hfs_delete_inode(struct inode *inode) @@ -252,7 +272,6 @@ void hfs_delete_inode(struct inode *inode) hfs_dbg("ino %lu\n", inode->i_ino); if (S_ISDIR(inode->i_mode)) { - BUG_ON(atomic64_read(&HFS_SB(sb)->folder_count) > U32_MAX); atomic64_dec(&HFS_SB(sb)->folder_count); if (HFS_I(inode)->cat_key.ParID == cpu_to_be32(HFS_ROOT_CNID)) HFS_SB(sb)->root_dirs--; @@ -261,7 +280,6 @@ void hfs_delete_inode(struct inode *inode) return; } - BUG_ON(atomic64_read(&HFS_SB(sb)->file_count) > U32_MAX); atomic64_dec(&HFS_SB(sb)->file_count); if (HFS_I(inode)->cat_key.ParID == cpu_to_be32(HFS_ROOT_CNID)) HFS_SB(sb)->root_files--; diff --git a/fs/hfs/mdb.c b/fs/hfs/mdb.c index f28cd24dee84..a97cea35ca2e 100644 --- a/fs/hfs/mdb.c +++ b/fs/hfs/mdb.c @@ -64,6 +64,27 @@ static int hfs_get_last_session(struct super_block *sb, return 0; } +bool is_hfs_cnid_counts_valid(struct super_block *sb) +{ + struct hfs_sb_info *sbi = HFS_SB(sb); + bool corrupted = false; + + if (unlikely(atomic64_read(&sbi->next_id) > U32_MAX)) { + pr_warn("next CNID exceeds limit\n"); + corrupted = true; + } + if (unlikely(atomic64_read(&sbi->file_count) > U32_MAX)) { + pr_warn("file count exceeds limit\n"); + corrupted = true; + } + if (unlikely(atomic64_read(&sbi->folder_count) > U32_MAX)) { + pr_warn("folder count exceeds limit\n"); + corrupted = true; + } + + return !corrupted; +} + /* * hfs_mdb_get() * @@ -159,6 +180,11 @@ int hfs_mdb_get(struct super_block *sb) atomic64_set(&HFS_SB(sb)->file_count, be32_to_cpu(mdb->drFilCnt)); atomic64_set(&HFS_SB(sb)->folder_count, be32_to_cpu(mdb->drDirCnt)); + if (!is_hfs_cnid_counts_valid(sb)) { + pr_warn("filesystem possibly corrupted, running fsck.hfs is recommended. Mounting read-only.\n"); + sb->s_flags |= SB_RDONLY; + } + /* TRY to get the alternate (backup) MDB. */ sect = part_start + part_size - 2; bh = sb_bread512(sb, sect, mdb2); @@ -212,7 +238,7 @@ int hfs_mdb_get(struct super_block *sb) attrib = mdb->drAtrb; if (!(attrib & cpu_to_be16(HFS_SB_ATTRIB_UNMNT))) { - pr_warn("filesystem was not cleanly unmounted, running fsck.hfs is recommended. mounting read-only.\n"); + pr_warn("filesystem was not cleanly unmounted, running fsck.hfs is recommended. Mounting read-only.\n"); sb->s_flags |= SB_RDONLY; } if ((attrib & cpu_to_be16(HFS_SB_ATTRIB_SLOCK))) { @@ -270,15 +296,12 @@ void hfs_mdb_commit(struct super_block *sb) /* These parameters may have been modified, so write them back */ mdb->drLsMod = hfs_mtime(); mdb->drFreeBks = cpu_to_be16(HFS_SB(sb)->free_ablocks); - BUG_ON(atomic64_read(&HFS_SB(sb)->next_id) > U32_MAX); mdb->drNxtCNID = cpu_to_be32((u32)atomic64_read(&HFS_SB(sb)->next_id)); mdb->drNmFls = cpu_to_be16(HFS_SB(sb)->root_files); mdb->drNmRtDirs = cpu_to_be16(HFS_SB(sb)->root_dirs); - BUG_ON(atomic64_read(&HFS_SB(sb)->file_count) > U32_MAX); mdb->drFilCnt = cpu_to_be32((u32)atomic64_read(&HFS_SB(sb)->file_count)); - BUG_ON(atomic64_read(&HFS_SB(sb)->folder_count) > U32_MAX); mdb->drDirCnt = cpu_to_be32((u32)atomic64_read(&HFS_SB(sb)->folder_count)); diff --git a/fs/hfs/super.c b/fs/hfs/super.c index df289cbdd4e8..97546d6b41f4 100644 --- a/fs/hfs/super.c +++ b/fs/hfs/super.c @@ -34,6 +34,7 @@ MODULE_LICENSE("GPL"); static int hfs_sync_fs(struct super_block *sb, int wait) { + is_hfs_cnid_counts_valid(sb); hfs_mdb_commit(sb); return 0; } @@ -65,6 +66,8 @@ static void flush_mdb(struct work_struct *work) sbi->work_queued = 0; spin_unlock(&sbi->work_lock); + is_hfs_cnid_counts_valid(sb); + hfs_mdb_commit(sb); } From d8a73cc46c8462a969a7516131feb3096f4c49d3 Mon Sep 17 00:00:00 2001 From: Shardul Bankar Date: Tue, 30 Dec 2025 02:19:38 +0530 Subject: [PATCH 07/12] hfsplus: return error when node already exists in hfs_bnode_create When hfs_bnode_create() finds that a node is already hashed (which should not happen in normal operation), it currently returns the existing node without incrementing its reference count. This causes a reference count inconsistency that leads to a kernel panic when the node is later freed in hfs_bnode_put(): kernel BUG at fs/hfsplus/bnode.c:676! BUG_ON(!atomic_read(&node->refcnt)) This scenario can occur when hfs_bmap_alloc() attempts to allocate a node that is already in use (e.g., when node 0's bitmap bit is incorrectly unset), or due to filesystem corruption. Returning an existing node from a create path is not normal operation. Fix this by returning ERR_PTR(-EEXIST) instead of the node when it's already hashed. This properly signals the error condition to callers, which already check for IS_ERR() return values. Reported-by: syzbot+1c8ff72d0cd8a50dfeaa@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?extid=1c8ff72d0cd8a50dfeaa Link: https://lore.kernel.org/all/784415834694f39902088fa8946850fc1779a318.camel@ibm.com/ Fixes: 634725a92938 ("[PATCH] hfs: cleanup HFS+ prints") Signed-off-by: Shardul Bankar Reviewed-by: Viacheslav Dubeyko Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20251229204938.1907089-1-shardul.b@mpiricsoftware.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/bnode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c index 191661af9677..250a226336ea 100644 --- a/fs/hfsplus/bnode.c +++ b/fs/hfsplus/bnode.c @@ -629,7 +629,7 @@ struct hfs_bnode *hfs_bnode_create(struct hfs_btree *tree, u32 num) if (node) { pr_crit("new node %u already hashed?\n", num); WARN_ON(1); - return node; + return ERR_PTR(-EEXIST); } node = __hfs_bnode_create(tree, num); if (!node) From ed8889ca21b6ab37bc1435c4009ce37a79acb9e6 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Tue, 6 Jan 2026 18:39:33 +0900 Subject: [PATCH 08/12] hfsplus: pretend special inodes as regular files Since commit af153bb63a33 ("vfs: catch invalid modes in may_open()") requires any inode be one of S_IFDIR/S_IFLNK/S_IFREG/S_IFCHR/S_IFBLK/ S_IFIFO/S_IFSOCK type, use S_IFREG for special inodes. Reported-by: syzbot Closes: https://syzkaller.appspot.com/bug?extid=895c23f6917da440ed0d Signed-off-by: Tetsuo Handa Reviewed-by: Viacheslav Dubeyko Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/d0a07b1b-8b73-4002-8e29-e2bd56871262@I-love.SAKURA.ne.jp Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/super.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index 829c8ac2639c..942b8ff01ad0 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -53,6 +53,12 @@ static int hfsplus_system_read_inode(struct inode *inode) return -EIO; } + /* + * Assign a dummy file type, for may_open() requires that + * an inode has a valid file type. + */ + inode->i_mode = S_IFREG; + return 0; } From b18c5b84fa4a3c3c41b25c9b8f52ed9471c0c98d Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Fri, 9 Jan 2026 15:42:13 -0800 Subject: [PATCH 09/12] hfsplus: fix generic/037 xfstests failure The xfstests' test-case generic/037 fails to execute correctly: FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.15.0-rc4+ #8 SMP PREEMPT_DYNAMIC Thu May 1 16:43:22 PDT 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/037 - output mismatch (see xfstests-dev/results//generic/037.out.bad) The goal of generic/037 test-case is to "verify that replacing a xattr's value is an atomic operation". The test "consists of removing the old value and then inserting the new value in a btree. This made readers (getxattr and listxattrs) not getting neither the old nor the new value during a short time window". The HFS+ has the issue of executing the xattr replace operation because __hfsplus_setxattr() method [1] implemented it as not atomic operation [2]: if (hfsplus_attr_exists(inode, name)) { if (flags & XATTR_CREATE) { pr_err("xattr exists yet\n"); err = -EOPNOTSUPP; goto end_setxattr; } err = hfsplus_delete_attr(inode, name); if (err) goto end_setxattr; err = hfsplus_create_attr(inode, name, value, size); if (err) goto end_setxattr; } The main issue of the logic that it implements delete and create of xattr as independent atomic operations, but the replace operation at whole is not atomic operation. This patch implements a new hfsplus_replace_attr() method that makes the xattr replace operation by atomic one. Also, it reworks hfsplus_create_attr() and hfsplus_delete_attr() with the goal of reusing the common logic in hfsplus_replace_attr() method. sudo ./check generic/037 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.19.0-rc1+ #47 SMP PREEMPT_DYNAMIC Thu Jan 8 15:37:20 PST 2026 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/037 37s ... 37s Ran: generic/037 Passed all 1 tests [1] https://elixir.bootlin.com/linux/v6.19-rc4/source/fs/hfsplus/xattr.c#L261 [2] https://elixir.bootlin.com/linux/v6.19-rc4/source/fs/hfsplus/xattr.c#L338 Signed-off-by: Viacheslav Dubeyko cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Link: https://lore.kernel.org/r/20260109234213.2805400-1-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/attributes.c | 193 ++++++++++++++++++++++++++++------------ fs/hfsplus/hfsplus_fs.h | 3 + fs/hfsplus/xattr.c | 7 +- 3 files changed, 142 insertions(+), 61 deletions(-) diff --git a/fs/hfsplus/attributes.c b/fs/hfsplus/attributes.c index 74b0ca9c4f17..4b79cd606276 100644 --- a/fs/hfsplus/attributes.c +++ b/fs/hfsplus/attributes.c @@ -193,14 +193,66 @@ attr_not_found: return 0; } +static +int hfsplus_create_attr_nolock(struct inode *inode, const char *name, + const void *value, size_t size, + struct hfs_find_data *fd, + hfsplus_attr_entry *entry_ptr) +{ + struct super_block *sb = inode->i_sb; + int entry_size; + int err; + + hfs_dbg("name %s, ino %ld\n", + name ? name : NULL, inode->i_ino); + + if (name) { + err = hfsplus_attr_build_key(sb, fd->search_key, + inode->i_ino, name); + if (err) + return err; + } else + return -EINVAL; + + /* Mac OS X supports only inline data attributes. */ + entry_size = hfsplus_attr_build_record(entry_ptr, + HFSPLUS_ATTR_INLINE_DATA, + inode->i_ino, + value, size); + if (entry_size == HFSPLUS_INVALID_ATTR_RECORD) { + if (size > HFSPLUS_MAX_INLINE_DATA_SIZE) + err = -E2BIG; + else + err = -EINVAL; + hfs_dbg("unable to store value: err %d\n", err); + return err; + } + + err = hfs_brec_find(fd, hfs_find_rec_by_key); + if (err != -ENOENT) { + if (!err) + err = -EEXIST; + return err; + } + + err = hfs_brec_insert(fd, entry_ptr, entry_size); + if (err) { + hfs_dbg("unable to store value: err %d\n", err); + return err; + } + + hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ATTR_DIRTY); + + return 0; +} + int hfsplus_create_attr(struct inode *inode, - const char *name, - const void *value, size_t size) + const char *name, + const void *value, size_t size) { struct super_block *sb = inode->i_sb; struct hfs_find_data fd; hfsplus_attr_entry *entry_ptr; - int entry_size; int err; hfs_dbg("name %s, ino %ld\n", @@ -224,44 +276,10 @@ int hfsplus_create_attr(struct inode *inode, if (err) goto failed_create_attr; - if (name) { - err = hfsplus_attr_build_key(sb, fd.search_key, - inode->i_ino, name); - if (err) - goto failed_create_attr; - } else { - err = -EINVAL; + err = hfsplus_create_attr_nolock(inode, name, value, size, + &fd, entry_ptr); + if (err) goto failed_create_attr; - } - - /* Mac OS X supports only inline data attributes. */ - entry_size = hfsplus_attr_build_record(entry_ptr, - HFSPLUS_ATTR_INLINE_DATA, - inode->i_ino, - value, size); - if (entry_size == HFSPLUS_INVALID_ATTR_RECORD) { - if (size > HFSPLUS_MAX_INLINE_DATA_SIZE) - err = -E2BIG; - else - err = -EINVAL; - hfs_dbg("unable to store value: err %d\n", err); - goto failed_create_attr; - } - - err = hfs_brec_find(&fd, hfs_find_rec_by_key); - if (err != -ENOENT) { - if (!err) - err = -EEXIST; - goto failed_create_attr; - } - - err = hfs_brec_insert(&fd, entry_ptr, entry_size); - if (err) { - hfs_dbg("unable to store value: err %d\n", err); - goto failed_create_attr; - } - - hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ATTR_DIRTY); failed_create_attr: hfs_find_exit(&fd); @@ -312,6 +330,37 @@ static int __hfsplus_delete_attr(struct inode *inode, u32 cnid, return err; } +static +int hfsplus_delete_attr_nolock(struct inode *inode, const char *name, + struct hfs_find_data *fd) +{ + struct super_block *sb = inode->i_sb; + int err; + + hfs_dbg("name %s, ino %ld\n", + name ? name : NULL, inode->i_ino); + + if (name) { + err = hfsplus_attr_build_key(sb, fd->search_key, + inode->i_ino, name); + if (err) + return err; + } else { + pr_err("invalid extended attribute name\n"); + return -EINVAL; + } + + err = hfs_brec_find(fd, hfs_find_rec_by_key); + if (err) + return err; + + err = __hfsplus_delete_attr(inode, inode->i_ino, fd); + if (err) + return err; + + return 0; +} + int hfsplus_delete_attr(struct inode *inode, const char *name) { int err = 0; @@ -335,22 +384,7 @@ int hfsplus_delete_attr(struct inode *inode, const char *name) if (err) goto out; - if (name) { - err = hfsplus_attr_build_key(sb, fd.search_key, - inode->i_ino, name); - if (err) - goto out; - } else { - pr_err("invalid extended attribute name\n"); - err = -EINVAL; - goto out; - } - - err = hfs_brec_find(&fd, hfs_find_rec_by_key); - if (err) - goto out; - - err = __hfsplus_delete_attr(inode, inode->i_ino, &fd); + err = hfsplus_delete_attr_nolock(inode, name, &fd); if (err) goto out; @@ -392,3 +426,50 @@ end_delete_all: hfs_find_exit(&fd); return err; } + +int hfsplus_replace_attr(struct inode *inode, + const char *name, + const void *value, size_t size) +{ + struct super_block *sb = inode->i_sb; + struct hfs_find_data fd; + hfsplus_attr_entry *entry_ptr; + int err = 0; + + hfs_dbg("name %s, ino %ld\n", + name ? name : NULL, inode->i_ino); + + if (!HFSPLUS_SB(sb)->attr_tree) { + pr_err("attributes file doesn't exist\n"); + return -EINVAL; + } + + entry_ptr = hfsplus_alloc_attr_entry(); + if (!entry_ptr) + return -ENOMEM; + + err = hfs_find_init(HFSPLUS_SB(sb)->attr_tree, &fd); + if (err) + goto failed_init_replace_attr; + + /* Fail early and avoid ENOSPC during the btree operation */ + err = hfs_bmap_reserve(fd.tree, fd.tree->depth + 1); + if (err) + goto failed_replace_attr; + + err = hfsplus_delete_attr_nolock(inode, name, &fd); + if (err) + goto failed_replace_attr; + + err = hfsplus_create_attr_nolock(inode, name, value, size, + &fd, entry_ptr); + if (err) + goto failed_replace_attr; + +failed_replace_attr: + hfs_find_exit(&fd); + +failed_init_replace_attr: + hfsplus_destroy_attr_entry(entry_ptr); + return err; +} diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h index 45fe3a12ecba..5f891b73a646 100644 --- a/fs/hfsplus/hfsplus_fs.h +++ b/fs/hfsplus/hfsplus_fs.h @@ -344,6 +344,9 @@ int hfsplus_create_attr(struct inode *inode, const char *name, const void *value, size_t size); int hfsplus_delete_attr(struct inode *inode, const char *name); int hfsplus_delete_all_attrs(struct inode *dir, u32 cnid); +int hfsplus_replace_attr(struct inode *inode, + const char *name, + const void *value, size_t size); /* bitmap.c */ int hfsplus_block_allocate(struct super_block *sb, u32 size, u32 offset, diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c index 504518a41212..c3dcbe30f16a 100644 --- a/fs/hfsplus/xattr.c +++ b/fs/hfsplus/xattr.c @@ -341,12 +341,9 @@ int __hfsplus_setxattr(struct inode *inode, const char *name, err = -EOPNOTSUPP; goto end_setxattr; } - err = hfsplus_delete_attr(inode, name); - if (err) - goto end_setxattr; - err = hfsplus_create_attr(inode, name, value, size); + err = hfsplus_replace_attr(inode, name, value, size); if (err) { - hfs_dbg("unable to store value: err %d\n", err); + hfs_dbg("unable to replace xattr: err %d\n", err); goto end_setxattr; } } else { From aef5078471294e7a7a2d6b9da4988a6a3b7aff54 Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Mon, 19 Jan 2026 20:19:38 -0800 Subject: [PATCH 10/12] hfsplus: fix generic/062 xfstests failure The xfstests' test-case generic/062 fails to execute correctly: FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.15.0-rc4+ #8 SMP PREEMPT_DYNAMIC Thu May 1 16:43:22 PDT 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/062 - output mismatch (see xfstests-dev/results//generic/062.out.bad) The generic/062 test tries to set and get xattrs for various types of objects (regular file, folder, block device, character device, pipe, etc) with the goal to check that xattr operations works correctly for all possible types of file system objects. But current HFS+ implementation somehow hasn't support of xattr operatioons for the case of block device, character device, and pipe objects. Also, it has not completely correct set of operations for the case symlinks. This patch implements proper declaration of xattrs operations hfsplus_special_inode_operations and hfsplus_symlink_inode_operations. Also, it slightly corrects the logic of hfsplus_listxattr() method. sudo ./check generic/062 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.19.0-rc1+ #59 SMP PREEMPT_DYNAMIC Mon Jan 19 16:26:21 PST 2026 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/062 20s ... 20s Ran: generic/062 Passed all 1 tests [1] https://github.com/hfs-linux-kernel/hfs-linux-kernel/issues/93 Signed-off-by: Viacheslav Dubeyko cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Link: https://lore.kernel.org/r/20260120041937.3450928-1-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/inode.c | 23 +++++++++++++++++++++-- fs/hfsplus/xattr.c | 29 ++++++++++++++++++----------- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c index 6153e5cc6eb6..533c43cc3768 100644 --- a/fs/hfsplus/inode.c +++ b/fs/hfsplus/inode.c @@ -396,6 +396,19 @@ static const struct inode_operations hfsplus_file_inode_operations = { .fileattr_set = hfsplus_fileattr_set, }; +const struct inode_operations hfsplus_symlink_inode_operations = { + .get_link = page_get_link, + .setattr = hfsplus_setattr, + .getattr = hfsplus_getattr, + .listxattr = hfsplus_listxattr, +}; + +const struct inode_operations hfsplus_special_inode_operations = { + .setattr = hfsplus_setattr, + .getattr = hfsplus_getattr, + .listxattr = hfsplus_listxattr, +}; + static const struct file_operations hfsplus_file_operations = { .llseek = generic_file_llseek, .read_iter = generic_file_read_iter, @@ -455,12 +468,17 @@ struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir, hip->clump_blocks = sbi->data_clump_blocks; } else if (S_ISLNK(inode->i_mode)) { sbi->file_count++; - inode->i_op = &page_symlink_inode_operations; + inode->i_op = &hfsplus_symlink_inode_operations; inode_nohighmem(inode); inode->i_mapping->a_ops = &hfsplus_aops; hip->clump_blocks = 1; + } else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode) || + S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) { + sbi->file_count++; + inode->i_op = &hfsplus_special_inode_operations; } else sbi->file_count++; + insert_inode_hash(inode); mark_inode_dirty(inode); hfsplus_mark_mdb_dirty(sb); @@ -591,10 +609,11 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd) inode->i_fop = &hfsplus_file_operations; inode->i_mapping->a_ops = &hfsplus_aops; } else if (S_ISLNK(inode->i_mode)) { - inode->i_op = &page_symlink_inode_operations; + inode->i_op = &hfsplus_symlink_inode_operations; inode_nohighmem(inode); inode->i_mapping->a_ops = &hfsplus_aops; } else { + inode->i_op = &hfsplus_special_inode_operations; init_special_inode(inode, inode->i_mode, be32_to_cpu(file->permissions.dev)); } diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c index c3dcbe30f16a..9904944cbd54 100644 --- a/fs/hfsplus/xattr.c +++ b/fs/hfsplus/xattr.c @@ -258,6 +258,15 @@ end_attr_file_creation: return err; } +static inline +bool is_xattr_operation_supported(struct inode *inode) +{ + if (HFSPLUS_IS_RSRC(inode)) + return false; + + return true; +} + int __hfsplus_setxattr(struct inode *inode, const char *name, const void *value, size_t size, int flags) { @@ -268,9 +277,11 @@ int __hfsplus_setxattr(struct inode *inode, const char *name, u16 folder_finderinfo_len = sizeof(DInfo) + sizeof(DXInfo); u16 file_finderinfo_len = sizeof(FInfo) + sizeof(FXInfo); - if ((!S_ISREG(inode->i_mode) && - !S_ISDIR(inode->i_mode)) || - HFSPLUS_IS_RSRC(inode)) + hfs_dbg("ino %lu, name %s, value %p, size %zu\n", + inode->i_ino, name ? name : NULL, + value, size); + + if (!is_xattr_operation_supported(inode)) return -EOPNOTSUPP; if (value == NULL) @@ -390,6 +401,7 @@ int __hfsplus_setxattr(struct inode *inode, const char *name, end_setxattr: hfs_find_exit(&cat_fd); + hfs_dbg("finished: res %d\n", err); return err; } @@ -514,9 +526,7 @@ ssize_t __hfsplus_getxattr(struct inode *inode, const char *name, u16 record_length = 0; ssize_t res; - if ((!S_ISREG(inode->i_mode) && - !S_ISDIR(inode->i_mode)) || - HFSPLUS_IS_RSRC(inode)) + if (!is_xattr_operation_supported(inode)) return -EOPNOTSUPP; if (!strcmp_xattr_finder_info(name)) @@ -709,9 +719,7 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size) hfs_dbg("ino %lu\n", inode->i_ino); - if ((!S_ISREG(inode->i_mode) && - !S_ISDIR(inode->i_mode)) || - HFSPLUS_IS_RSRC(inode)) + if (!is_xattr_operation_supported(inode)) return -EOPNOTSUPP; res = hfsplus_listxattr_finder_info(dentry, buffer, size); @@ -737,8 +745,7 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size) err = hfsplus_find_attr(inode->i_sb, inode->i_ino, NULL, &fd); if (err) { if (err == -ENOENT) { - if (res == 0) - res = -ENODATA; + res = 0; goto end_listxattr; } else { res = err; From 14b428cfba2d610c20cf0a5e00db10e95f4cbf88 Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Thu, 29 Jan 2026 11:54:43 -0800 Subject: [PATCH 11/12] hfsplus: fix warning issue in inode.c This patch fixes the sparse warning issue in inode.c by adding static to hfsplus_symlink_inode_operations and hfsplus_special_inode_operations declarations. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202601291957.bunRsD8R-lkp@intel.com/ Signed-off-by: Viacheslav Dubeyko cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Link: https://lore.kernel.org/r/20260129195442.594884-1-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/inode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c index 533c43cc3768..922ff41df042 100644 --- a/fs/hfsplus/inode.c +++ b/fs/hfsplus/inode.c @@ -396,14 +396,14 @@ static const struct inode_operations hfsplus_file_inode_operations = { .fileattr_set = hfsplus_fileattr_set, }; -const struct inode_operations hfsplus_symlink_inode_operations = { +static const struct inode_operations hfsplus_symlink_inode_operations = { .get_link = page_get_link, .setattr = hfsplus_setattr, .getattr = hfsplus_getattr, .listxattr = hfsplus_listxattr, }; -const struct inode_operations hfsplus_special_inode_operations = { +static const struct inode_operations hfsplus_special_inode_operations = { .setattr = hfsplus_setattr, .getattr = hfsplus_getattr, .listxattr = hfsplus_listxattr, From ebebb04baefdace1e0dc17f7779e5549063ca592 Mon Sep 17 00:00:00 2001 From: Shardul Bankar Date: Wed, 4 Feb 2026 22:34:40 +0530 Subject: [PATCH 12/12] hfsplus: avoid double unload_nls() on mount failure The recent commit "hfsplus: ensure sb->s_fs_info is always cleaned up" [1] introduced a custom ->kill_sb() handler (hfsplus_kill_super) that cleans up the s_fs_info structure (including the NLS table) on superblock destruction. However, the error handling path in hfsplus_fill_super() still calls unload_nls() before returning an error. Since the VFS layer calls ->kill_sb() when fill_super fails, this results in unload_nls() being called twice for the same sbi->nls pointer: once in hfsplus_fill_super() and again in hfsplus_kill_super() (via delayed_free). Remove the explicit unload_nls() call from the error path in hfsplus_fill_super() to rely solely on the cleanup in ->kill_sb(). [1] https://lore.kernel.org/r/20251201222843.82310-3-mehdi.benhadjkhelifa@gmail.com/ Reported-by: Al Viro Link: https://lore.kernel.org/r/20260203043806.GF3183987@ZenIV/ Signed-off-by: Shardul Bankar Link: https://lore.kernel.org/r/20260204170440.1337261-1-shardul.b@mpiricsoftware.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/super.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index 942b8ff01ad0..592d8fbb748c 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -652,7 +652,6 @@ out_free_vhdr: kfree(sbi->s_vhdr_buf); kfree(sbi->s_backup_vhdr_buf); out_unload_nls: - unload_nls(sbi->nls); unload_nls(nls); return err; }