From ee8422d00b7cfa028823ebf1f28bf9dea428cac3 Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Fri, 20 Feb 2026 14:01:53 -0800 Subject: [PATCH 01/15] hfsplus: fix potential Allocation File corruption after fsync The generic/348 test-case has revealed the issue of HFS+ volume corruption after simulated power failure: 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/348 _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent (see xfstests-dev/results//generic/348.full for details) The fsck tool complains about Allocation File (block bitmap) corruption as a result of such event. The generic/348 creates a symlink, fsync its parent directory, power fail and mount again the filesystem. Currently, HFS+ logic has several flags HFSPLUS_I_CAT_DIRTY, HFSPLUS_I_EXT_DIRTY, HFSPLUS_I_ATTR_DIRTY, HFSPLUS_I_ALLOC_DIRTY. If inode operation modified the Catalog File, Extents Overflow File, Attributes File, or Allocation File, then inode is marked as dirty and one of the mentioned flags has been set. When hfsplus_file_fsync() has been called, then this set of flags is checked and dirty b-tree or/and block bitmap is flushed. However, block bitmap can be modified during file's content allocation. It means that if we call hfsplus_file_fsync() for directory, then we never flush the modified Allocation File in such case because such inode cannot receive HFSPLUS_I_ALLOC_DIRTY flag. Moreover, this inode-centric model is not good at all because Catalog File, Extents Overflow File, Attributes File, and Allocation File represent the whole state of file system metadata. This inode-centric policy is the main reason of the issue. This patch saves the whole approach of using HFSPLUS_I_CAT_DIRTY, HFSPLUS_I_EXT_DIRTY, HFSPLUS_I_ATTR_DIRTY, and HFSPLUS_I_ALLOC_DIRTY flags. But Catalog File, Extents Overflow File, Attributes File, and Allocation File have associated inodes. And namely these inodes become the mechanism of checking the dirty state of metadata. The hfsplus_file_fsync() method checks the dirtiness of file system metadata by testing HFSPLUS_I_CAT_DIRTY, HFSPLUS_I_EXT_DIRTY, HFSPLUS_I_ATTR_DIRTY, and HFSPLUS_I_ALLOC_DIRTY flags of Catalog File's, Extents Overflow File's, Attributes File's, or Allocation File's inodes. As a result, even if we call hfsplus_file_fsync() for parent folder, then dirty Allocation File will be flushed anyway. 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/20260220220152.152721-1-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/attributes.c | 3 +++ fs/hfsplus/catalog.c | 3 +++ fs/hfsplus/dir.c | 6 ++++++ fs/hfsplus/extents.c | 7 +++++++ fs/hfsplus/hfsplus_fs.h | 7 +++++++ fs/hfsplus/inode.c | 27 ++++++++++++++++++++------- fs/hfsplus/super.c | 2 ++ fs/hfsplus/xattr.c | 19 +++++++++++++++++-- 8 files changed, 65 insertions(+), 9 deletions(-) diff --git a/fs/hfsplus/attributes.c b/fs/hfsplus/attributes.c index 4b79cd606276..6585bcea731c 100644 --- a/fs/hfsplus/attributes.c +++ b/fs/hfsplus/attributes.c @@ -241,6 +241,7 @@ int hfsplus_create_attr_nolock(struct inode *inode, const char *name, return err; } + hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(sb), HFSPLUS_I_ATTR_DIRTY); hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ATTR_DIRTY); return 0; @@ -326,6 +327,8 @@ static int __hfsplus_delete_attr(struct inode *inode, u32 cnid, if (err) return err; + hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(inode->i_sb), + HFSPLUS_I_ATTR_DIRTY); hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ATTR_DIRTY); return err; } diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c index 02c1eee4a4b8..eef7412a4d58 100644 --- a/fs/hfsplus/catalog.c +++ b/fs/hfsplus/catalog.c @@ -313,6 +313,7 @@ int hfsplus_create_cat(u32 cnid, struct inode *dir, if (S_ISDIR(inode->i_mode)) hfsplus_subfolders_inc(dir); inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir)); + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(sb), HFSPLUS_I_CAT_DIRTY); hfsplus_mark_inode_dirty(dir, HFSPLUS_I_CAT_DIRTY); hfs_find_exit(&fd); @@ -418,6 +419,7 @@ int hfsplus_delete_cat(u32 cnid, struct inode *dir, const struct qstr *str) if (type == HFSPLUS_FOLDER) hfsplus_subfolders_dec(dir); inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir)); + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(sb), HFSPLUS_I_CAT_DIRTY); hfsplus_mark_inode_dirty(dir, HFSPLUS_I_CAT_DIRTY); if (type == HFSPLUS_FILE || type == HFSPLUS_FOLDER) { @@ -540,6 +542,7 @@ int hfsplus_rename_cat(u32 cnid, } err = hfs_brec_insert(&dst_fd, &entry, entry_size); + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(sb), HFSPLUS_I_CAT_DIRTY); hfsplus_mark_inode_dirty(dst_dir, HFSPLUS_I_CAT_DIRTY); hfsplus_mark_inode_dirty(src_dir, HFSPLUS_I_CAT_DIRTY); out: diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c index d559bf8625f8..9f0345bf1b5a 100644 --- a/fs/hfsplus/dir.c +++ b/fs/hfsplus/dir.c @@ -478,6 +478,9 @@ static int hfsplus_symlink(struct mnt_idmap *idmap, struct inode *dir, if (!inode) goto out; + hfs_dbg("dir->i_ino %lu, inode->i_ino %lu\n", + dir->i_ino, inode->i_ino); + res = page_symlink(inode, symname, strlen(symname) + 1); if (res) goto out_err; @@ -526,6 +529,9 @@ static int hfsplus_mknod(struct mnt_idmap *idmap, struct inode *dir, if (!inode) goto out; + hfs_dbg("dir->i_ino %lu, inode->i_ino %lu\n", + dir->i_ino, inode->i_ino); + if (S_ISBLK(mode) || S_ISCHR(mode) || S_ISFIFO(mode) || S_ISSOCK(mode)) init_special_inode(inode, mode, rdev); diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c index 8e886514d27f..a5f772de9985 100644 --- a/fs/hfsplus/extents.c +++ b/fs/hfsplus/extents.c @@ -121,6 +121,8 @@ static int __hfsplus_ext_write_extent(struct inode *inode, * redirty the inode. Instead the callers have to be careful * to explicily mark the inode dirty, too. */ + set_bit(HFSPLUS_I_EXT_DIRTY, + &HFSPLUS_I(HFSPLUS_EXT_TREE_I(inode->i_sb))->flags); set_bit(HFSPLUS_I_EXT_DIRTY, &hip->flags); return 0; @@ -513,6 +515,8 @@ int hfsplus_file_extend(struct inode *inode, bool zeroout) if (!res) { hip->alloc_blocks += len; mutex_unlock(&hip->extents_lock); + hfsplus_mark_inode_dirty(HFSPLUS_SB(sb)->alloc_file, + HFSPLUS_I_ALLOC_DIRTY); hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ALLOC_DIRTY); return 0; } @@ -582,6 +586,7 @@ void hfsplus_file_truncate(struct inode *inode) /* XXX: We lack error handling of hfsplus_file_truncate() */ return; } + while (1) { if (alloc_cnt == hip->first_blocks) { mutex_unlock(&fd.tree->tree_lock); @@ -623,5 +628,7 @@ void hfsplus_file_truncate(struct inode *inode) hip->fs_blocks = (inode->i_size + sb->s_blocksize - 1) >> sb->s_blocksize_bits; inode_set_bytes(inode, hip->fs_blocks << sb->s_blocksize_bits); + hfsplus_mark_inode_dirty(HFSPLUS_SB(sb)->alloc_file, + HFSPLUS_I_ALLOC_DIRTY); hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ALLOC_DIRTY); } diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h index 5f891b73a646..122ab57193bb 100644 --- a/fs/hfsplus/hfsplus_fs.h +++ b/fs/hfsplus/hfsplus_fs.h @@ -238,6 +238,13 @@ static inline struct hfsplus_inode_info *HFSPLUS_I(struct inode *inode) return container_of(inode, struct hfsplus_inode_info, vfs_inode); } +#define HFSPLUS_CAT_TREE_I(sb) \ + HFSPLUS_SB(sb)->cat_tree->inode +#define HFSPLUS_EXT_TREE_I(sb) \ + HFSPLUS_SB(sb)->ext_tree->inode +#define HFSPLUS_ATTR_TREE_I(sb) \ + HFSPLUS_SB(sb)->attr_tree->inode + /* * Mark an inode dirty, and also mark the btree in which the * specific type of metadata is stored. diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c index 922ff41df042..cdf08393de44 100644 --- a/fs/hfsplus/inode.c +++ b/fs/hfsplus/inode.c @@ -324,6 +324,7 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end, { struct inode *inode = file->f_mapping->host; struct hfsplus_inode_info *hip = HFSPLUS_I(inode); + struct super_block *sb = inode->i_sb; struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb); struct hfsplus_vh *vhdr = sbi->s_vhdr; int error = 0, error2; @@ -344,29 +345,39 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end, /* * And explicitly write out the btrees. */ - if (test_and_clear_bit(HFSPLUS_I_CAT_DIRTY, &hip->flags)) + if (test_and_clear_bit(HFSPLUS_I_CAT_DIRTY, + &HFSPLUS_I(HFSPLUS_CAT_TREE_I(sb))->flags)) { + clear_bit(HFSPLUS_I_CAT_DIRTY, &hip->flags); error = filemap_write_and_wait(sbi->cat_tree->inode->i_mapping); + } - if (test_and_clear_bit(HFSPLUS_I_EXT_DIRTY, &hip->flags)) { + if (test_and_clear_bit(HFSPLUS_I_EXT_DIRTY, + &HFSPLUS_I(HFSPLUS_EXT_TREE_I(sb))->flags)) { + clear_bit(HFSPLUS_I_EXT_DIRTY, &hip->flags); error2 = filemap_write_and_wait(sbi->ext_tree->inode->i_mapping); if (!error) error = error2; } - if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags)) { - if (sbi->attr_tree) { + if (sbi->attr_tree) { + if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY, + &HFSPLUS_I(HFSPLUS_ATTR_TREE_I(sb))->flags)) { + clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags); error2 = filemap_write_and_wait( sbi->attr_tree->inode->i_mapping); if (!error) error = error2; - } else { - pr_err("sync non-existent attributes tree\n"); } + } else { + if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags)) + pr_err("sync non-existent attributes tree\n"); } - if (test_and_clear_bit(HFSPLUS_I_ALLOC_DIRTY, &hip->flags)) { + if (test_and_clear_bit(HFSPLUS_I_ALLOC_DIRTY, + &HFSPLUS_I(sbi->alloc_file)->flags)) { + clear_bit(HFSPLUS_I_ALLOC_DIRTY, &hip->flags); error2 = filemap_write_and_wait(sbi->alloc_file->i_mapping); if (!error) error = error2; @@ -709,6 +720,8 @@ int hfsplus_cat_write_inode(struct inode *inode) sizeof(struct hfsplus_cat_file)); } + set_bit(HFSPLUS_I_CAT_DIRTY, + &HFSPLUS_I(HFSPLUS_CAT_TREE_I(inode->i_sb))->flags); set_bit(HFSPLUS_I_CAT_DIRTY, &HFSPLUS_I(inode)->flags); out: hfs_find_exit(&fd); diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index 7229a8ae89f9..2a135c78d0b2 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -625,6 +625,8 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc) } mutex_unlock(&sbi->vh_mutex); + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(sb), + HFSPLUS_I_CAT_DIRTY); hfsplus_mark_inode_dirty(sbi->hidden_dir, HFSPLUS_I_CAT_DIRTY); } diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c index 9904944cbd54..31b6cb9db770 100644 --- a/fs/hfsplus/xattr.c +++ b/fs/hfsplus/xattr.c @@ -236,6 +236,7 @@ static int hfsplus_create_attributes_file(struct super_block *sb) put_page(page); } + hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(sb), HFSPLUS_I_ATTR_DIRTY); hfsplus_mark_inode_dirty(attr_file, HFSPLUS_I_ATTR_DIRTY); sbi->attr_tree = hfs_btree_open(sb, HFSPLUS_ATTR_CNID); @@ -314,8 +315,11 @@ int __hfsplus_setxattr(struct inode *inode, const char *name, hfs_bnode_write(cat_fd.bnode, &entry, cat_fd.entryoffset, sizeof(struct hfsplus_cat_folder)); - hfsplus_mark_inode_dirty(inode, + hfsplus_mark_inode_dirty( + HFSPLUS_CAT_TREE_I(inode->i_sb), HFSPLUS_I_CAT_DIRTY); + hfsplus_mark_inode_dirty(inode, + HFSPLUS_I_CAT_DIRTY); } else { err = -ERANGE; goto end_setxattr; @@ -327,8 +331,11 @@ int __hfsplus_setxattr(struct inode *inode, const char *name, hfs_bnode_write(cat_fd.bnode, &entry, cat_fd.entryoffset, sizeof(struct hfsplus_cat_file)); - hfsplus_mark_inode_dirty(inode, + hfsplus_mark_inode_dirty( + HFSPLUS_CAT_TREE_I(inode->i_sb), HFSPLUS_I_CAT_DIRTY); + hfsplus_mark_inode_dirty(inode, + HFSPLUS_I_CAT_DIRTY); } else { err = -ERANGE; goto end_setxattr; @@ -381,6 +388,8 @@ int __hfsplus_setxattr(struct inode *inode, const char *name, hfs_bnode_write_u16(cat_fd.bnode, cat_fd.entryoffset + offsetof(struct hfsplus_cat_folder, flags), cat_entry_flags); + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(inode->i_sb), + HFSPLUS_I_CAT_DIRTY); hfsplus_mark_inode_dirty(inode, HFSPLUS_I_CAT_DIRTY); } else if (cat_entry_type == HFSPLUS_FILE) { cat_entry_flags = hfs_bnode_read_u16(cat_fd.bnode, @@ -392,6 +401,8 @@ int __hfsplus_setxattr(struct inode *inode, const char *name, hfs_bnode_write_u16(cat_fd.bnode, cat_fd.entryoffset + offsetof(struct hfsplus_cat_file, flags), cat_entry_flags); + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(inode->i_sb), + HFSPLUS_I_CAT_DIRTY); hfsplus_mark_inode_dirty(inode, HFSPLUS_I_CAT_DIRTY); } else { pr_err("invalid catalog entry type\n"); @@ -862,6 +873,8 @@ static int hfsplus_removexattr(struct inode *inode, const char *name) hfs_bnode_write_u16(cat_fd.bnode, cat_fd.entryoffset + offsetof(struct hfsplus_cat_folder, flags), flags); + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(inode->i_sb), + HFSPLUS_I_CAT_DIRTY); hfsplus_mark_inode_dirty(inode, HFSPLUS_I_CAT_DIRTY); } else if (cat_entry_type == HFSPLUS_FILE) { flags = hfs_bnode_read_u16(cat_fd.bnode, cat_fd.entryoffset + @@ -873,6 +886,8 @@ static int hfsplus_removexattr(struct inode *inode, const char *name) hfs_bnode_write_u16(cat_fd.bnode, cat_fd.entryoffset + offsetof(struct hfsplus_cat_file, flags), flags); + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(inode->i_sb), + HFSPLUS_I_CAT_DIRTY); hfsplus_mark_inode_dirty(inode, HFSPLUS_I_CAT_DIRTY); } else { pr_err("invalid catalog entry type\n"); From b6b592275aeff184aa82fcf6abccd833fb71b393 Mon Sep 17 00:00:00 2001 From: Deepanshu Kartikey Date: Sat, 7 Mar 2026 06:33:02 +0530 Subject: [PATCH 02/15] hfsplus: fix uninit-value by validating catalog record size Syzbot reported a KMSAN uninit-value issue in hfsplus_strcasecmp(). The root cause is that hfs_brec_read() doesn't validate that the on-disk record size matches the expected size for the record type being read. When mounting a corrupted filesystem, hfs_brec_read() may read less data than expected. For example, when reading a catalog thread record, the debug output showed: HFSPLUS_BREC_READ: rec_len=520, fd->entrylength=26 HFSPLUS_BREC_READ: WARNING - entrylength (26) < rec_len (520) - PARTIAL READ! hfs_brec_read() only validates that entrylength is not greater than the buffer size, but doesn't check if it's less than expected. It successfully reads 26 bytes into a 520-byte structure and returns success, leaving 494 bytes uninitialized. This uninitialized data in tmp.thread.nodeName then gets copied by hfsplus_cat_build_key_uni() and used by hfsplus_strcasecmp(), triggering the KMSAN warning when the uninitialized bytes are used as array indices in case_fold(). Fix by introducing hfsplus_brec_read_cat() wrapper that: 1. Calls hfs_brec_read() to read the data 2. Validates the record size based on the type field: - Fixed size for folder and file records - Variable size for thread records (depends on string length) 3. Returns -EIO if size doesn't match expected For thread records, check against HFSPLUS_MIN_THREAD_SZ before reading nodeName.length to avoid reading uninitialized data at call sites that don't zero-initialize the entry structure. Also initialize the tmp variable in hfsplus_find_cat() as defensive programming to ensure no uninitialized data even if validation is bypassed. Reported-by: syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=d80abb5b890d39261e72 Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Tested-by: syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com Reviewed-by: Viacheslav Dubeyko Tested-by: Viacheslav Dubeyko Suggested-by: Charalampos Mitrodimas Link: https://lore.kernel.org/all/20260120051114.1281285-1-kartikey406@gmail.com/ [v1] Link: https://lore.kernel.org/all/20260121063109.1830263-1-kartikey406@gmail.com/ [v2] Link: https://lore.kernel.org/all/20260212014233.2422046-1-kartikey406@gmail.com/ [v3] Link: https://lore.kernel.org/all/20260214002100.436125-1-kartikey406@gmail.com/T/ [v4] Link: https://lore.kernel.org/all/20260221061626.15853-1-kartikey406@gmail.com/T/ [v5] Signed-off-by: Deepanshu Kartikey Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20260307010302.41547-1-kartikey406@gmail.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/bfind.c | 51 +++++++++++++++++++++++++++++++++++++++++ fs/hfsplus/catalog.c | 4 ++-- fs/hfsplus/dir.c | 2 +- fs/hfsplus/hfsplus_fs.h | 9 ++++++++ fs/hfsplus/super.c | 2 +- 5 files changed, 64 insertions(+), 4 deletions(-) diff --git a/fs/hfsplus/bfind.c b/fs/hfsplus/bfind.c index 336d654861c5..9a55fa6d5294 100644 --- a/fs/hfsplus/bfind.c +++ b/fs/hfsplus/bfind.c @@ -287,3 +287,54 @@ int hfs_brec_goto(struct hfs_find_data *fd, int cnt) fd->bnode = bnode; return res; } + +/** + * hfsplus_brec_read_cat - read and validate a catalog record + * @fd: find data structure + * @entry: pointer to catalog entry to read into + * + * Reads a catalog record and validates its size matches the expected + * size based on the record type. + * + * Returns 0 on success, or negative error code on failure. + */ +int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry) +{ + int res; + u32 expected_size; + + res = hfs_brec_read(fd, entry, sizeof(hfsplus_cat_entry)); + if (res) + return res; + + /* Validate catalog record size based on type */ + switch (be16_to_cpu(entry->type)) { + case HFSPLUS_FOLDER: + expected_size = sizeof(struct hfsplus_cat_folder); + break; + case HFSPLUS_FILE: + expected_size = sizeof(struct hfsplus_cat_file); + break; + case HFSPLUS_FOLDER_THREAD: + case HFSPLUS_FILE_THREAD: + /* Ensure we have at least the fixed fields before reading nodeName.length */ + if (fd->entrylength < HFSPLUS_MIN_THREAD_SZ) { + pr_err("thread record too short (got %u)\n", fd->entrylength); + return -EIO; + } + expected_size = hfsplus_cat_thread_size(&entry->thread); + break; + default: + pr_err("unknown catalog record type %d\n", + be16_to_cpu(entry->type)); + return -EIO; + } + + if (fd->entrylength != expected_size) { + pr_err("catalog record size mismatch (type %d, got %u, expected %u)\n", + be16_to_cpu(entry->type), fd->entrylength, expected_size); + return -EIO; + } + + return 0; +} diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c index eef7412a4d58..708046c5dae6 100644 --- a/fs/hfsplus/catalog.c +++ b/fs/hfsplus/catalog.c @@ -194,12 +194,12 @@ static int hfsplus_fill_cat_thread(struct super_block *sb, int hfsplus_find_cat(struct super_block *sb, u32 cnid, struct hfs_find_data *fd) { - hfsplus_cat_entry tmp; + hfsplus_cat_entry tmp = {0}; int err; u16 type; hfsplus_cat_build_key_with_cnid(sb, fd->search_key, cnid); - err = hfs_brec_read(fd, &tmp, sizeof(hfsplus_cat_entry)); + err = hfsplus_brec_read_cat(fd, &tmp); if (err) return err; diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c index 9f0345bf1b5a..6059bd59d66e 100644 --- a/fs/hfsplus/dir.c +++ b/fs/hfsplus/dir.c @@ -49,7 +49,7 @@ static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry, if (unlikely(err < 0)) goto fail; again: - err = hfs_brec_read(&fd, &entry, sizeof(entry)); + err = hfsplus_brec_read_cat(&fd, &entry); if (err) { if (err == -ENOENT) { hfs_find_exit(&fd); diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h index 122ab57193bb..420dc920e097 100644 --- a/fs/hfsplus/hfsplus_fs.h +++ b/fs/hfsplus/hfsplus_fs.h @@ -516,6 +516,15 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf, void **data, blk_opf_t opf); int hfsplus_read_wrapper(struct super_block *sb); +static inline u32 hfsplus_cat_thread_size(const struct hfsplus_cat_thread *thread) +{ + return offsetof(struct hfsplus_cat_thread, nodeName) + + offsetof(struct hfsplus_unistr, unicode) + + be16_to_cpu(thread->nodeName.length) * sizeof(hfsplus_unichr); +} + +int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry); + /* * time helpers: convert between 1904-base and 1970-base timestamps * diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index 2a135c78d0b2..242ccca3acb7 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -571,7 +571,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc) err = hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str); if (unlikely(err < 0)) goto out_put_root; - if (!hfs_brec_read(&fd, &entry, sizeof(entry))) { + if (!hfsplus_brec_read_cat(&fd, &entry)) { hfs_find_exit(&fd); if (entry.type != cpu_to_be16(HFSPLUS_FOLDER)) { err = -EIO; From e89b5724aaf362cc84ecacaf56eb09a88e57441e Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Mon, 9 Mar 2026 14:49:48 -0700 Subject: [PATCH 03/15] hfsplus: set ctime after setxattr and removexattr The generic/728 test-case complains that: (1) Expected ctime needs to be changed after setxattr; (2) Expected ctime needs to be changed after removexattr. This patch adds calling inode_set_ctime_current() in __hfsplus_setxattr() and hfsplus_removexattr(). sudo ./check generic/728 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 7.0.0-rc1+ #6 SMP PREEMPT_DYNAMIC Mon Mar 9 14:29:30 PDT 2026 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/728 35s ... 44s Ran: generic/728 Passed all 1 tests cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20260309214947.1114618-2-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/xattr.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c index 31b6cb9db770..a824bcaac172 100644 --- a/fs/hfsplus/xattr.c +++ b/fs/hfsplus/xattr.c @@ -410,6 +410,8 @@ int __hfsplus_setxattr(struct inode *inode, const char *name, goto end_setxattr; } + inode_set_ctime_current(inode); + end_setxattr: hfs_find_exit(&cat_fd); hfs_dbg("finished: res %d\n", err); @@ -895,6 +897,8 @@ static int hfsplus_removexattr(struct inode *inode, const char *name) goto end_removexattr; } + inode_set_ctime_current(inode); + end_removexattr: hfs_find_exit(&cat_fd); From a46aaa76ad21de033f188595173e8ae7afefddc0 Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Thu, 12 Mar 2026 15:19:21 -0700 Subject: [PATCH 04/15] hfsplus: fix generic/533 test-case failure The xfstests' test-case generic/533 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/533 _check_generic_filesystem: filesystem on /dev/loop50 is inconsistent (see xfstests-dev/results//generic/533.full for details) The key reason of the issue is returning -ENOENT error code from hfsplus_find_attr(), __hfsplus_delete_attr(), hfsplus_delete_attr_nolock(), hfsplus_delete_all_attrs(). The file exists but we don't have any xattr for this file. Finally, -ENODATA error code is expected by application logic. This patch reworks xattr logic of HFS+ by means exchanging the -ENOENT error code on -ENODATA error code if xattr has not been found for existing file or folder. sudo ./check generic/533 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 7.0.0-rc1+ #16 SMP PREEMPT_DYNAMIC Wed Mar 11 15:04:58 PDT 2026 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/533 33s ... 32s Ran: generic/533 Passed all 1 tests Closes: https://github.com/hfs-linux-kernel/hfs-linux-kernel/issues/184 cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20260312221920.1422683-2-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/attributes.c | 38 +++++++++++++++++++++++++++++--------- fs/hfsplus/hfsplus_fs.h | 7 ++++++- fs/hfsplus/xattr.c | 6 +++--- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/fs/hfsplus/attributes.c b/fs/hfsplus/attributes.c index 6585bcea731c..704635c65e9a 100644 --- a/fs/hfsplus/attributes.c +++ b/fs/hfsplus/attributes.c @@ -153,14 +153,22 @@ int hfsplus_find_attr(struct super_block *sb, u32 cnid, if (err) goto failed_find_attr; err = hfs_brec_find(fd, hfs_find_rec_by_key); - if (err) + if (err == -ENOENT) { + /* file exists but xattr is absent */ + err = -ENODATA; + goto failed_find_attr; + } else if (err) goto failed_find_attr; } else { err = hfsplus_attr_build_key(sb, fd->search_key, cnid, NULL); if (err) goto failed_find_attr; err = hfs_brec_find(fd, hfs_find_1st_rec_by_cnid); - if (err) + if (err == -ENOENT) { + /* file exists but xattr is absent */ + err = -ENODATA; + goto failed_find_attr; + } else if (err) goto failed_find_attr; } @@ -174,6 +182,9 @@ int hfsplus_attr_exists(struct inode *inode, const char *name) struct super_block *sb = inode->i_sb; struct hfs_find_data fd; + hfs_dbg("name %s, ino %ld\n", + name ? name : NULL, inode->i_ino); + if (!HFSPLUS_SB(sb)->attr_tree) return 0; @@ -293,15 +304,16 @@ int hfsplus_create_attr(struct inode *inode, static int __hfsplus_delete_attr(struct inode *inode, u32 cnid, struct hfs_find_data *fd) { - int err = 0; + int err; __be32 found_cnid, record_type; + found_cnid = U32_MAX; hfs_bnode_read(fd->bnode, &found_cnid, fd->keyoffset + offsetof(struct hfsplus_attr_key, cnid), sizeof(__be32)); if (cnid != be32_to_cpu(found_cnid)) - return -ENOENT; + return -ENODATA; hfs_bnode_read(fd->bnode, &record_type, fd->entryoffset, sizeof(record_type)); @@ -330,7 +342,7 @@ static int __hfsplus_delete_attr(struct inode *inode, u32 cnid, hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(inode->i_sb), HFSPLUS_I_ATTR_DIRTY); hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ATTR_DIRTY); - return err; + return 0; } static @@ -354,7 +366,10 @@ int hfsplus_delete_attr_nolock(struct inode *inode, const char *name, } err = hfs_brec_find(fd, hfs_find_rec_by_key); - if (err) + if (err == -ENOENT) { + /* file exists but xattr is absent */ + return -ENODATA; + } else if (err) return err; err = __hfsplus_delete_attr(inode, inode->i_ino, fd); @@ -414,9 +429,14 @@ int hfsplus_delete_all_attrs(struct inode *dir, u32 cnid) for (;;) { err = hfsplus_find_attr(dir->i_sb, cnid, NULL, &fd); - if (err) { - if (err != -ENOENT) - pr_err("xattr search failed\n"); + if (err == -ENOENT || err == -ENODATA) { + /* + * xattr has not been found + */ + err = -ENODATA; + goto end_delete_all; + } else if (err) { + pr_err("xattr search failed\n"); goto end_delete_all; } diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h index 420dc920e097..caba698814fe 100644 --- a/fs/hfsplus/hfsplus_fs.h +++ b/fs/hfsplus/hfsplus_fs.h @@ -571,7 +571,12 @@ hfsplus_btree_lock_class(struct hfs_btree *tree) static inline bool is_bnode_offset_valid(struct hfs_bnode *node, u32 off) { - bool is_valid = off < node->tree->node_size; + bool is_valid; + + if (!node || !node->tree) + return false; + + is_valid = off < node->tree->node_size; if (!is_valid) { pr_err("requested invalid offset: " diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c index a824bcaac172..89e2e7e46e96 100644 --- a/fs/hfsplus/xattr.c +++ b/fs/hfsplus/xattr.c @@ -562,10 +562,10 @@ ssize_t __hfsplus_getxattr(struct inode *inode, const char *name, res = hfsplus_find_attr(inode->i_sb, inode->i_ino, name, &fd); if (res) { - if (res == -ENOENT) + if (res == -ENOENT || res == -ENODATA) res = -ENODATA; else - pr_err("xattr searching failed\n"); + pr_err("xattr search failed\n"); goto out; } @@ -757,7 +757,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 (err == -ENOENT || err == -ENODATA) { res = 0; goto end_listxattr; } else { From b099ed598c64c8d275fc8877ec521b58712ab103 Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Tue, 17 Mar 2026 15:14:00 -0700 Subject: [PATCH 05/15] hfsplus: fix to update ctime after rename [BUG] $ sudo ./check generic/003 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 graphic 6.8.0-58-generic #60~22.04.1-Ubuntu MKFS_OPTIONS -- /dev/loop29 MOUNT_OPTIONS -- /dev/loop29 /mnt/scratch generic/003 - output mismatch --- tests/generic/003.out 2025-04-27 08:49:39.876945323 -0600 +++ /home/graphic/fs/xfstests-dev/results//generic/003.out.bad QA output created by 003 +ERROR: change time has not been updated after changing file1 Silence is golden ... Ran: generic/003 Failures: generic/003 Failed 1 of 1 tests [CAUSE] change time has not been updated after changing file1 [FIX] Update file ctime after rename in hfsplus_rename(). Signed-off-by: Yangtao Li Tested-by: Viacheslav Dubeyko Reviewed-by: Viacheslav Dubeyko Link: https://lore.kernel.org/linux-fsdevel/20250530081719.2430291-1-frank.li@vivo.com/ Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/dir.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c index 6059bd59d66e..8ad73378ed64 100644 --- a/fs/hfsplus/dir.c +++ b/fs/hfsplus/dir.c @@ -603,11 +603,22 @@ static int hfsplus_rename(struct mnt_idmap *idmap, old_dir, &old_dentry->d_name, new_dir, &new_dentry->d_name); if (!res) { + struct inode *inode = d_inode(old_dentry); + new_dentry->d_fsdata = old_dentry->d_fsdata; + inode_set_ctime_current(inode); + mark_inode_dirty(inode); + res = hfsplus_cat_write_inode(old_dir); - if (!res) - res = hfsplus_cat_write_inode(new_dir); + if (res) + return res; + + res = hfsplus_cat_write_inode(new_dir); + if (res) + return res; + + res = hfsplus_cat_write_inode(inode); } return res; } From a8eed0ba6a4b2f1803ecdfa9f11a4818cf87c474 Mon Sep 17 00:00:00 2001 From: Shardul Bankar Date: Wed, 18 Mar 2026 13:08:22 +0530 Subject: [PATCH 06/15] hfsplus: refactor b-tree map page access and add node-type validation In HFS+ b-trees, the node allocation bitmap is stored across multiple records. The first chunk resides in the b-tree Header Node at record index 2, while all subsequent chunks are stored in dedicated Map Nodes at record index 0. This structural quirk forces callers like hfs_bmap_alloc() and hfs_bmap_free() to duplicate boilerplate code to validate offsets, correct lengths, and map the underlying pages via kmap_local_page(). There is also currently no strict node-type validation before reading these records, leaving the allocator vulnerable if a corrupted image points a map linkage to an Index or Leaf node. Introduce a unified bit-level API to encapsulate the map record access: 1. A new `struct hfs_bmap_ctx` to cleanly pass state and safely handle page math across all architectures. 2. `hfs_bmap_get_map_page()`: Automatically validates node types (HFS_NODE_HEADER vs HFS_NODE_MAP), infers the correct record index, handles page-boundary math, and returns the unmapped `struct page *` directly to the caller to avoid asymmetric mappings. 3. `hfs_bmap_clear_bit()`: A clean wrapper that internally handles page mapping/unmapping for single-bit operations. Refactor hfs_bmap_alloc() and hfs_bmap_free() to utilize this new API. This deduplicates the allocator logic, hardens the map traversal against fuzzed images, and provides the exact abstractions needed for upcoming mount-time validation checks. Signed-off-by: Shardul Bankar Reviewed-by: Viacheslav Dubeyko Tested-by: Viacheslav Dubeyko Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20260318073823.3933718-2-shardul.b@mpiricsoftware.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/btree.c | 169 ++++++++++++++++++++++++++----------- include/linux/hfs_common.h | 2 + 2 files changed, 124 insertions(+), 47 deletions(-) diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c index 1220a2f22737..64d168347b4b 100644 --- a/fs/hfsplus/btree.c +++ b/fs/hfsplus/btree.c @@ -129,6 +129,95 @@ u32 hfsplus_calc_btree_clump_size(u32 block_size, u32 node_size, return clump_size; } +/* Context for iterating b-tree map pages + * @page_idx: The index of the page within the b-node's page array + * @off: The byte offset within the mapped page + * @len: The remaining length of the map record + */ +struct hfs_bmap_ctx { + unsigned int page_idx; + unsigned int off; + u16 len; +}; + +/* + * Finds the specific page containing the requested byte offset within the map + * record. Automatically handles the difference between header and map nodes. + * Returns the struct page pointer, or an ERR_PTR on failure. + * Note: The caller is responsible for mapping/unmapping the returned page. + */ +static struct page *hfs_bmap_get_map_page(struct hfs_bnode *node, struct hfs_bmap_ctx *ctx, + u32 byte_offset) +{ + u16 rec_idx, off16; + unsigned int page_off; + + if (node->this == HFSPLUS_TREE_HEAD) { + if (node->type != HFS_NODE_HEADER) { + pr_err("hfsplus: invalid btree header node\n"); + return ERR_PTR(-EIO); + } + rec_idx = HFSPLUS_BTREE_HDR_MAP_REC_INDEX; + } else { + if (node->type != HFS_NODE_MAP) { + pr_err("hfsplus: invalid btree map node\n"); + return ERR_PTR(-EIO); + } + rec_idx = HFSPLUS_BTREE_MAP_NODE_REC_INDEX; + } + + ctx->len = hfs_brec_lenoff(node, rec_idx, &off16); + if (!ctx->len) + return ERR_PTR(-ENOENT); + + if (!is_bnode_offset_valid(node, off16)) + return ERR_PTR(-EIO); + + ctx->len = check_and_correct_requested_length(node, off16, ctx->len); + + if (byte_offset >= ctx->len) + return ERR_PTR(-EINVAL); + + page_off = (u32)off16 + node->page_offset + byte_offset; + ctx->page_idx = page_off >> PAGE_SHIFT; + ctx->off = page_off & ~PAGE_MASK; + + return node->page[ctx->page_idx]; +} + +/** + * hfs_bmap_clear_bit - clear a bit in the b-tree map + * @node: the b-tree node containing the map record + * @node_bit_idx: the relative bit index within the node's map record + * + * Returns 0 on success, -EINVAL if already clear, or negative error code. + */ +static int hfs_bmap_clear_bit(struct hfs_bnode *node, u32 node_bit_idx) +{ + struct hfs_bmap_ctx ctx; + struct page *page; + u8 *bmap, mask; + + page = hfs_bmap_get_map_page(node, &ctx, node_bit_idx / BITS_PER_BYTE); + if (IS_ERR(page)) + return PTR_ERR(page); + + bmap = kmap_local_page(page); + + mask = 1 << (7 - (node_bit_idx % BITS_PER_BYTE)); + + if (!(bmap[ctx.off] & mask)) { + kunmap_local(bmap); + return -EINVAL; + } + + bmap[ctx.off] &= ~mask; + set_page_dirty(page); + kunmap_local(bmap); + + return 0; +} + /* Get a reference to a B*Tree and do some initial checks */ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id) { @@ -374,11 +463,9 @@ int hfs_bmap_reserve(struct hfs_btree *tree, u32 rsvd_nodes) struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree) { struct hfs_bnode *node, *next_node; - struct page **pagep; + struct hfs_bmap_ctx ctx; + struct page *page; u32 nidx, idx; - unsigned off; - u16 off16; - u16 len; u8 *data, byte, m; int i, res; @@ -390,30 +477,26 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree) node = hfs_bnode_find(tree, nidx); if (IS_ERR(node)) return node; - len = hfs_brec_lenoff(node, 2, &off16); - off = off16; - if (!is_bnode_offset_valid(node, off)) { + page = hfs_bmap_get_map_page(node, &ctx, 0); + if (IS_ERR(page)) { + res = PTR_ERR(page); hfs_bnode_put(node); - return ERR_PTR(-EIO); + return ERR_PTR(res); } - len = check_and_correct_requested_length(node, off, len); - off += node->page_offset; - pagep = node->page + (off >> PAGE_SHIFT); - data = kmap_local_page(*pagep); - off &= ~PAGE_MASK; + data = kmap_local_page(page); idx = 0; for (;;) { - while (len) { - byte = data[off]; + while (ctx.len) { + byte = data[ctx.off]; if (byte != 0xff) { for (m = 0x80, i = 0; i < 8; m >>= 1, i++) { if (!(byte & m)) { idx += i; - data[off] |= m; - set_page_dirty(*pagep); + data[ctx.off] |= m; + set_page_dirty(page); kunmap_local(data); tree->free_nodes--; mark_inode_dirty(tree->inode); @@ -423,13 +506,14 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree) } } } - if (++off >= PAGE_SIZE) { + if (++ctx.off >= PAGE_SIZE) { kunmap_local(data); - data = kmap_local_page(*++pagep); - off = 0; + page = node->page[++ctx.page_idx]; + data = kmap_local_page(page); + ctx.off = 0; } idx += 8; - len--; + ctx.len--; } kunmap_local(data); nidx = node->next; @@ -443,22 +527,22 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree) return next_node; node = next_node; - len = hfs_brec_lenoff(node, 0, &off16); - off = off16; - off += node->page_offset; - pagep = node->page + (off >> PAGE_SHIFT); - data = kmap_local_page(*pagep); - off &= ~PAGE_MASK; + page = hfs_bmap_get_map_page(node, &ctx, 0); + if (IS_ERR(page)) { + res = PTR_ERR(page); + hfs_bnode_put(node); + return ERR_PTR(res); + } + data = kmap_local_page(page); } } void hfs_bmap_free(struct hfs_bnode *node) { struct hfs_btree *tree; - struct page *page; u16 off, len; u32 nidx; - u8 *data, byte, m; + int res; hfs_dbg("node %u\n", node->this); BUG_ON(!node->this); @@ -495,24 +579,15 @@ void hfs_bmap_free(struct hfs_bnode *node) } len = hfs_brec_lenoff(node, 0, &off); } - off += node->page_offset + nidx / 8; - page = node->page[off >> PAGE_SHIFT]; - data = kmap_local_page(page); - off &= ~PAGE_MASK; - m = 1 << (~nidx & 7); - byte = data[off]; - if (!(byte & m)) { - pr_crit("trying to free free bnode " - "%u(%d)\n", - node->this, node->type); - kunmap_local(data); - hfs_bnode_put(node); - return; + + res = hfs_bmap_clear_bit(node, nidx); + if (res == -EINVAL) { + pr_crit("trying to free free bnode %u(%d)\n", + node->this, node->type); + } else if (!res) { + tree->free_nodes++; + mark_inode_dirty(tree->inode); } - data[off] = byte & ~m; - set_page_dirty(page); - kunmap_local(data); + hfs_bnode_put(node); - tree->free_nodes++; - mark_inode_dirty(tree->inode); } diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h index dadb5e0aa8a3..be24c687858e 100644 --- a/include/linux/hfs_common.h +++ b/include/linux/hfs_common.h @@ -510,6 +510,8 @@ struct hfs_btree_header_rec { #define HFSPLUS_NODE_MXSZ 32768 #define HFSPLUS_ATTR_TREE_NODE_SIZE 8192 #define HFSPLUS_BTREE_HDR_NODE_RECS_COUNT 3 +#define HFSPLUS_BTREE_HDR_MAP_REC_INDEX 2 /* Map (bitmap) record in Header node */ +#define HFSPLUS_BTREE_MAP_NODE_REC_INDEX 0 /* Map record in Map Node */ #define HFSPLUS_BTREE_HDR_USER_BYTES 128 /* btree key type */ From 8ad2c6a36ac4328072377906a47ea0bff11e4032 Mon Sep 17 00:00:00 2001 From: Shardul Bankar Date: Wed, 18 Mar 2026 13:08:23 +0530 Subject: [PATCH 07/15] hfsplus: validate b-tree node 0 bitmap at mount time Syzkaller reported an issue with corrupted HFS+ images where the b-tree allocation bitmap indicates that the header node (Node 0) is free. Node 0 must always be allocated as it contains the b-tree header record and the allocation bitmap itself. Violating this invariant leads to allocator corruption, which cascades into kernel panics or undefined behavior when the filesystem attempts to allocate blocks. Prevent trusting a corrupted allocator state by adding a validation check during hfs_btree_open(). Introduce the hfs_bmap_test_bit() helper (utilizing the newly added map-access API) to safely verify that the MSB of the first bitmap byte (representing Node 0) is marked as allocated. The helper returns a boolean, allowing the caller to safely catch both structural IO errors and illegally cleared bits in a single check. If corruption is detected, print a warning identifying the specific corrupted tree and force the filesystem to mount read-only (SB_RDONLY). This prevents kernel panics from corrupted images while enabling data recovery. As a minor cleanup to support the warning logs, replace the verbose CNID logic with cleaner macro definitions (using official structural names like "Extents Overflow File") and a dedicated string lookup helper. Reported-by: syzbot+1c8ff72d0cd8a50dfeaa@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?extid=1c8ff72d0cd8a50dfeaa Link: https://lore.kernel.org/all/20260315172005.2066677-1-shardul.b@mpiricsoftware.com/ Signed-off-by: Shardul Bankar Reviewed-by: Viacheslav Dubeyko Tested-by: Viacheslav Dubeyko Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20260318073823.3933718-3-shardul.b@mpiricsoftware.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/btree.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c index 64d168347b4b..04304f952f6b 100644 --- a/fs/hfsplus/btree.c +++ b/fs/hfsplus/btree.c @@ -185,6 +185,32 @@ static struct page *hfs_bmap_get_map_page(struct hfs_bnode *node, struct hfs_bma return node->page[ctx->page_idx]; } +/** + * hfs_bmap_test_bit - test a bit in the b-tree map + * @node: the b-tree node containing the map record + * @node_bit_idx: the relative bit index within the node's map record + * + * Returns true if set, false if clear or on failure. + */ +static bool hfs_bmap_test_bit(struct hfs_bnode *node, u32 node_bit_idx) +{ + struct hfs_bmap_ctx ctx; + struct page *page; + u8 *bmap, byte, mask; + + page = hfs_bmap_get_map_page(node, &ctx, node_bit_idx / BITS_PER_BYTE); + if (IS_ERR(page)) + return false; + + bmap = kmap_local_page(page); + byte = bmap[ctx.off]; + kunmap_local(bmap); + + mask = 1 << (7 - (node_bit_idx % BITS_PER_BYTE)); + return (byte & mask) != 0; +} + + /** * hfs_bmap_clear_bit - clear a bit in the b-tree map * @node: the b-tree node containing the map record @@ -218,12 +244,32 @@ static int hfs_bmap_clear_bit(struct hfs_bnode *node, u32 node_bit_idx) return 0; } +#define HFS_EXTENT_TREE_NAME "Extents Overflow File" +#define HFS_CATALOG_TREE_NAME "Catalog File" +#define HFS_ATTR_TREE_NAME "Attributes File" +#define HFS_UNKNOWN_TREE_NAME "Unknown B-tree" + +static const char *hfs_btree_name(u32 cnid) +{ + switch (cnid) { + case HFSPLUS_EXT_CNID: + return HFS_EXTENT_TREE_NAME; + case HFSPLUS_CAT_CNID: + return HFS_CATALOG_TREE_NAME; + case HFSPLUS_ATTR_CNID: + return HFS_ATTR_TREE_NAME; + default: + return HFS_UNKNOWN_TREE_NAME; + } +} + /* Get a reference to a B*Tree and do some initial checks */ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id) { struct hfs_btree *tree; struct hfs_btree_header_rec *head; struct address_space *mapping; + struct hfs_bnode *node; struct inode *inode; struct page *page; unsigned int size; @@ -331,6 +377,20 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id) kunmap_local(head); put_page(page); + + node = hfs_bnode_find(tree, HFSPLUS_TREE_HEAD); + if (IS_ERR(node)) + goto free_inode; + + if (!hfs_bmap_test_bit(node, 0)) { + pr_warn("(%s): %s (cnid 0x%x) map record invalid or bitmap corruption detected, forcing read-only.\n", + sb->s_id, hfs_btree_name(id), id); + pr_warn("Run fsck.hfsplus to repair.\n"); + sb->s_flags |= SB_RDONLY; + } + + hfs_bnode_put(node); + return tree; fail_page: From 897c2beb4a7799154a67942fa85a9678f885f36b Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Mon, 23 Mar 2026 17:39:50 -0700 Subject: [PATCH 08/15] hfsplus: fix generic/523 test-case failure The xfstests' test-case generic/523 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/523 - output mismatch (see xfstests-dev/results//generic/523.out.bad) The test-case expects to have '/' in the xattr name. However, HFS+ unicode logic makes conversion of '/' into ':'. In HFS+, a filename can contain '/' because ':' is the separator. The slash is a valid filename character on macOS. But on Linux, / is the path separator and it cannot appear in a filename component. But xattr name can contain any of these symbols. It means that this unicode logic conversion doesn't need to be executed for the case of xattr name. This patch adds distinguishing the regular and xattr names. If we have a regular name, then this conversion of special symbols will be executed. Otherwise, the conversion is skipped for the case of xattr names. sudo ./check -g auto FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 7.0.0-rc1+ #24 SMP PREEMPT_DYNAMIC Fri Mar 20 12:36:49 PDT 2026 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/523 33s ... 25s Closes: https://github.com/hfs-linux-kernel/hfs-linux-kernel/issues/178 cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20260324003949.417048-2-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/attributes.c | 3 +- fs/hfsplus/catalog.c | 4 +- fs/hfsplus/hfsplus_fs.h | 3 +- fs/hfsplus/unicode.c | 121 ++++++++++++++++++++++++++----------- fs/hfsplus/unicode_test.c | 51 ++++++++++------ include/linux/hfs_common.h | 5 ++ 6 files changed, 132 insertions(+), 55 deletions(-) diff --git a/fs/hfsplus/attributes.c b/fs/hfsplus/attributes.c index 704635c65e9a..fa87496409c9 100644 --- a/fs/hfsplus/attributes.c +++ b/fs/hfsplus/attributes.c @@ -57,7 +57,8 @@ int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key, if (name) { int res = hfsplus_asc2uni(sb, (struct hfsplus_unistr *)&key->attr.key_name, - HFSPLUS_ATTR_MAX_STRLEN, name, strlen(name)); + HFSPLUS_ATTR_MAX_STRLEN, name, strlen(name), + HFS_XATTR_NAME); if (res) return res; len = be16_to_cpu(key->attr.key_name.length); diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c index 708046c5dae6..1f7ef61fc318 100644 --- a/fs/hfsplus/catalog.c +++ b/fs/hfsplus/catalog.c @@ -47,7 +47,7 @@ int hfsplus_cat_build_key(struct super_block *sb, key->cat.parent = cpu_to_be32(parent); err = hfsplus_asc2uni(sb, &key->cat.name, HFSPLUS_MAX_STRLEN, - str->name, str->len); + str->name, str->len, HFS_REGULAR_NAME); if (unlikely(err < 0)) return err; @@ -183,7 +183,7 @@ static int hfsplus_fill_cat_thread(struct super_block *sb, entry->thread.reserved = 0; entry->thread.parentID = cpu_to_be32(parentid); err = hfsplus_asc2uni(sb, &entry->thread.nodeName, HFSPLUS_MAX_STRLEN, - str->name, str->len); + str->name, str->len, HFS_REGULAR_NAME); if (unlikely(err < 0)) return err; diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h index caba698814fe..3545b8dbf11c 100644 --- a/fs/hfsplus/hfsplus_fs.h +++ b/fs/hfsplus/hfsplus_fs.h @@ -506,7 +506,8 @@ int hfsplus_uni2asc_xattr_str(struct super_block *sb, const struct hfsplus_attr_unistr *ustr, char *astr, int *len_p); int hfsplus_asc2uni(struct super_block *sb, struct hfsplus_unistr *ustr, - int max_unistr_len, const char *astr, int len); + int max_unistr_len, const char *astr, int len, + int name_type); int hfsplus_hash_dentry(const struct dentry *dentry, struct qstr *str); int hfsplus_compare_dentry(const struct dentry *dentry, unsigned int len, const char *str, const struct qstr *name); diff --git a/fs/hfsplus/unicode.c b/fs/hfsplus/unicode.c index d3a142f4518b..008fec186382 100644 --- a/fs/hfsplus/unicode.c +++ b/fs/hfsplus/unicode.c @@ -147,9 +147,44 @@ static u16 *hfsplus_compose_lookup(u16 *p, u16 cc) return NULL; } +/* + * In HFS+, a filename can contain / because : is the separator. + * The slash is a valid filename character on macOS. + * But on Linux, / is the path separator and + * it cannot appear in a filename component. + * There's a parallel mapping for the NUL character (0 -> U+2400). + * NUL terminates strings in C/POSIX but is valid in HFS+ filenames. + */ +static inline +void hfsplus_mac2linux_compatibility_check(u16 symbol, u16 *conversion, + int name_type) +{ + *conversion = symbol; + + switch (name_type) { + case HFS_XATTR_NAME: + /* ignore conversion */ + return; + + default: + /* continue logic */ + break; + } + + switch (symbol) { + case 0: + *conversion = 0x2400; + break; + case '/': + *conversion = ':'; + break; + } +} + static int hfsplus_uni2asc(struct super_block *sb, const struct hfsplus_unistr *ustr, - int max_len, char *astr, int *len_p) + int max_len, char *astr, int *len_p, + int name_type) { const hfsplus_unichr *ip; struct nls_table *nls = HFSPLUS_SB(sb)->nls; @@ -217,14 +252,8 @@ static int hfsplus_uni2asc(struct super_block *sb, hfsplus_compose_table, c1); if (ce1) break; - switch (c0) { - case 0: - c0 = 0x2400; - break; - case '/': - c0 = ':'; - break; - } + hfsplus_mac2linux_compatibility_check(c0, &c0, + name_type); res = nls->uni2char(c0, op, len); if (res < 0) { if (res == -ENAMETOOLONG) @@ -257,16 +286,8 @@ static int hfsplus_uni2asc(struct super_block *sb, } } same: - switch (c0) { - case 0: - cc = 0x2400; - break; - case '/': - cc = ':'; - break; - default: - cc = c0; - } + hfsplus_mac2linux_compatibility_check(c0, &cc, + name_type); done: res = nls->uni2char(cc, op, len); if (res < 0) { @@ -288,7 +309,10 @@ inline int hfsplus_uni2asc_str(struct super_block *sb, const struct hfsplus_unistr *ustr, char *astr, int *len_p) { - return hfsplus_uni2asc(sb, ustr, HFSPLUS_MAX_STRLEN, astr, len_p); + return hfsplus_uni2asc(sb, + ustr, HFSPLUS_MAX_STRLEN, + astr, len_p, + HFS_REGULAR_NAME); } EXPORT_SYMBOL_IF_KUNIT(hfsplus_uni2asc_str); @@ -297,22 +321,32 @@ inline int hfsplus_uni2asc_xattr_str(struct super_block *sb, char *astr, int *len_p) { return hfsplus_uni2asc(sb, (const struct hfsplus_unistr *)ustr, - HFSPLUS_ATTR_MAX_STRLEN, astr, len_p); + HFSPLUS_ATTR_MAX_STRLEN, astr, len_p, + HFS_XATTR_NAME); } EXPORT_SYMBOL_IF_KUNIT(hfsplus_uni2asc_xattr_str); /* - * Convert one or more ASCII characters into a single unicode character. - * Returns the number of ASCII characters corresponding to the unicode char. + * In HFS+, a filename can contain / because : is the separator. + * The slash is a valid filename character on macOS. + * But on Linux, / is the path separator and + * it cannot appear in a filename component. + * There's a parallel mapping for the NUL character (0 -> U+2400). + * NUL terminates strings in C/POSIX but is valid in HFS+ filenames. */ -static inline int asc2unichar(struct super_block *sb, const char *astr, int len, - wchar_t *uc) +static inline +void hfsplus_linux2mac_compatibility_check(wchar_t *uc, int name_type) { - int size = HFSPLUS_SB(sb)->nls->char2uni(astr, len, uc); - if (size <= 0) { - *uc = '?'; - size = 1; + switch (name_type) { + case HFS_XATTR_NAME: + /* ignore conversion */ + return; + + default: + /* continue logic */ + break; } + switch (*uc) { case 0x2400: *uc = 0; @@ -321,6 +355,23 @@ static inline int asc2unichar(struct super_block *sb, const char *astr, int len, *uc = '/'; break; } +} + +/* + * Convert one or more ASCII characters into a single unicode character. + * Returns the number of ASCII characters corresponding to the unicode char. + */ +static inline int asc2unichar(struct super_block *sb, const char *astr, int len, + wchar_t *uc, int name_type) +{ + int size = HFSPLUS_SB(sb)->nls->char2uni(astr, len, uc); + + if (size <= 0) { + *uc = '?'; + size = 1; + } + + hfsplus_linux2mac_compatibility_check(uc, name_type); return size; } @@ -395,7 +446,7 @@ static u16 *decompose_unichar(wchar_t uc, int *size, u16 *hangul_buffer) int hfsplus_asc2uni(struct super_block *sb, struct hfsplus_unistr *ustr, int max_unistr_len, - const char *astr, int len) + const char *astr, int len, int name_type) { int size, dsize, decompose; u16 *dstr, outlen = 0; @@ -404,7 +455,7 @@ int hfsplus_asc2uni(struct super_block *sb, decompose = !test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags); while (outlen < max_unistr_len && len > 0) { - size = asc2unichar(sb, astr, len, &c); + size = asc2unichar(sb, astr, len, &c, name_type); if (decompose) dstr = decompose_unichar(c, &dsize, dhangul); @@ -452,7 +503,7 @@ int hfsplus_hash_dentry(const struct dentry *dentry, struct qstr *str) len = str->len; while (len > 0) { int dsize; - size = asc2unichar(sb, astr, len, &c); + size = asc2unichar(sb, astr, len, &c, HFS_REGULAR_NAME); astr += size; len -= size; @@ -510,7 +561,8 @@ int hfsplus_compare_dentry(const struct dentry *dentry, while (len1 > 0 && len2 > 0) { if (!dsize1) { - size = asc2unichar(sb, astr1, len1, &c); + size = asc2unichar(sb, astr1, len1, &c, + HFS_REGULAR_NAME); astr1 += size; len1 -= size; @@ -525,7 +577,8 @@ int hfsplus_compare_dentry(const struct dentry *dentry, } if (!dsize2) { - size = asc2unichar(sb, astr2, len2, &c); + size = asc2unichar(sb, astr2, len2, &c, + HFS_REGULAR_NAME); astr2 += size; len2 -= size; diff --git a/fs/hfsplus/unicode_test.c b/fs/hfsplus/unicode_test.c index 26145bf88946..83737c9bafa0 100644 --- a/fs/hfsplus/unicode_test.c +++ b/fs/hfsplus/unicode_test.c @@ -715,28 +715,32 @@ static void hfsplus_asc2uni_basic_test(struct kunit *test) /* Test simple ASCII string conversion */ result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str1, - HFSPLUS_MAX_STRLEN, "hello", 5); + HFSPLUS_MAX_STRLEN, "hello", 5, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); check_unistr_content(test, &mock_env->str1, "hello"); /* Test empty string */ result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str1, - HFSPLUS_MAX_STRLEN, "", 0); + HFSPLUS_MAX_STRLEN, "", 0, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); KUNIT_EXPECT_EQ(test, 0, be16_to_cpu(mock_env->str1.length)); /* Test single character */ result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str1, - HFSPLUS_MAX_STRLEN, "A", 1); + HFSPLUS_MAX_STRLEN, "A", 1, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); check_unistr_content(test, &mock_env->str1, "A"); /* Test null-terminated string with explicit length */ result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str1, - HFSPLUS_MAX_STRLEN, "test\0extra", 4); + HFSPLUS_MAX_STRLEN, "test\0extra", 4, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); check_unistr_content(test, &mock_env->str1, "test"); @@ -762,7 +766,8 @@ static void hfsplus_asc2uni_special_chars_test(struct kunit *test) /* Test colon conversion (should become forward slash) */ result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str1, - HFSPLUS_MAX_STRLEN, ":", 1); + HFSPLUS_MAX_STRLEN, ":", 1, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); KUNIT_EXPECT_EQ(test, 1, be16_to_cpu(mock_env->str1.length)); @@ -770,7 +775,8 @@ static void hfsplus_asc2uni_special_chars_test(struct kunit *test) /* Test string with mixed special characters */ result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str1, - HFSPLUS_MAX_STRLEN, "a:b", 3); + HFSPLUS_MAX_STRLEN, "a:b", 3, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); KUNIT_EXPECT_EQ(test, 3, be16_to_cpu(mock_env->str1.length)); @@ -780,7 +786,8 @@ static void hfsplus_asc2uni_special_chars_test(struct kunit *test) /* Test multiple special characters */ result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str1, - HFSPLUS_MAX_STRLEN, ":::", 3); + HFSPLUS_MAX_STRLEN, ":::", 3, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); KUNIT_EXPECT_EQ(test, 3, be16_to_cpu(mock_env->str1.length)); @@ -811,7 +818,8 @@ static void hfsplus_asc2uni_buffer_limits_test(struct kunit *test) memset(mock_env->buf, 'a', HFSPLUS_MAX_STRLEN); result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str1, HFSPLUS_MAX_STRLEN, - mock_env->buf, HFSPLUS_MAX_STRLEN); + mock_env->buf, HFSPLUS_MAX_STRLEN, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); KUNIT_EXPECT_EQ(test, HFSPLUS_MAX_STRLEN, @@ -821,7 +829,8 @@ static void hfsplus_asc2uni_buffer_limits_test(struct kunit *test) memset(mock_env->buf, 'a', HFSPLUS_MAX_STRLEN + 5); result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str1, HFSPLUS_MAX_STRLEN, - mock_env->buf, HFSPLUS_MAX_STRLEN + 5); + mock_env->buf, HFSPLUS_MAX_STRLEN + 5, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, -ENAMETOOLONG, result); KUNIT_EXPECT_EQ(test, HFSPLUS_MAX_STRLEN, @@ -829,13 +838,15 @@ static void hfsplus_asc2uni_buffer_limits_test(struct kunit *test) /* Test with smaller max_unistr_len */ result = hfsplus_asc2uni(&mock_sb->sb, - &mock_env->str1, 5, "toolongstring", 13); + &mock_env->str1, 5, "toolongstring", 13, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, -ENAMETOOLONG, result); KUNIT_EXPECT_EQ(test, 5, be16_to_cpu(mock_env->str1.length)); /* Test zero max length */ - result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str1, 0, "test", 4); + result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str1, 0, "test", 4, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, -ENAMETOOLONG, result); KUNIT_EXPECT_EQ(test, 0, be16_to_cpu(mock_env->str1.length)); @@ -859,28 +870,32 @@ static void hfsplus_asc2uni_edge_cases_test(struct kunit *test) /* Test zero length input */ result = hfsplus_asc2uni(&mock_sb->sb, - &ustr, HFSPLUS_MAX_STRLEN, "test", 0); + &ustr, HFSPLUS_MAX_STRLEN, "test", 0, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); KUNIT_EXPECT_EQ(test, 0, be16_to_cpu(ustr.length)); /* Test input with length mismatch */ result = hfsplus_asc2uni(&mock_sb->sb, - &ustr, HFSPLUS_MAX_STRLEN, "hello", 3); + &ustr, HFSPLUS_MAX_STRLEN, "hello", 3, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); check_unistr_content(test, &ustr, "hel"); /* Test with various printable ASCII characters */ result = hfsplus_asc2uni(&mock_sb->sb, - &ustr, HFSPLUS_MAX_STRLEN, "ABC123!@#", 9); + &ustr, HFSPLUS_MAX_STRLEN, "ABC123!@#", 9, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); check_unistr_content(test, &ustr, "ABC123!@#"); /* Test null character in the middle */ result = hfsplus_asc2uni(&mock_sb->sb, - &ustr, HFSPLUS_MAX_STRLEN, test_str, 3); + &ustr, HFSPLUS_MAX_STRLEN, test_str, 3, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); KUNIT_EXPECT_EQ(test, 3, be16_to_cpu(ustr.length)); @@ -909,7 +924,8 @@ static void hfsplus_asc2uni_decompose_test(struct kunit *test) /* Test with decomposition disabled (default) */ clear_bit(HFSPLUS_SB_NODECOMPOSE, &mock_sb->sb_info.flags); result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str1, - HFSPLUS_MAX_STRLEN, "test", 4); + HFSPLUS_MAX_STRLEN, "test", 4, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); check_unistr_content(test, &mock_env->str1, "test"); @@ -917,7 +933,8 @@ static void hfsplus_asc2uni_decompose_test(struct kunit *test) /* Test with decomposition enabled */ set_bit(HFSPLUS_SB_NODECOMPOSE, &mock_sb->sb_info.flags); result = hfsplus_asc2uni(&mock_sb->sb, &mock_env->str2, - HFSPLUS_MAX_STRLEN, "test", 4); + HFSPLUS_MAX_STRLEN, "test", 4, + HFS_REGULAR_NAME); KUNIT_EXPECT_EQ(test, 0, result); check_unistr_content(test, &mock_env->str2, "test"); diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h index be24c687858e..9e71b9a03b60 100644 --- a/include/linux/hfs_common.h +++ b/include/linux/hfs_common.h @@ -166,6 +166,11 @@ struct hfsplus_attr_unistr { hfsplus_unichr unicode[HFSPLUS_ATTR_MAX_STRLEN]; } __packed; +enum { + HFS_REGULAR_NAME, + HFS_XATTR_NAME, +}; + struct hfs_extent { __be16 block; __be16 count; From 90c500e4fd83fa33c09bc7ee23b6d9cc487ac733 Mon Sep 17 00:00:00 2001 From: Zilin Guan Date: Fri, 27 Mar 2026 16:47:41 +0800 Subject: [PATCH 09/15] hfsplus: fix held lock freed on hfsplus_fill_super() hfsplus_fill_super() calls hfs_find_init() to initialize a search structure, which acquires tree->tree_lock. If the subsequent call to hfsplus_cat_build_key() fails, the function jumps to the out_put_root error label without releasing the lock. The later cleanup path then frees the tree data structure with the lock still held, triggering a held lock freed warning. Fix this by adding the missing hfs_find_exit(&fd) call before jumping to the out_put_root error label. This ensures that tree->tree_lock is properly released on the error path. The bug was originally detected on v6.13-rc1 using an experimental static analysis tool we are developing, and we have verified that the issue persists in the latest mainline kernel. The tool is specifically designed to detect memory management issues. It is currently under active development and not yet publicly available. We confirmed the bug by runtime testing under QEMU with x86_64 defconfig, lockdep enabled, and CONFIG_HFSPLUS_FS=y. To trigger the error path, we used GDB to dynamically shrink the max_unistr_len parameter to 1 before hfsplus_asc2uni() is called. This forces hfsplus_asc2uni() to naturally return -ENAMETOOLONG, which propagates to hfsplus_cat_build_key() and exercises the faulty error path. The following warning was observed during mount: ========================= WARNING: held lock freed! 7.0.0-rc3-00016-gb4f0dd314b39 #4 Not tainted ------------------------- mount/174 is freeing memory ffff888103f92000-ffff888103f92fff, with a lock still held there! ffff888103f920b0 (&tree->tree_lock){+.+.}-{4:4}, at: hfsplus_find_init+0x154/0x1e0 2 locks held by mount/174: #0: ffff888103f960e0 (&type->s_umount_key#42/1){+.+.}-{4:4}, at: alloc_super.constprop.0+0x167/0xa40 #1: ffff888103f920b0 (&tree->tree_lock){+.+.}-{4:4}, at: hfsplus_find_init+0x154/0x1e0 stack backtrace: CPU: 2 UID: 0 PID: 174 Comm: mount Not tainted 7.0.0-rc3-00016-gb4f0dd314b39 #4 PREEMPT(lazy) Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014 Call Trace: dump_stack_lvl+0x82/0xd0 debug_check_no_locks_freed+0x13a/0x180 kfree+0x16b/0x510 ? hfsplus_fill_super+0xcb4/0x18a0 hfsplus_fill_super+0xcb4/0x18a0 ? __pfx_hfsplus_fill_super+0x10/0x10 ? srso_return_thunk+0x5/0x5f ? bdev_open+0x65f/0xc30 ? srso_return_thunk+0x5/0x5f ? pointer+0x4ce/0xbf0 ? trace_contention_end+0x11c/0x150 ? __pfx_pointer+0x10/0x10 ? srso_return_thunk+0x5/0x5f ? bdev_open+0x79b/0xc30 ? srso_return_thunk+0x5/0x5f ? srso_return_thunk+0x5/0x5f ? vsnprintf+0x6da/0x1270 ? srso_return_thunk+0x5/0x5f ? __mutex_unlock_slowpath+0x157/0x740 ? __pfx_vsnprintf+0x10/0x10 ? srso_return_thunk+0x5/0x5f ? srso_return_thunk+0x5/0x5f ? mark_held_locks+0x49/0x80 ? srso_return_thunk+0x5/0x5f ? srso_return_thunk+0x5/0x5f ? irqentry_exit+0x17b/0x5e0 ? trace_irq_disable.constprop.0+0x116/0x150 ? __pfx_hfsplus_fill_super+0x10/0x10 ? __pfx_hfsplus_fill_super+0x10/0x10 get_tree_bdev_flags+0x302/0x580 ? __pfx_get_tree_bdev_flags+0x10/0x10 ? vfs_parse_fs_qstr+0x129/0x1a0 ? __pfx_vfs_parse_fs_qstr+0x3/0x10 vfs_get_tree+0x89/0x320 fc_mount+0x10/0x1d0 path_mount+0x5c5/0x21c0 ? __pfx_path_mount+0x10/0x10 ? trace_irq_enable.constprop.0+0x116/0x150 ? trace_irq_enable.constprop.0+0x116/0x150 ? srso_return_thunk+0x5/0x5f ? srso_return_thunk+0x5/0x5f ? kmem_cache_free+0x307/0x540 ? user_path_at+0x51/0x60 ? __x64_sys_mount+0x212/0x280 ? srso_return_thunk+0x5/0x5f __x64_sys_mount+0x212/0x280 ? __pfx___x64_sys_mount+0x10/0x10 ? srso_return_thunk+0x5/0x5f ? trace_irq_enable.constprop.0+0x116/0x150 ? srso_return_thunk+0x5/0x5f do_syscall_64+0x111/0x680 entry_SYSCALL_64_after_hwframe+0x77/0x7f RIP: 0033:0x7ffacad55eae Code: 48 8b 0d 85 1f 0f 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 8 RSP: 002b:00007fff1ab55718 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007ffacad55eae RDX: 000055740c64e5b0 RSI: 000055740c64e630 RDI: 000055740c651ab0 RBP: 000055740c64e380 R08: 0000000000000000 R09: 0000000000000001 R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 R13: 000055740c64e5b0 R14: 000055740c651ab0 R15: 000055740c64e380 After applying this patch, the warning no longer appears. Fixes: 89ac9b4d3d1a ("hfsplus: fix longname handling") CC: stable@vger.kernel.org Signed-off-by: Zilin Guan Reviewed-by: Viacheslav Dubeyko Tested-by: Viacheslav Dubeyko Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/super.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index 242ccca3acb7..a36243b08c7c 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -569,8 +569,10 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc) if (err) goto out_put_root; err = hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str); - if (unlikely(err < 0)) + if (unlikely(err < 0)) { + hfs_find_exit(&fd); goto out_put_root; + } if (!hfsplus_brec_read_cat(&fd, &entry)) { hfs_find_exit(&fd); if (entry.type != cpu_to_be16(HFSPLUS_FOLDER)) { From d47059dcc472ae823c7eebe87fb7cec9148b9f06 Mon Sep 17 00:00:00 2001 From: Zilin Guan Date: Fri, 27 Mar 2026 16:47:42 +0800 Subject: [PATCH 10/15] hfsplus: extract hidden directory search into a helper function In hfsplus_fill_super(), the process of looking up the hidden directory involves initializing a catalog search, building a search key, reading the b-tree record, and releasing the search data. Currently, this logic is open-coded directly within the main superblock initialization routine. This makes hfsplus_fill_super() quite lengthy and its error handling paths less straightforward. Extract the hidden directory search sequence into a new helper function, hfsplus_get_hidden_dir_entry(). This improves overall code readability, cleanly encapsulates the hfs_find_data lifecycle, and simplifies the error exits in hfsplus_fill_super(). Signed-off-by: Zilin Guan Reviewed-by: Viacheslav Dubeyko Tested-by: Viacheslav Dubeyko Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/super.c | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index a36243b08c7c..8bd1627ffc9c 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -424,12 +424,35 @@ void hfsplus_prepare_volume_header_for_commit(struct hfsplus_vh *vhdr) vhdr->attributes |= cpu_to_be32(HFSPLUS_VOL_INCNSTNT); } +static inline int hfsplus_get_hidden_dir_entry(struct super_block *sb, + const struct qstr *str, + hfsplus_cat_entry *entry) +{ + struct hfs_find_data fd; + int err; + + err = hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd); + if (unlikely(err)) + return err; + + err = hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, str); + if (unlikely(err)) + goto free_fd; + + err = hfsplus_brec_read_cat(&fd, entry); + if (err) + err = -ENOENT; + +free_fd: + hfs_find_exit(&fd); + return err; +} + static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc) { struct hfsplus_vh *vhdr; struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb); hfsplus_cat_entry entry; - struct hfs_find_data fd; struct inode *root, *inode; struct qstr str; struct nls_table *nls; @@ -565,16 +588,14 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc) str.len = sizeof(HFSP_HIDDENDIR_NAME) - 1; str.name = HFSP_HIDDENDIR_NAME; - err = hfs_find_init(sbi->cat_tree, &fd); - if (err) + err = hfsplus_get_hidden_dir_entry(sb, &str, &entry); + if (err == -ENOENT) { + /* + * Hidden directory is absent or it cannot be read. + */ + } else if (unlikely(err)) { goto out_put_root; - err = hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str); - if (unlikely(err < 0)) { - hfs_find_exit(&fd); - goto out_put_root; - } - if (!hfsplus_brec_read_cat(&fd, &entry)) { - hfs_find_exit(&fd); + } else { if (entry.type != cpu_to_be16(HFSPLUS_FOLDER)) { err = -EIO; goto out_put_root; @@ -585,8 +606,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc) goto out_put_root; } sbi->hidden_dir = inode; - } else - hfs_find_exit(&fd); + } if (!sb_rdonly(sb)) { /* From 6dca66d7ba1767d1e8688ee63162eca8d2248e8c Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Fri, 3 Apr 2026 16:05:52 -0700 Subject: [PATCH 11/15] hfsplus: fix potential race conditions in b-tree functionality The HFS_BNODE_DELETED flag is checked in hfs_bnode_put() under locked tree->hash_lock. This patch adds locking for the case of setting the HFS_BNODE_DELETED flag in hfs_bnode_unlink() with the goal to avoid potential race conditions. The hfs_btree_write() method should be called under tree->tree_lock. This patch reworks logic by adding locking the tree->tree_lock for the calls of hfs_btree_write() in hfsplus_cat_write_inode() and hfsplus_system_write_inode(). This patch adds also the lockdep_assert_held() in hfs_bmap_reserve(), hfs_bmap_alloc(), and hfs_bmap_free(). cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20260403230556.614171-2-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/bnode.c | 3 +++ fs/hfsplus/btree.c | 5 +++++ fs/hfsplus/inode.c | 15 +++++++-------- fs/hfsplus/super.c | 3 +++ 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c index 250a226336ea..f8b5a8ae58ff 100644 --- a/fs/hfsplus/bnode.c +++ b/fs/hfsplus/bnode.c @@ -420,7 +420,10 @@ void hfs_bnode_unlink(struct hfs_bnode *node) tree->root = 0; tree->depth = 0; } + + spin_lock(&tree->hash_lock); set_bit(HFS_BNODE_DELETED, &node->flags); + spin_unlock(&tree->hash_lock); } static inline int hfs_bnode_hash(u32 num) diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c index 04304f952f6b..f1ed7c336593 100644 --- a/fs/hfsplus/btree.c +++ b/fs/hfsplus/btree.c @@ -500,6 +500,8 @@ int hfs_bmap_reserve(struct hfs_btree *tree, u32 rsvd_nodes) u32 count; int res; + lockdep_assert_held(&tree->tree_lock); + if (rsvd_nodes <= 0) return 0; @@ -529,6 +531,8 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree) u8 *data, byte, m; int i, res; + lockdep_assert_held(&tree->tree_lock); + res = hfs_bmap_reserve(tree, 1); if (res) return ERR_PTR(res); @@ -607,6 +611,7 @@ void hfs_bmap_free(struct hfs_bnode *node) hfs_dbg("node %u\n", node->this); BUG_ON(!node->this); tree = node->tree; + lockdep_assert_held(&tree->tree_lock); nidx = node->this; node = hfs_bnode_find(tree, 0); if (IS_ERR(node)) diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c index cdf08393de44..e8f359d69328 100644 --- a/fs/hfsplus/inode.c +++ b/fs/hfsplus/inode.c @@ -720,20 +720,19 @@ int hfsplus_cat_write_inode(struct inode *inode) sizeof(struct hfsplus_cat_file)); } + res = hfs_btree_write(tree); + if (res) { + pr_err("b-tree write err: %d, ino %lu\n", + res, inode->i_ino); + goto out; + } + set_bit(HFSPLUS_I_CAT_DIRTY, &HFSPLUS_I(HFSPLUS_CAT_TREE_I(inode->i_sb))->flags); 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; } diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index 8bd1627ffc9c..44635f92ada9 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -153,7 +153,10 @@ static int hfsplus_system_write_inode(struct inode *inode) } hfsplus_inode_write_fork(inode, fork); if (tree) { + mutex_lock_nested(&tree->tree_lock, + hfsplus_btree_lock_class(tree)); int err = hfs_btree_write(tree); + mutex_unlock(&tree->tree_lock); if (err) { pr_err("b-tree write err: %d, ino %lu\n", From cd3901f4c0348da84f33b6b6e3e8e9aa7e441d01 Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Fri, 3 Apr 2026 16:05:53 -0700 Subject: [PATCH 12/15] hfsplus: fix error processing issue in hfs_bmap_free() Currently, we check only -EINVAL error code in hfs_bmap_free() after calling the hfs_bmap_clear_bit(). It means that other error codes will be silently ignored. This patch adds the checking of all other error codes. cc: Shardul Bankar cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20260403230556.614171-3-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/btree.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c index f1ed7c336593..80aa816ae1c0 100644 --- a/fs/hfsplus/btree.c +++ b/fs/hfsplus/btree.c @@ -146,8 +146,9 @@ struct hfs_bmap_ctx { * Returns the struct page pointer, or an ERR_PTR on failure. * Note: The caller is responsible for mapping/unmapping the returned page. */ -static struct page *hfs_bmap_get_map_page(struct hfs_bnode *node, struct hfs_bmap_ctx *ctx, - u32 byte_offset) +static struct page *hfs_bmap_get_map_page(struct hfs_bnode *node, + struct hfs_bmap_ctx *ctx, + u32 byte_offset) { u16 rec_idx, off16; unsigned int page_off; @@ -647,9 +648,12 @@ void hfs_bmap_free(struct hfs_bnode *node) res = hfs_bmap_clear_bit(node, nidx); if (res == -EINVAL) { - pr_crit("trying to free free bnode %u(%d)\n", - node->this, node->type); - } else if (!res) { + pr_crit("trying to free the freed bnode %u(%d)\n", + nidx, node->type); + } else if (res) { + pr_crit("fail to free bnode %u(%d)\n", + nidx, node->type); + } else { tree->free_nodes++; mark_inode_dirty(tree->inode); } From 63584d76765bb3e212f70c4c3951ea785fabef1b Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Fri, 3 Apr 2026 16:05:54 -0700 Subject: [PATCH 13/15] hfsplus: fix logic of alloc/free b-tree node The hfs_bmap_alloc() and hfs_bmap_free() modify the b-tree's counters and nodes' bitmap of b-tree. However, hfs_btree_write() synchronizes the state of in-core b-tree's counters and node's bitmap with b-tree's descriptor in header node. Postponing this synchronization could result in inconsistent state of file system volume. This patch adds calling of hfs_btree_write() in hfs_bmap_alloc() and hfs_bmap_free() methods. cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20260403230556.614171-4-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/btree.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c index 80aa816ae1c0..761c74ccd653 100644 --- a/fs/hfsplus/btree.c +++ b/fs/hfsplus/btree.c @@ -564,6 +564,7 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree) set_page_dirty(page); kunmap_local(data); tree->free_nodes--; + hfs_btree_write(tree); mark_inode_dirty(tree->inode); hfs_bnode_put(node); return hfs_bnode_create(tree, @@ -585,6 +586,7 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree) if (!nidx) { hfs_dbg("create new bmap node\n"); next_node = hfs_bmap_new_bmap(node, idx); + hfs_btree_write(tree); } else next_node = hfs_bnode_find(tree, nidx); hfs_bnode_put(node); @@ -655,6 +657,7 @@ void hfs_bmap_free(struct hfs_bnode *node) nidx, node->type); } else { tree->free_nodes++; + hfs_btree_write(tree); mark_inode_dirty(tree->inode); } From 732af3aa6337fd56025c0548a9e54d6231052144 Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Fri, 3 Apr 2026 16:05:55 -0700 Subject: [PATCH 14/15] hfsplus: rework logic of map nodes creation in xattr b-tree In hfsplus_init_header_node() when node_count > 63488 (header bitmap capacity), the code calculates map_nodes, subtracts them from free_nodes, and marks their positions used in the bitmap. However, it doesn't write the actual map node structure (type, record offsets, bitmap) for those physical positions, only node 0 is written. This patch reworks hfsplus_create_attributes_file() logic by introducing a specialized method of hfsplus_init_map_node() and writing the allocated map b-tree's nodes by means of hfsplus_write_attributes_file_node() method. cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20260403230556.614171-5-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/xattr.c | 127 ++++++++++++++++++++++++++++++------- include/linux/hfs_common.h | 2 + 2 files changed, 106 insertions(+), 23 deletions(-) diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c index 89e2e7e46e96..75cb74466738 100644 --- a/fs/hfsplus/xattr.c +++ b/fs/hfsplus/xattr.c @@ -50,7 +50,7 @@ static bool is_known_namespace(const char *name) return true; } -static void hfsplus_init_header_node(struct inode *attr_file, +static u32 hfsplus_init_header_node(struct inode *attr_file, u32 clump_size, char *buf, u16 node_size) { @@ -59,6 +59,7 @@ static void hfsplus_init_header_node(struct inode *attr_file, u16 offset; __be16 *rec_offsets; u32 hdr_node_map_rec_bits; + u32 map_nodes = 0; char *bmp; u32 used_nodes; u32 used_bmp_bytes; @@ -93,7 +94,6 @@ static void hfsplus_init_header_node(struct inode *attr_file, hdr_node_map_rec_bits = 8 * (node_size - offset - (4 * sizeof(u16))); if (be32_to_cpu(head->node_count) > hdr_node_map_rec_bits) { u32 map_node_bits; - u32 map_nodes; desc->next = cpu_to_be32(be32_to_cpu(head->leaf_tail) + 1); map_node_bits = 8 * (node_size - sizeof(struct hfs_bnode_desc) - @@ -116,21 +116,100 @@ static void hfsplus_init_header_node(struct inode *attr_file, *bmp = ~(0xFF >> used_nodes); offset += hdr_node_map_rec_bits / 8; *--rec_offsets = cpu_to_be16(offset); + + return map_nodes; +} + +/* + * Initialize a map node buffer. Map nodes have a single bitmap record. + */ +static void hfsplus_init_map_node(u8 *buf, u16 node_size, u32 next_node) +{ + struct hfs_bnode_desc *desc; + __be16 *rec_offsets; + size_t rec_size = sizeof(__be16); + u16 offset; + + memset(buf, 0, node_size); + + desc = (struct hfs_bnode_desc *)buf; + desc->type = HFS_NODE_MAP; + desc->num_recs = cpu_to_be16(1); + desc->next = cpu_to_be32(next_node); + + /* + * A map node consists of the node descriptor and a single + * map record. The map record is a continuation of the map + * record contained in the header node. The size of the map + * record is the size of the node, minus the size of + * the node descriptor (14 bytes), minus the size of two offsets + * (4 bytes), minus two bytes of free space. That is, the size of + * the map record is the size of the node minus 20 bytes; + * this keeps the length of the map record an even multiple of 4 bytes. + * The start of the map record is not aligned to a 4-byte boundary: + * it starts immediately after the node descriptor + * (at an offset of 14 bytes). + * + * Two record offsets stored at the end of the node: + * record[1] = start of record 0 -> sizeof(hfs_bnode_desc) + * record[2] = start of free space + */ + rec_offsets = (__be16 *)(buf + node_size); + + /* record #1 */ + offset = sizeof(struct hfs_bnode_desc); + *--rec_offsets = cpu_to_be16(offset); + + /* record #2 */ + offset = node_size; + offset -= (u16)HFSPLUS_BTREE_MAP_NODE_RECS_COUNT * rec_size; + offset -= HFSPLUS_BTREE_MAP_NODE_RESERVED_BYTES; + *--rec_offsets = cpu_to_be16(offset); +} + +static inline +int hfsplus_write_attributes_file_node(struct inode *attr_file, char *buf, + u16 node_size, int *index) +{ + struct address_space *mapping; + struct page *page; + void *kaddr; + u32 written = 0; + + mapping = attr_file->i_mapping; + + for (; written < node_size; (*index)++, written += PAGE_SIZE) { + page = read_mapping_page(mapping, *index, NULL); + if (IS_ERR(page)) + return PTR_ERR(page); + + kaddr = kmap_local_page(page); + memcpy(kaddr, buf + written, + min_t(size_t, PAGE_SIZE, node_size - written)); + kunmap_local(kaddr); + + set_page_dirty(page); + put_page(page); + } + + return 0; } static int hfsplus_create_attributes_file(struct super_block *sb) { - int err = 0; struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb); struct inode *attr_file; struct hfsplus_inode_info *hip; + struct hfs_bnode_desc *desc; u32 clump_size; u16 node_size = HFSPLUS_ATTR_TREE_NODE_SIZE; + u32 next_node; + u32 map_node_idx; + u32 map_nodes; char *buf; - int index, written; - struct address_space *mapping; - struct page *page; + int index; int old_state = HFSPLUS_EMPTY_ATTR_TREE; + int err = 0; hfs_dbg("ino %d\n", HFSPLUS_ATTR_CNID); @@ -195,7 +274,7 @@ static int hfsplus_create_attributes_file(struct super_block *sb) } while (hip->alloc_blocks < hip->clump_blocks) { - err = hfsplus_file_extend(attr_file, false); + err = hfsplus_file_extend(attr_file, true); if (unlikely(err)) { pr_err("failed to extend attributes file\n"); goto end_attr_file_creation; @@ -212,28 +291,30 @@ static int hfsplus_create_attributes_file(struct super_block *sb) goto end_attr_file_creation; } - hfsplus_init_header_node(attr_file, clump_size, buf, node_size); + map_nodes = hfsplus_init_header_node(attr_file, clump_size, buf, node_size); - mapping = attr_file->i_mapping; + desc = (struct hfs_bnode_desc *)buf; + next_node = be32_to_cpu(desc->next); index = 0; - written = 0; - for (; written < node_size; index++, written += PAGE_SIZE) { - void *kaddr; - page = read_mapping_page(mapping, index, NULL); - if (IS_ERR(page)) { - err = PTR_ERR(page); + err = hfsplus_write_attributes_file_node(attr_file, buf, + node_size, &index); + if (unlikely(err)) + goto failed_header_node_init; + + for (map_node_idx = 0; map_node_idx < map_nodes; map_node_idx++) { + if (next_node >= map_nodes) + next_node = 0; + + hfsplus_init_map_node(buf, node_size, next_node); + + err = hfsplus_write_attributes_file_node(attr_file, buf, + node_size, &index); + if (unlikely(err)) goto failed_header_node_init; - } - kaddr = kmap_atomic(page); - memcpy(kaddr, buf + written, - min_t(size_t, PAGE_SIZE, node_size - written)); - kunmap_atomic(kaddr); - - set_page_dirty(page); - put_page(page); + next_node++; } hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(sb), HFSPLUS_I_ATTR_DIRTY); diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h index 9e71b9a03b60..07dfc39630ab 100644 --- a/include/linux/hfs_common.h +++ b/include/linux/hfs_common.h @@ -518,6 +518,8 @@ struct hfs_btree_header_rec { #define HFSPLUS_BTREE_HDR_MAP_REC_INDEX 2 /* Map (bitmap) record in Header node */ #define HFSPLUS_BTREE_MAP_NODE_REC_INDEX 0 /* Map record in Map Node */ #define HFSPLUS_BTREE_HDR_USER_BYTES 128 +#define HFSPLUS_BTREE_MAP_NODE_RECS_COUNT 2 +#define HFSPLUS_BTREE_MAP_NODE_RESERVED_BYTES 2 /* btree key type */ #define HFSPLUS_KEY_CASEFOLDING 0xCF /* case-insensitive */ From c1307d18caa819ddc28459d858eb38fdd6c3f8a0 Mon Sep 17 00:00:00 2001 From: Viacheslav Dubeyko Date: Fri, 3 Apr 2026 16:05:56 -0700 Subject: [PATCH 15/15] hfsplus: fix generic/642 failure The xfstests' test-case generic/642 finishes with corrupted HFS+ volume: sudo ./check generic/642 [sudo] password for slavad: FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 7.0.0-rc1+ #26 SMP PREEMPT_DYNAMIC Mon Mar 23 17:24:32 PDT 2026 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/642 6s ... _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent (see xfstests-dev/results//generic/642.full for details) Ran: generic/642 Failures: generic/642 Failed 1 of 1 tests sudo fsck.hfs -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. ** Checking catalog hierarchy. ** Checking extended attributes file. invalid free nodes - calculated 1637 header 1260 Invalid B-tree header Invalid map node (8, 0) ** Checking volume bitmap. ** Checking volume information. Verify Status: VIStat = 0x0000, ABTStat = 0xc000 EBTStat = 0x0000 CBTStat = 0x0000 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. ** Checking catalog hierarchy. ** Checking extended attributes file. ** Checking volume bitmap. ** Checking volume information. ** The volume untitled was repaired successfully. The fsck tool detected that Extended Attributes b-tree is corrupted. Namely, the free nodes number is incorrect and map node bitmap has inconsistent state. Analysis has shown that during b-tree closing there are still some lost b-tree's nodes in the hash out of b-tree structure. But this orphaned b-tree nodes are still accounted as used in map node bitmap: tree_cnid 8, nidx 0, node_count 1408, free_nodes 1403 tree_cnid 8, nidx 1, node_count 1408, free_nodes 1403 tree_cnid 8, nidx 3, node_count 1408, free_nodes 1403 tree_cnid 8, nidx 54, node_count 1408, free_nodes 1403 tree_cnid 8, nidx 67, node_count 1408, free_nodes 1403 tree_cnid 8, nidx 0, prev 0, next 0, parent 0, num_recs 3, type 0x1, height 0 tree_cnid 8, nidx 1, prev 0, next 0, parent 3, num_recs 1, type 0xff, height 1 tree_cnid 8, nidx 3, prev 0, next 0, parent 0, num_recs 1, type 0x0, height 2 tree_cnid 8, nidx 54, prev 29, next 46, parent 3, num_recs 0, type 0xff, height 1 tree_cnid 8, nidx 67, prev 8, next 14, parent 3, num_recs 0, type 0xff, height 1 This issue happens in hfs_bnode_split() logic during detection the possibility of moving half ot the records out of the node. The hfs_bnode_split() contains a loop that implements a roughly 50/50 split of the B-tree node's records by scanning the offset table to find where the data crosses the node's midpoint. If this logic detects the incapability of spliting the node, then it simply calls hfs_bnode_put() for newly created node. However, node is not set as HFS_BNODE_DELETED and real deletion of node doesn't happen. As a result, the empty node becomes orphaned but it is still accounted as used. Finally, fsck tool detects this inconsistency of HFS+ volume. This patch adds call of hfs_bnode_unlink() before hfs_bnode_put() for the case if new node cannot be used for spliting the existing node. sudo ./check generic/642 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 7.0.0-rc1+ #26 SMP PREEMPT_DYNAMIC Fri Apr 3 12:39:13 PDT 2026 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/642 40s ... 39s Ran: generic/642 Passed all 1 tests Closes: https://github.com/hfs-linux-kernel/hfs-linux-kernel/issues/242 cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Signed-off-by: Viacheslav Dubeyko Link: https://lore.kernel.org/r/20260403230556.614171-6-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko --- fs/hfsplus/brec.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/fs/hfsplus/brec.c b/fs/hfsplus/brec.c index 6796c1a80e99..e3df89284079 100644 --- a/fs/hfsplus/brec.c +++ b/fs/hfsplus/brec.c @@ -239,6 +239,9 @@ static struct hfs_bnode *hfs_bnode_split(struct hfs_find_data *fd) struct hfs_bnode_desc node_desc; int num_recs, new_rec_off, new_off, old_rec_off; int data_start, data_end, size; + size_t rec_off_tbl_size; + size_t node_desc_size = sizeof(struct hfs_bnode_desc); + size_t rec_size = sizeof(__be16); tree = fd->tree; node = fd->bnode; @@ -265,18 +268,22 @@ static struct hfs_bnode *hfs_bnode_split(struct hfs_find_data *fd) return next_node; } - size = tree->node_size / 2 - node->num_recs * 2 - 14; - old_rec_off = tree->node_size - 4; + rec_off_tbl_size = node->num_recs * rec_size; + size = tree->node_size / 2; + size -= node_desc_size; + size -= rec_off_tbl_size; + old_rec_off = tree->node_size - (2 * rec_size); + num_recs = 1; for (;;) { data_start = hfs_bnode_read_u16(node, old_rec_off); if (data_start > size) break; - old_rec_off -= 2; + old_rec_off -= rec_size; if (++num_recs < node->num_recs) continue; - /* panic? */ hfs_bnode_put(node); + hfs_bnode_unlink(new_node); hfs_bnode_put(new_node); if (next_node) hfs_bnode_put(next_node); @@ -287,7 +294,7 @@ static struct hfs_bnode *hfs_bnode_split(struct hfs_find_data *fd) /* new record is in the lower half, * so leave some more space there */ - old_rec_off += 2; + old_rec_off += rec_size; num_recs--; data_start = hfs_bnode_read_u16(node, old_rec_off); } else { @@ -295,27 +302,28 @@ static struct hfs_bnode *hfs_bnode_split(struct hfs_find_data *fd) hfs_bnode_get(new_node); fd->bnode = new_node; fd->record -= num_recs; - fd->keyoffset -= data_start - 14; - fd->entryoffset -= data_start - 14; + fd->keyoffset -= data_start - node_desc_size; + fd->entryoffset -= data_start - node_desc_size; } new_node->num_recs = node->num_recs - num_recs; node->num_recs = num_recs; - new_rec_off = tree->node_size - 2; - new_off = 14; + new_rec_off = tree->node_size - rec_size; + new_off = node_desc_size; size = data_start - new_off; num_recs = new_node->num_recs; data_end = data_start; while (num_recs) { hfs_bnode_write_u16(new_node, new_rec_off, new_off); - old_rec_off -= 2; - new_rec_off -= 2; + old_rec_off -= rec_size; + new_rec_off -= rec_size; data_end = hfs_bnode_read_u16(node, old_rec_off); new_off = data_end - size; num_recs--; } hfs_bnode_write_u16(new_node, new_rec_off, new_off); - hfs_bnode_copy(new_node, 14, node, data_start, data_end - data_start); + hfs_bnode_copy(new_node, node_desc_size, + node, data_start, data_end - data_start); /* update new bnode header */ node_desc.next = cpu_to_be32(new_node->next);