Skip to content

Commit bea4429

Browse files
dubeykodubeyko
committed
hfsplus: fix volume corruption issue for generic/480
The xfstests' test-case generic/480 leaves HFS+ volume in corrupted state: sudo ./check generic/480 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.17.0-rc1+ #4 SMP PREEMPT_DYNAMIC Wed Oct 1 15:02:44 PDT 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/480 _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent (see XFSTESTS-2/xfstests-dev/results//generic/480.full for details) Ran: generic/480 Failures: generic/480 Failed 1 of 1 tests sudo fsck.hfsplus -d /dev/loop51 ** /dev/loop51 Using cacheBlockSize=32K cacheTotalBlock=1024 cacheSize=32768K. Executing fsck_hfs (version 540.1-Linux). ** Checking non-journaled HFS Plus Volume. The volume name is untitled ** Checking extents overflow file. ** Checking catalog file. ** Checking multi-linked files. CheckHardLinks: found 1 pre-Leopard file inodes. Incorrect number of file hard links ** Checking catalog hierarchy. ** Checking extended attributes file. ** Checking volume bitmap. ** Checking volume information. invalid VHB nextCatalogID Volume header needs minor repair (2, 0) Verify Status: VIStat = 0x8000, ABTStat = 0x0000 EBTStat = 0x0000 CBTStat = 0x0000 CatStat = 0x00000002 ** Repairing volume. Incorrect flags for file hard link (id = 19) (It should be 0x22 instead of 0x2) Incorrect flags for file inode (id = 18) (It should be 0x22 instead of 0x2) first link ID=0 is < 16 for fileinode=18 Error getting first link ID for inode = 18 (result=2) Invalid first link in hard link chain (id = 18) (It should be 19 instead of 0) Indirect node 18 needs link count adjustment (It should be 1 instead of 2) ** Rechecking volume. ** Checking non-journaled HFS Plus Volume. The volume name is untitled ** Checking extents overflow file. ** Checking catalog file. ** Checking multi-linked files. ** Checking catalog hierarchy. ** Checking extended attributes file. ** Checking volume bitmap. ** Checking volume information. ** The volume untitled was repaired successfully. The generic/480 test executes such steps on final phase: "Now remove of the links of our file and create a new file with the same name and in the same parent directory, and finally fsync this new file." unlink $SCRATCH_MNT/testdir/bar touch $SCRATCH_MNT/testdir/bar $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir/bar "Simulate a power failure and mount the filesystem to check that replaying the fsync log/journal succeeds, that is the mount operation does not fail." _flakey_drop_and_remount The key issue in HFS+ logic is that hfsplus_link(), hfsplus_unlink(), hfsplus_rmdir(), hfsplus_symlink(), and hfsplus_mknod() methods don't call hfsplus_cat_write_inode() for the case of modified inode objects. As a result, even if hfsplus_file_fsync() is trying to flush the dirty Catalog File, but because of not calling hfsplus_cat_write_inode() not all modified inodes save the new state into Catalog File's records. Finally, simulation of power failure results in inconsistent state of Catalog File and FSCK tool reports about volume corruption. This patch adds calling of hfsplus_cat_write_inode() method for modified inodes in hfsplus_link(), hfsplus_unlink(), hfsplus_rmdir(), hfsplus_symlink(), and hfsplus_mknod() methods. Also, it adds debug output in several methods. sudo ./check generic/480 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.18.0-rc1+ #18 SMP PREEMPT_DYNAMIC Thu Dec 4 12:24:45 PST 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/480 16s ... 16s Ran: generic/480 Passed all 1 tests Signed-off-by: Viacheslav Dubeyko <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/20251205000054.3670326-1-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
1 parent 126fb0c commit bea4429

2 files changed

Lines changed: 50 additions & 1 deletion

File tree

‎fs/hfsplus/dir.c‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,9 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir,
313313
if (!S_ISREG(inode->i_mode))
314314
return -EPERM;
315315

316+
hfs_dbg("src_dir->i_ino %lu, dst_dir->i_ino %lu, inode->i_ino %lu\n",
317+
src_dir->i_ino, dst_dir->i_ino, inode->i_ino);
318+
316319
mutex_lock(&sbi->vh_mutex);
317320
if (inode->i_ino == (u32)(unsigned long)src_dentry->d_fsdata) {
318321
for (;;) {
@@ -332,7 +335,7 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir,
332335
cnid = sbi->next_cnid++;
333336
src_dentry->d_fsdata = (void *)(unsigned long)cnid;
334337
res = hfsplus_create_cat(cnid, src_dir,
335-
&src_dentry->d_name, inode);
338+
&src_dentry->d_name, inode);
336339
if (res)
337340
/* panic? */
338341
goto out;
@@ -350,6 +353,21 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir,
350353
mark_inode_dirty(inode);
351354
sbi->file_count++;
352355
hfsplus_mark_mdb_dirty(dst_dir->i_sb);
356+
357+
res = hfsplus_cat_write_inode(src_dir);
358+
if (res)
359+
goto out;
360+
361+
res = hfsplus_cat_write_inode(dst_dir);
362+
if (res)
363+
goto out;
364+
365+
res = hfsplus_cat_write_inode(sbi->hidden_dir);
366+
if (res)
367+
goto out;
368+
369+
res = hfsplus_cat_write_inode(inode);
370+
353371
out:
354372
mutex_unlock(&sbi->vh_mutex);
355373
return res;
@@ -367,6 +385,9 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry)
367385
if (HFSPLUS_IS_RSRC(inode))
368386
return -EPERM;
369387

388+
hfs_dbg("dir->i_ino %lu, inode->i_ino %lu\n",
389+
dir->i_ino, inode->i_ino);
390+
370391
mutex_lock(&sbi->vh_mutex);
371392
cnid = (u32)(unsigned long)dentry->d_fsdata;
372393
if (inode->i_ino == cnid &&
@@ -408,6 +429,15 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry)
408429
inode_set_ctime_current(inode);
409430
mark_inode_dirty(inode);
410431
out:
432+
if (!res) {
433+
res = hfsplus_cat_write_inode(dir);
434+
if (!res) {
435+
res = hfsplus_cat_write_inode(sbi->hidden_dir);
436+
if (!res)
437+
res = hfsplus_cat_write_inode(inode);
438+
}
439+
}
440+
411441
mutex_unlock(&sbi->vh_mutex);
412442
return res;
413443
}
@@ -429,6 +459,8 @@ static int hfsplus_rmdir(struct inode *dir, struct dentry *dentry)
429459
inode_set_ctime_current(inode);
430460
hfsplus_delete_inode(inode);
431461
mark_inode_dirty(inode);
462+
463+
res = hfsplus_cat_write_inode(dir);
432464
out:
433465
mutex_unlock(&sbi->vh_mutex);
434466
return res;
@@ -465,6 +497,12 @@ static int hfsplus_symlink(struct mnt_idmap *idmap, struct inode *dir,
465497

466498
hfsplus_instantiate(dentry, inode, inode->i_ino);
467499
mark_inode_dirty(inode);
500+
501+
res = hfsplus_cat_write_inode(dir);
502+
if (res)
503+
goto out;
504+
505+
res = hfsplus_cat_write_inode(inode);
468506
goto out;
469507

470508
out_err:
@@ -506,6 +544,12 @@ static int hfsplus_mknod(struct mnt_idmap *idmap, struct inode *dir,
506544

507545
hfsplus_instantiate(dentry, inode, inode->i_ino);
508546
mark_inode_dirty(inode);
547+
548+
res = hfsplus_cat_write_inode(dir);
549+
if (res)
550+
goto out;
551+
552+
res = hfsplus_cat_write_inode(inode);
509553
goto out;
510554

511555
failed_mknod:

‎fs/hfsplus/inode.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,9 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
328328
struct hfsplus_vh *vhdr = sbi->s_vhdr;
329329
int error = 0, error2;
330330

331+
hfs_dbg("inode->i_ino %lu, start %llu, end %llu\n",
332+
inode->i_ino, start, end);
333+
331334
error = file_write_and_wait_range(file, start, end);
332335
if (error)
333336
return error;
@@ -616,6 +619,8 @@ int hfsplus_cat_write_inode(struct inode *inode)
616619
hfsplus_cat_entry entry;
617620
int res = 0;
618621

622+
hfs_dbg("inode->i_ino %lu\n", inode->i_ino);
623+
619624
if (HFSPLUS_IS_RSRC(inode))
620625
main_inode = HFSPLUS_I(inode)->rsrc_inode;
621626

0 commit comments

Comments
 (0)