From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B963519ABD8; Tue, 10 Feb 2026 23:31:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770766301; cv=none; b=nCMwRxAmuUuZNV7E5T3xuiy8MVas4MXFs/WMFmKN5ek+9tcddGLx924zGtSIqFfroHBmztdGKXaWx04hw2u6PfwpldTsz5XAPwbQy752koOHUFi79s95hijCLvnXFugiBTS94uO+pq0dpG3ilG/EqHx+4ml4+/NHKmN262BzMn8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770766301; c=relaxed/simple; bh=yG52MkQGVKxZLjcLBo7RuOZ0hYK225uvZlhai6bDedE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=hRFA32+8O4VpxPNGdm2iY2lVNxcCTzqSHKs1lwbwW0MXacfm9VqOyn7t0hS0qROJUWxrtehoFhDDeo9EgbcIgh2rqB9e0CqXUNOzU3h/KWyYRa3yQiQPMg+706ynac09hhv37S4jk9kS0Z2XRNJOsStbfnep7XiyRUq6TR0b6BI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=ljbpDYzK; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="ljbpDYzK" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9A57CC116C6; Tue, 10 Feb 2026 23:31:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1770766301; bh=yG52MkQGVKxZLjcLBo7RuOZ0hYK225uvZlhai6bDedE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ljbpDYzK/uSCd24PC7YJiZUe/Cro5IfNtt/4op0Vw7qTyvFncSNI+p9I44P5xPweI IJJ84KAFeen5hhqmXki0AwG3jGz5xgd6cQ7ul+HlZQi5SEACyUkhKgkiMa2anVjW26 uYOn0hXPK/JGPr3pED1G4B9eO/e5UpL1g7iw2MeAX/NT2WJKu6DeRi6UlgL8OSZR+l /Vj857HnxPIR13A1tqi0QrG6PZNUML6osgivx1C2ufTyEJXvyRe0Gl7kfcHOLPS+V0 TutHjzc+bOWIe0bFxhIAwE2+Nu5QQuEXz+BSxTMwjBCaCj0l92PT0qKhox0uJhxUGa BVxwX118U3q4A== From: Sasha Levin To: patches@lists.linux.dev, stable@vger.kernel.org Cc: Viacheslav Dubeyko , John Paul Adrian Glaubitz , Yangtao Li , linux-fsdevel@vger.kernel.org, Sasha Levin Subject: [PATCH AUTOSEL 6.19-6.18] hfsplus: fix volume corruption issue for generic/480 Date: Tue, 10 Feb 2026 18:30:55 -0500 Message-ID: <20260210233123.2905307-10-sashal@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260210233123.2905307-1-sashal@kernel.org> References: <20260210233123.2905307-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: linux-fsdevel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 6.19 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From: Viacheslav Dubeyko [ 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 cc: John Paul Adrian Glaubitz cc: Yangtao Li cc: linux-fsdevel@vger.kernel.org Link: https://lore.kernel.org/r/20251205000054.3670326-1-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko Signed-off-by: Sasha Levin --- 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