[PATCH AUTOSEL 6.19-6.18] hfsplus: fix volume corruption issue for generic/480

[Sasha Levin <sashal@kernel.org>]

From: Viacheslav Dubeyko <slava@dubeyko.com>

[ Upstream commit bea4429eb30190c59b5ac7c8ff6c90176c7c110f ]

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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Here is my complete analysis:

---

## Commit Analysis: "hfsplus: fix volume corruption issue for
generic/480"

### 1. COMMIT MESSAGE ANALYSIS

The subject line explicitly says "fix volume corruption" - this is a
high-severity filesystem integrity bug. The commit message is
exceptionally detailed:

- Provides full xfstests output showing the corruption
- Includes `fsck.hfsplus` output documenting the specific corruption
  found: "Incorrect number of file hard links", "invalid VHB
  nextCatalogID", "Incorrect flags for file hard link"
- Describes the exact test scenario: unlink a hard link, create new file
  with same name, fsync, simulate power failure
- Explains the root cause clearly: directory operations modify inodes
  in-memory but don't persist changes to the HFS+ Catalog File B-tree
- Shows "after" test results proving the fix works
- Author is Viacheslav Dubeyko, the HFS+ subsystem maintainer

### 2. CODE CHANGE ANALYSIS

**The root cause**: HFS+ has a two-level write system:
1. VFS `mark_inode_dirty()` -> deferred writeback ->
   `hfsplus_write_inode()` -> `hfsplus_cat_write_inode()` (asynchronous)
2. Direct `hfsplus_cat_write_inode()` call (synchronous, writes
   immediately to catalog B-tree)

Five directory operations — `hfsplus_link()`, `hfsplus_unlink()`,
`hfsplus_rmdir()`, `hfsplus_symlink()`, `hfsplus_mknod()` — modify
multiple inodes (directory entries, link counts, timestamps) but only
called `mark_inode_dirty()` without directly writing to the Catalog
File. If a power failure occurs between the directory operation and the
deferred writeback, the on-disk catalog tree becomes inconsistent.

**The fix adds explicit `hfsplus_cat_write_inode()` calls**:
- `hfsplus_link()`: Writes `src_dir`, `dst_dir`, `sbi->hidden_dir`, and
  `inode` to catalog
- `hfsplus_unlink()`: Writes `dir`, `sbi->hidden_dir`, and `inode` to
  catalog (only on success)
- `hfsplus_rmdir()`: Writes `dir` to catalog
- `hfsplus_symlink()`: Writes `dir` and `inode` to catalog
- `hfsplus_mknod()`: Writes `dir` and `inode` to catalog

**This pattern already existed in `hfsplus_rename()`** (added by commit
24e17a29cf753 which fixed the same class of bug for generic/073):

```532:563:fs/hfsplus/dir.c
static int hfsplus_rename(struct mnt_idmap *idmap,
                          struct inode *old_dir, struct dentry
*old_dentry,
                          struct inode *new_dir, struct dentry
*new_dentry,
                          unsigned int flags)
{
        // ...
        if (!res) {
                new_dentry->d_fsdata = old_dentry->d_fsdata;
                res = hfsplus_cat_write_inode(old_dir);
                if (!res)
                        res = hfsplus_cat_write_inode(new_dir);
        }
        return res;
}
```

The current commit extends this same proven pattern to the five other
directory operations that were missing it.

**Additional changes**: The commit adds `hfs_dbg()` debug output in
multiple functions. The `hfs_dbg` macro maps to `pr_debug()` (compiled
out by default), so this is harmless at runtime. One minor whitespace
alignment change in `hfsplus_link()`.

### 3. CLASSIFICATION

- **Bug type**: Filesystem data corruption / inconsistency on power
  failure
- **Category**: Missing write-through of inode metadata to on-disk
  B-tree
- **Severity**: HIGH - filesystem corruption detected by fsck, incorrect
  hard link counts, flags, catalog IDs
- **Reproducibility**: Reliably reproducible with xfstests generic/480

### 4. SCOPE AND RISK ASSESSMENT

- **Files changed**: 2 (fs/hfsplus/dir.c, fs/hfsplus/inode.c)
- **Functional change**: ~33 lines of `hfsplus_cat_write_inode()` calls
  + error handling
- **Debug noise**: ~10 lines of `hfs_dbg()` additions
- **Risk**: LOW - `hfsplus_cat_write_inode()` is already called from the
  VFS writeback path (`hfsplus_write_inode()`) and from
  `hfsplus_rename()`, so it's thoroughly tested. The new calls add it at
  the same logical points in other directory operations.
- **Subsystem**: HFS+ filesystem - relatively narrow impact (only
  affects HFS+ users)

### 5. USER IMPACT

HFS+ is used for:
- Mac OS X / macOS compatibility
- External storage devices formatted as HFS+
- Dual-boot Linux/Mac systems

Without this fix, operations involving hard links, file deletion,
symlinks, or device node creation can lead to volume corruption after
power failure. The corruption manifests as incorrect link counts, wrong
catalog IDs, and incorrect file flags - requiring fsck repair.

### 6. DEPENDENCY CHECK

**The core fix is self-contained.** The `hfsplus_cat_write_inode()`
function exists in all kernel versions with HFS+ support. The additions
to `hfsplus_link()`, `hfsplus_unlink()`, `hfsplus_rmdir()`,
`hfsplus_symlink()`, and `hfsplus_mknod()` are independent of the
earlier `hfsplus_rename()` fix (24e17a29cf753).

**Backport concern**: The `hfs_dbg()` macro signature changed in v6.18
(commit f32a26fab3672). Before 6.18, `hfs_dbg` required a flag argument
as the first parameter (e.g., `hfs_dbg(INODE, "...")`). The current
commit uses the new format (no flag argument). For stable trees before
6.18, the `hfs_dbg()` additions would need to be stripped from the
backport, but the core fix applies cleanly without them.

### 7. STABILITY INDICATORS

- Author is the HFS+ subsystem maintainer
- This is the third in a series of similar corruption fixes
  (generic/073, generic/101, generic/480), demonstrating the author has
  deep understanding of the issue class
- Verified with xfstests before and after
- The pattern used (calling `hfsplus_cat_write_inode()` from directory
  operations) is already proven in `hfsplus_rename()`

### CONCLUSION

This commit fixes a real, reproducible filesystem corruption bug in HFS+
that manifests during power failure scenarios. The fix is conceptually
simple - ensuring that all modified inodes are persisted to the catalog
B-tree during directory operations, matching the pattern already
established in `hfsplus_rename()`. The risk is low because the called
function is well-tested and the change pattern is proven. The debug
output additions are harmless noise that may need stripping for older
stable trees but don't affect the core fix.

Filesystem corruption is one of the highest-priority categories for
stable backporting, as it directly impacts data integrity.

**YES**

 fs/hfsplus/dir.c   | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 fs/hfsplus/inode.c |  5 +++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index cadf0b5f93422..ca5f74a140ec1 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -313,6 +313,9 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir,
 	if (!S_ISREG(inode->i_mode))
 		return -EPERM;
 
+	hfs_dbg("src_dir->i_ino %lu, dst_dir->i_ino %lu, inode->i_ino %lu\n",
+		src_dir->i_ino, dst_dir->i_ino, inode->i_ino);
+
 	mutex_lock(&sbi->vh_mutex);
 	if (inode->i_ino == (u32)(unsigned long)src_dentry->d_fsdata) {
 		for (;;) {
@@ -332,7 +335,7 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir,
 		cnid = sbi->next_cnid++;
 		src_dentry->d_fsdata = (void *)(unsigned long)cnid;
 		res = hfsplus_create_cat(cnid, src_dir,
-			&src_dentry->d_name, inode);
+					 &src_dentry->d_name, inode);
 		if (res)
 			/* panic? */
 			goto out;
@@ -350,6 +353,21 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir,
 	mark_inode_dirty(inode);
 	sbi->file_count++;
 	hfsplus_mark_mdb_dirty(dst_dir->i_sb);
+
+	res = hfsplus_cat_write_inode(src_dir);
+	if (res)
+		goto out;
+
+	res = hfsplus_cat_write_inode(dst_dir);
+	if (res)
+		goto out;
+
+	res = hfsplus_cat_write_inode(sbi->hidden_dir);
+	if (res)
+		goto out;
+
+	res = hfsplus_cat_write_inode(inode);
+
 out:
 	mutex_unlock(&sbi->vh_mutex);
 	return res;
@@ -367,6 +385,9 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry)
 	if (HFSPLUS_IS_RSRC(inode))
 		return -EPERM;
 
+	hfs_dbg("dir->i_ino %lu, inode->i_ino %lu\n",
+		dir->i_ino, inode->i_ino);
+
 	mutex_lock(&sbi->vh_mutex);
 	cnid = (u32)(unsigned long)dentry->d_fsdata;
 	if (inode->i_ino == cnid &&
@@ -408,6 +429,15 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry)
 	inode_set_ctime_current(inode);
 	mark_inode_dirty(inode);
 out:
+	if (!res) {
+		res = hfsplus_cat_write_inode(dir);
+		if (!res) {
+			res = hfsplus_cat_write_inode(sbi->hidden_dir);
+			if (!res)
+				res = hfsplus_cat_write_inode(inode);
+		}
+	}
+
 	mutex_unlock(&sbi->vh_mutex);
 	return res;
 }
@@ -429,6 +459,8 @@ static int hfsplus_rmdir(struct inode *dir, struct dentry *dentry)
 	inode_set_ctime_current(inode);
 	hfsplus_delete_inode(inode);
 	mark_inode_dirty(inode);
+
+	res = hfsplus_cat_write_inode(dir);
 out:
 	mutex_unlock(&sbi->vh_mutex);
 	return res;
@@ -465,6 +497,12 @@ static int hfsplus_symlink(struct mnt_idmap *idmap, struct inode *dir,
 
 	hfsplus_instantiate(dentry, inode, inode->i_ino);
 	mark_inode_dirty(inode);
+
+	res = hfsplus_cat_write_inode(dir);
+	if (res)
+		goto out;
+
+	res = hfsplus_cat_write_inode(inode);
 	goto out;
 
 out_err:
@@ -506,6 +544,12 @@ static int hfsplus_mknod(struct mnt_idmap *idmap, struct inode *dir,
 
 	hfsplus_instantiate(dentry, inode, inode->i_ino);
 	mark_inode_dirty(inode);
+
+	res = hfsplus_cat_write_inode(dir);
+	if (res)
+		goto out;
+
+	res = hfsplus_cat_write_inode(inode);
 	goto out;
 
 failed_mknod:
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 7ae6745ca7ae1..c762bf909d1aa 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -328,6 +328,9 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
 	struct hfsplus_vh *vhdr = sbi->s_vhdr;
 	int error = 0, error2;
 
+	hfs_dbg("inode->i_ino %lu, start %llu, end %llu\n",
+		inode->i_ino, start, end);
+
 	error = file_write_and_wait_range(file, start, end);
 	if (error)
 		return error;
@@ -616,6 +619,8 @@ int hfsplus_cat_write_inode(struct inode *inode)
 	hfsplus_cat_entry entry;
 	int res = 0;
 
+	hfs_dbg("inode->i_ino %lu\n", inode->i_ino);
+
 	if (HFSPLUS_IS_RSRC(inode))
 		main_inode = HFSPLUS_I(inode)->rsrc_inode;
 
-- 
2.51.0