Skip to content

Commit 3f04ee2

Browse files
committed
hfsplus: fix volume corruption issue for generic/101
The xfstests' test-case generic/101 leaves HFS+ volume in corrupted state: sudo ./check generic/101 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/101 _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent (see XFSTESTS-2/xfstests-dev/results//generic/101.full for details) Ran: generic/101 Failures: generic/101 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. ** Checking catalog hierarchy. ** Checking extended attributes file. ** Checking volume bitmap. ** Checking volume information. Invalid volume free block count (It should be 2614350 instead of 2614382) Verify Status: VIStat = 0x8000, ABTStat = 0x0000 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. This test executes such steps: "Test that if we truncate a file to a smaller size, then truncate it to its original size or a larger size, then fsyncing it and a power failure happens, the file will have the range [first_truncate_size, last_size[ with all bytes having a value of 0x00 if we read it the next time the filesystem is mounted.". HFS+ keeps volume's free block count in the superblock. However, hfsplus_file_fsync() doesn't store superblock's content. As a result, superblock contains not correct value of free blocks if a power failure happens. This patch adds functionality of saving superblock's content during hfsplus_file_fsync() call. sudo ./check generic/101 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.18.0-rc3+ #96 SMP PREEMPT_DYNAMIC Wed Nov 19 12:47:37 PST 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/101 32s ... 30s Ran: generic/101 Passed all 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. ** Checking catalog hierarchy. ** Checking extended attributes file. ** Checking volume bitmap. ** Checking volume information. ** The volume untitled appears to be OK. 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/20251119223219.1824434-1-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
1 parent 6f84ceb commit 3f04ee2

File tree

3 files changed

+65
-33
lines changed

3 files changed

+65
-33
lines changed

fs/hfsplus/hfsplus_fs.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,8 @@ int hfs_part_find(struct super_block *sb, sector_t *part_start,
477477
/* super.c */
478478
struct inode *hfsplus_iget(struct super_block *sb, unsigned long ino);
479479
void hfsplus_mark_mdb_dirty(struct super_block *sb);
480+
void hfsplus_prepare_volume_header_for_commit(struct hfsplus_vh *vhdr);
481+
int hfsplus_commit_superblock(struct super_block *sb);
480482

481483
/* tables.c */
482484
extern u16 hfsplus_case_fold_table[];

fs/hfsplus/inode.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
325325
struct inode *inode = file->f_mapping->host;
326326
struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
327327
struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb);
328+
struct hfsplus_vh *vhdr = sbi->s_vhdr;
328329
int error = 0, error2;
329330

330331
error = file_write_and_wait_range(file, start, end);
@@ -368,6 +369,14 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
368369
error = error2;
369370
}
370371

372+
mutex_lock(&sbi->vh_mutex);
373+
hfsplus_prepare_volume_header_for_commit(vhdr);
374+
mutex_unlock(&sbi->vh_mutex);
375+
376+
error2 = hfsplus_commit_superblock(inode->i_sb);
377+
if (!error)
378+
error = error2;
379+
371380
if (!test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags))
372381
blkdev_issue_flush(inode->i_sb->s_bdev);
373382

fs/hfsplus/super.c

Lines changed: 54 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -187,40 +187,15 @@ static void hfsplus_evict_inode(struct inode *inode)
187187
}
188188
}
189189

190-
static int hfsplus_sync_fs(struct super_block *sb, int wait)
190+
int hfsplus_commit_superblock(struct super_block *sb)
191191
{
192192
struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
193193
struct hfsplus_vh *vhdr = sbi->s_vhdr;
194194
int write_backup = 0;
195-
int error, error2;
196-
197-
if (!wait)
198-
return 0;
195+
int error = 0, error2;
199196

200197
hfs_dbg("starting...\n");
201198

202-
/*
203-
* Explicitly write out the special metadata inodes.
204-
*
205-
* While these special inodes are marked as hashed and written
206-
* out peridocically by the flusher threads we redirty them
207-
* during writeout of normal inodes, and thus the life lock
208-
* prevents us from getting the latest state to disk.
209-
*/
210-
error = filemap_write_and_wait(sbi->cat_tree->inode->i_mapping);
211-
error2 = filemap_write_and_wait(sbi->ext_tree->inode->i_mapping);
212-
if (!error)
213-
error = error2;
214-
if (sbi->attr_tree) {
215-
error2 =
216-
filemap_write_and_wait(sbi->attr_tree->inode->i_mapping);
217-
if (!error)
218-
error = error2;
219-
}
220-
error2 = filemap_write_and_wait(sbi->alloc_file->i_mapping);
221-
if (!error)
222-
error = error2;
223-
224199
mutex_lock(&sbi->vh_mutex);
225200
mutex_lock(&sbi->alloc_mutex);
226201
vhdr->free_blocks = cpu_to_be32(sbi->free_blocks);
@@ -249,11 +224,52 @@ static int hfsplus_sync_fs(struct super_block *sb, int wait)
249224
sbi->part_start + sbi->sect_count - 2,
250225
sbi->s_backup_vhdr_buf, NULL, REQ_OP_WRITE);
251226
if (!error)
252-
error2 = error;
227+
error = error2;
253228
out:
254229
mutex_unlock(&sbi->alloc_mutex);
255230
mutex_unlock(&sbi->vh_mutex);
256231

232+
hfs_dbg("finished: err %d\n", error);
233+
234+
return error;
235+
}
236+
237+
static int hfsplus_sync_fs(struct super_block *sb, int wait)
238+
{
239+
struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
240+
int error, error2;
241+
242+
if (!wait)
243+
return 0;
244+
245+
hfs_dbg("starting...\n");
246+
247+
/*
248+
* Explicitly write out the special metadata inodes.
249+
*
250+
* While these special inodes are marked as hashed and written
251+
* out peridocically by the flusher threads we redirty them
252+
* during writeout of normal inodes, and thus the life lock
253+
* prevents us from getting the latest state to disk.
254+
*/
255+
error = filemap_write_and_wait(sbi->cat_tree->inode->i_mapping);
256+
error2 = filemap_write_and_wait(sbi->ext_tree->inode->i_mapping);
257+
if (!error)
258+
error = error2;
259+
if (sbi->attr_tree) {
260+
error2 =
261+
filemap_write_and_wait(sbi->attr_tree->inode->i_mapping);
262+
if (!error)
263+
error = error2;
264+
}
265+
error2 = filemap_write_and_wait(sbi->alloc_file->i_mapping);
266+
if (!error)
267+
error = error2;
268+
269+
error2 = hfsplus_commit_superblock(sb);
270+
if (!error)
271+
error = error2;
272+
257273
if (!test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags))
258274
blkdev_issue_flush(sb->s_bdev);
259275

@@ -395,6 +411,15 @@ static const struct super_operations hfsplus_sops = {
395411
.show_options = hfsplus_show_options,
396412
};
397413

414+
void hfsplus_prepare_volume_header_for_commit(struct hfsplus_vh *vhdr)
415+
{
416+
vhdr->last_mount_vers = cpu_to_be32(HFSP_MOUNT_VERSION);
417+
vhdr->modify_date = hfsp_now2mt();
418+
be32_add_cpu(&vhdr->write_count, 1);
419+
vhdr->attributes &= cpu_to_be32(~HFSPLUS_VOL_UNMNT);
420+
vhdr->attributes |= cpu_to_be32(HFSPLUS_VOL_INCNSTNT);
421+
}
422+
398423
static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
399424
{
400425
struct hfsplus_vh *vhdr;
@@ -562,11 +587,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
562587
* H+LX == hfsplusutils, H+Lx == this driver, H+lx is unused
563588
* all three are registered with Apple for our use
564589
*/
565-
vhdr->last_mount_vers = cpu_to_be32(HFSP_MOUNT_VERSION);
566-
vhdr->modify_date = hfsp_now2mt();
567-
be32_add_cpu(&vhdr->write_count, 1);
568-
vhdr->attributes &= cpu_to_be32(~HFSPLUS_VOL_UNMNT);
569-
vhdr->attributes |= cpu_to_be32(HFSPLUS_VOL_INCNSTNT);
590+
hfsplus_prepare_volume_header_for_commit(vhdr);
570591
hfsplus_sync_fs(sb, 1);
571592

572593
if (!sbi->hidden_dir) {

0 commit comments

Comments
 (0)