tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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 <slava@dubeyko.com>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
Link: https://lore.kernel.org/r/20260220220152.152721-1-slava@dubeyko.com
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>

Viacheslav Dubeyko ee8422d0 6de23f81

+65 -9
+3
fs/hfsplus/attributes.c
··· 241 241 return err; 242 242 } 243 243 244 + hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(sb), HFSPLUS_I_ATTR_DIRTY); 244 245 hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ATTR_DIRTY); 245 246 246 247 return 0; ··· 327 326 if (err) 328 327 return err; 329 328 329 + hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(inode->i_sb), 330 + HFSPLUS_I_ATTR_DIRTY); 330 331 hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ATTR_DIRTY); 331 332 return err; 332 333 }
+3
fs/hfsplus/catalog.c
··· 313 313 if (S_ISDIR(inode->i_mode)) 314 314 hfsplus_subfolders_inc(dir); 315 315 inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir)); 316 + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(sb), HFSPLUS_I_CAT_DIRTY); 316 317 hfsplus_mark_inode_dirty(dir, HFSPLUS_I_CAT_DIRTY); 317 318 318 319 hfs_find_exit(&fd); ··· 419 418 if (type == HFSPLUS_FOLDER) 420 419 hfsplus_subfolders_dec(dir); 421 420 inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir)); 421 + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(sb), HFSPLUS_I_CAT_DIRTY); 422 422 hfsplus_mark_inode_dirty(dir, HFSPLUS_I_CAT_DIRTY); 423 423 424 424 if (type == HFSPLUS_FILE || type == HFSPLUS_FOLDER) { ··· 542 540 } 543 541 err = hfs_brec_insert(&dst_fd, &entry, entry_size); 544 542 543 + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(sb), HFSPLUS_I_CAT_DIRTY); 545 544 hfsplus_mark_inode_dirty(dst_dir, HFSPLUS_I_CAT_DIRTY); 546 545 hfsplus_mark_inode_dirty(src_dir, HFSPLUS_I_CAT_DIRTY); 547 546 out:
+6
fs/hfsplus/dir.c
··· 478 478 if (!inode) 479 479 goto out; 480 480 481 + hfs_dbg("dir->i_ino %lu, inode->i_ino %lu\n", 482 + dir->i_ino, inode->i_ino); 483 + 481 484 res = page_symlink(inode, symname, strlen(symname) + 1); 482 485 if (res) 483 486 goto out_err; ··· 528 525 inode = hfsplus_new_inode(dir->i_sb, dir, mode); 529 526 if (!inode) 530 527 goto out; 528 + 529 + hfs_dbg("dir->i_ino %lu, inode->i_ino %lu\n", 530 + dir->i_ino, inode->i_ino); 531 531 532 532 if (S_ISBLK(mode) || S_ISCHR(mode) || S_ISFIFO(mode) || S_ISSOCK(mode)) 533 533 init_special_inode(inode, mode, rdev);
+7
fs/hfsplus/extents.c
··· 121 121 * redirty the inode. Instead the callers have to be careful 122 122 * to explicily mark the inode dirty, too. 123 123 */ 124 + set_bit(HFSPLUS_I_EXT_DIRTY, 125 + &HFSPLUS_I(HFSPLUS_EXT_TREE_I(inode->i_sb))->flags); 124 126 set_bit(HFSPLUS_I_EXT_DIRTY, &hip->flags); 125 127 126 128 return 0; ··· 515 513 if (!res) { 516 514 hip->alloc_blocks += len; 517 515 mutex_unlock(&hip->extents_lock); 516 + hfsplus_mark_inode_dirty(HFSPLUS_SB(sb)->alloc_file, 517 + HFSPLUS_I_ALLOC_DIRTY); 518 518 hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ALLOC_DIRTY); 519 519 return 0; 520 520 } ··· 586 582 /* XXX: We lack error handling of hfsplus_file_truncate() */ 587 583 return; 588 584 } 585 + 589 586 while (1) { 590 587 if (alloc_cnt == hip->first_blocks) { 591 588 mutex_unlock(&fd.tree->tree_lock); ··· 628 623 hip->fs_blocks = (inode->i_size + sb->s_blocksize - 1) >> 629 624 sb->s_blocksize_bits; 630 625 inode_set_bytes(inode, hip->fs_blocks << sb->s_blocksize_bits); 626 + hfsplus_mark_inode_dirty(HFSPLUS_SB(sb)->alloc_file, 627 + HFSPLUS_I_ALLOC_DIRTY); 631 628 hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ALLOC_DIRTY); 632 629 }
+7
fs/hfsplus/hfsplus_fs.h
··· 238 238 return container_of(inode, struct hfsplus_inode_info, vfs_inode); 239 239 } 240 240 241 + #define HFSPLUS_CAT_TREE_I(sb) \ 242 + HFSPLUS_SB(sb)->cat_tree->inode 243 + #define HFSPLUS_EXT_TREE_I(sb) \ 244 + HFSPLUS_SB(sb)->ext_tree->inode 245 + #define HFSPLUS_ATTR_TREE_I(sb) \ 246 + HFSPLUS_SB(sb)->attr_tree->inode 247 + 241 248 /* 242 249 * Mark an inode dirty, and also mark the btree in which the 243 250 * specific type of metadata is stored.
+20 -7
fs/hfsplus/inode.c
··· 324 324 { 325 325 struct inode *inode = file->f_mapping->host; 326 326 struct hfsplus_inode_info *hip = HFSPLUS_I(inode); 327 + struct super_block *sb = inode->i_sb; 327 328 struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb); 328 329 struct hfsplus_vh *vhdr = sbi->s_vhdr; 329 330 int error = 0, error2; ··· 345 344 /* 346 345 * And explicitly write out the btrees. 347 346 */ 348 - if (test_and_clear_bit(HFSPLUS_I_CAT_DIRTY, &hip->flags)) 347 + if (test_and_clear_bit(HFSPLUS_I_CAT_DIRTY, 348 + &HFSPLUS_I(HFSPLUS_CAT_TREE_I(sb))->flags)) { 349 + clear_bit(HFSPLUS_I_CAT_DIRTY, &hip->flags); 349 350 error = filemap_write_and_wait(sbi->cat_tree->inode->i_mapping); 351 + } 350 352 351 - if (test_and_clear_bit(HFSPLUS_I_EXT_DIRTY, &hip->flags)) { 353 + if (test_and_clear_bit(HFSPLUS_I_EXT_DIRTY, 354 + &HFSPLUS_I(HFSPLUS_EXT_TREE_I(sb))->flags)) { 355 + clear_bit(HFSPLUS_I_EXT_DIRTY, &hip->flags); 352 356 error2 = 353 357 filemap_write_and_wait(sbi->ext_tree->inode->i_mapping); 354 358 if (!error) 355 359 error = error2; 356 360 } 357 361 358 - if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags)) { 359 - if (sbi->attr_tree) { 362 + if (sbi->attr_tree) { 363 + if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY, 364 + &HFSPLUS_I(HFSPLUS_ATTR_TREE_I(sb))->flags)) { 365 + clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags); 360 366 error2 = 361 367 filemap_write_and_wait( 362 368 sbi->attr_tree->inode->i_mapping); 363 369 if (!error) 364 370 error = error2; 365 - } else { 366 - pr_err("sync non-existent attributes tree\n"); 367 371 } 372 + } else { 373 + if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags)) 374 + pr_err("sync non-existent attributes tree\n"); 368 375 } 369 376 370 - if (test_and_clear_bit(HFSPLUS_I_ALLOC_DIRTY, &hip->flags)) { 377 + if (test_and_clear_bit(HFSPLUS_I_ALLOC_DIRTY, 378 + &HFSPLUS_I(sbi->alloc_file)->flags)) { 379 + clear_bit(HFSPLUS_I_ALLOC_DIRTY, &hip->flags); 371 380 error2 = filemap_write_and_wait(sbi->alloc_file->i_mapping); 372 381 if (!error) 373 382 error = error2; ··· 720 709 sizeof(struct hfsplus_cat_file)); 721 710 } 722 711 712 + set_bit(HFSPLUS_I_CAT_DIRTY, 713 + &HFSPLUS_I(HFSPLUS_CAT_TREE_I(inode->i_sb))->flags); 723 714 set_bit(HFSPLUS_I_CAT_DIRTY, &HFSPLUS_I(inode)->flags); 724 715 out: 725 716 hfs_find_exit(&fd);
+2
fs/hfsplus/super.c
··· 625 625 } 626 626 627 627 mutex_unlock(&sbi->vh_mutex); 628 + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(sb), 629 + HFSPLUS_I_CAT_DIRTY); 628 630 hfsplus_mark_inode_dirty(sbi->hidden_dir, 629 631 HFSPLUS_I_CAT_DIRTY); 630 632 }
+17 -2
fs/hfsplus/xattr.c
··· 236 236 put_page(page); 237 237 } 238 238 239 + hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(sb), HFSPLUS_I_ATTR_DIRTY); 239 240 hfsplus_mark_inode_dirty(attr_file, HFSPLUS_I_ATTR_DIRTY); 240 241 241 242 sbi->attr_tree = hfs_btree_open(sb, HFSPLUS_ATTR_CNID); ··· 315 314 hfs_bnode_write(cat_fd.bnode, &entry, 316 315 cat_fd.entryoffset, 317 316 sizeof(struct hfsplus_cat_folder)); 318 - hfsplus_mark_inode_dirty(inode, 317 + hfsplus_mark_inode_dirty( 318 + HFSPLUS_CAT_TREE_I(inode->i_sb), 319 319 HFSPLUS_I_CAT_DIRTY); 320 + hfsplus_mark_inode_dirty(inode, 321 + HFSPLUS_I_CAT_DIRTY); 320 322 } else { 321 323 err = -ERANGE; 322 324 goto end_setxattr; ··· 331 327 hfs_bnode_write(cat_fd.bnode, &entry, 332 328 cat_fd.entryoffset, 333 329 sizeof(struct hfsplus_cat_file)); 334 - hfsplus_mark_inode_dirty(inode, 330 + hfsplus_mark_inode_dirty( 331 + HFSPLUS_CAT_TREE_I(inode->i_sb), 335 332 HFSPLUS_I_CAT_DIRTY); 333 + hfsplus_mark_inode_dirty(inode, 334 + HFSPLUS_I_CAT_DIRTY); 336 335 } else { 337 336 err = -ERANGE; 338 337 goto end_setxattr; ··· 388 381 hfs_bnode_write_u16(cat_fd.bnode, cat_fd.entryoffset + 389 382 offsetof(struct hfsplus_cat_folder, flags), 390 383 cat_entry_flags); 384 + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(inode->i_sb), 385 + HFSPLUS_I_CAT_DIRTY); 391 386 hfsplus_mark_inode_dirty(inode, HFSPLUS_I_CAT_DIRTY); 392 387 } else if (cat_entry_type == HFSPLUS_FILE) { 393 388 cat_entry_flags = hfs_bnode_read_u16(cat_fd.bnode, ··· 401 392 hfs_bnode_write_u16(cat_fd.bnode, cat_fd.entryoffset + 402 393 offsetof(struct hfsplus_cat_file, flags), 403 394 cat_entry_flags); 395 + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(inode->i_sb), 396 + HFSPLUS_I_CAT_DIRTY); 404 397 hfsplus_mark_inode_dirty(inode, HFSPLUS_I_CAT_DIRTY); 405 398 } else { 406 399 pr_err("invalid catalog entry type\n"); ··· 873 862 hfs_bnode_write_u16(cat_fd.bnode, cat_fd.entryoffset + 874 863 offsetof(struct hfsplus_cat_folder, flags), 875 864 flags); 865 + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(inode->i_sb), 866 + HFSPLUS_I_CAT_DIRTY); 876 867 hfsplus_mark_inode_dirty(inode, HFSPLUS_I_CAT_DIRTY); 877 868 } else if (cat_entry_type == HFSPLUS_FILE) { 878 869 flags = hfs_bnode_read_u16(cat_fd.bnode, cat_fd.entryoffset + ··· 886 873 hfs_bnode_write_u16(cat_fd.bnode, cat_fd.entryoffset + 887 874 offsetof(struct hfsplus_cat_file, flags), 888 875 flags); 876 + hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(inode->i_sb), 877 + HFSPLUS_I_CAT_DIRTY); 889 878 hfsplus_mark_inode_dirty(inode, HFSPLUS_I_CAT_DIRTY); 890 879 } else { 891 880 pr_err("invalid catalog entry type\n");