[PATCH AUTOSEL 6.18-5.10] hfsplus: Verify inode mode when loading from disk

[Sasha Levin <sashal@kernel.org>]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

[ Upstream commit 005d4b0d33f6b4a23d382b7930f7a96b95b01f39 ]

syzbot is reporting that S_IFMT bits of inode->i_mode can become bogus when
the S_IFMT bits of the 16bits "mode" field loaded from disk are corrupted.

According to [1], the permissions field was treated as reserved in Mac OS
8 and 9. According to [2], the reserved field was explicitly initialized
with 0, and that field must remain 0 as long as reserved. Therefore, when
the "mode" field is not 0 (i.e. no longer reserved), the file must be
S_IFDIR if dir == 1, and the file must be one of S_IFREG/S_IFLNK/S_IFCHR/
S_IFBLK/S_IFIFO/S_IFSOCK if dir == 0.

Reported-by: syzbot <syzbot+895c23f6917da440ed0d@syzkaller.appspotmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=895c23f6917da440ed0d
Link: https://developer.apple.com/library/archive/technotes/tn/tn1150.html#HFSPlusPermissions [1]
Link: https://developer.apple.com/library/archive/technotes/tn/tn1150.html#ReservedAndPadFields [2]
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Link: https://lore.kernel.org/r/04ded9f9-73fb-496c-bfa5-89c4f5d1d7bb@I-love.SAKURA.ne.jp
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of hfsplus: Verify inode mode when loading from disk

### 1. COMMIT MESSAGE ANALYSIS

**Key indicators:**
- **Reported-by: syzbot** - This is a real bug found by automated
  fuzzing
- **Closes:** link to syzkaller bug report confirms this is a genuine
  issue
- **Reviewed-by:** present from the HFS+ maintainer (Viacheslav Dubeyko)
- **No "Cc: stable@vger.kernel.org"** tag
- **No "Fixes:" tag** - bug appears to exist since original HFS+
  implementation

The commit describes that corrupted S_IFMT bits in the on-disk "mode"
field can cause inode->i_mode to become bogus when loaded from disk. The
commit message references Apple technical documentation explaining the
expected values for the mode field.

### 2. CODE CHANGE ANALYSIS

The fix modifies `hfsplus_get_perms()` in two ways:

**a) Adds validation logic (the core fix):**
```c
if (dir) {
    if (mode && !S_ISDIR(mode))
        goto bad_type;
} else if (mode) {
    switch (mode & S_IFMT) {
    case S_IFREG:
    case S_IFLNK:
    case S_IFCHR:
    case S_IFBLK:
    case S_IFIFO:
    case S_IFSOCK:
        break;
    default:
        goto bad_type;
    }
}
```
This validates that:
- For directories (`dir=1`): mode must be 0 or actually be a directory
  type
- For files (`dir=0`): mode must be 0 or one of the valid file types
  (regular, symlink, char/block device, FIFO, socket)

**b) Changes return type from `void` to `int`:**
- Returns -EIO on invalid mode with an error message
- Callers (`hfsplus_cat_read_inode`) now check the return value and
  propagate errors

**Root cause:** The original code blindly trusted the mode field from
disk without validating that the S_IFMT bits are consistent with the
directory flag.

### 3. CLASSIFICATION

- **Type:** Bug fix (input validation)
- **Security relevance:** Yes - crafted filesystem images could trigger
  this
- **Category:** Filesystem robustness/hardening against corrupted data

### 4. SCOPE AND RISK ASSESSMENT

| Aspect | Assessment |
|--------|------------|
| Lines changed | ~30+ additions, moderate size |
| Files touched | 1 file (fs/hfsplus/inode.c) |
| Complexity | Low - straightforward validation logic |
| Regression risk | **LOW** - only rejects clearly invalid data |

The validation is conservative and follows Apple's official HFS+
specification. It only rejects modes that are definitively wrong.

### 5. USER IMPACT

- **Affected users:** Those mounting HFS+ filesystems (macOS external
  drives, dual-boot setups)
- **Trigger:** Mounting a corrupted or maliciously crafted HFS+
  filesystem image
- **Impact of bug:** Bogus inode mode can lead to undefined kernel
  behavior when processing the inode
- **Impact of fix:** Graceful rejection with -EIO instead of corrupted
  internal state

### 6. STABILITY INDICATORS

- Reviewed by subsystem maintainer ✓
- Clean, standalone fix with no dependencies ✓
- The modified functions exist in older stable kernels ✓
- No unusual code patterns or risky constructs ✓

### 7. DEPENDENCY CHECK

This is a standalone fix. The `hfsplus_get_perms` and
`hfsplus_cat_read_inode` functions exist in all stable trees where HFS+
is supported.

---

## Summary

**What it fixes:** Prevents corrupted or maliciously crafted HFS+
filesystem images from causing bogus inode modes to be loaded into the
kernel.

**Why it matters for stable:** This is a defensive fix that prevents
accepting corrupted data, which could lead to undefined behavior. syzbot
found this bug, indicating it can be triggered by crafted input - a
potential security concern.

**Meets stable criteria:**
- ✓ Obviously correct (validates according to Apple's HFS+
  specification)
- ✓ Fixes a real bug that affects users (syzbot found it with crafted
  images)
- ✓ Small and contained (single file, ~30 lines of validation)
- ✓ Low regression risk (only rejects clearly invalid data)
- ✗ No explicit stable tag (author didn't request it)

**Risk vs Benefit:**
- **Risk:** Very low - adds validation that only rejects invalid data
- **Benefit:** Prevents kernel from operating on corrupted inode modes
  from crafted/corrupted filesystems

The lack of explicit stable tags suggests the author may not have
intended this for stable, but the fix clearly meets stable criteria as a
filesystem robustness fix against corrupted input. Filesystem fixes for
handling crafted/corrupted data are generally appropriate for stable
trees.

**YES**

 fs/hfsplus/inode.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index b51a411ecd237..e290e417ed3a7 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -180,13 +180,29 @@ const struct dentry_operations hfsplus_dentry_operations = {
 	.d_compare    = hfsplus_compare_dentry,
 };
 
-static void hfsplus_get_perms(struct inode *inode,
-		struct hfsplus_perm *perms, int dir)
+static int hfsplus_get_perms(struct inode *inode,
+			     struct hfsplus_perm *perms, int dir)
 {
 	struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb);
 	u16 mode;
 
 	mode = be16_to_cpu(perms->mode);
+	if (dir) {
+		if (mode && !S_ISDIR(mode))
+			goto bad_type;
+	} else if (mode) {
+		switch (mode & S_IFMT) {
+		case S_IFREG:
+		case S_IFLNK:
+		case S_IFCHR:
+		case S_IFBLK:
+		case S_IFIFO:
+		case S_IFSOCK:
+			break;
+		default:
+			goto bad_type;
+		}
+	}
 
 	i_uid_write(inode, be32_to_cpu(perms->owner));
 	if ((test_bit(HFSPLUS_SB_UID, &sbi->flags)) || (!i_uid_read(inode) && !mode))
@@ -212,6 +228,10 @@ static void hfsplus_get_perms(struct inode *inode,
 		inode->i_flags |= S_APPEND;
 	else
 		inode->i_flags &= ~S_APPEND;
+	return 0;
+bad_type:
+	pr_err("invalid file type 0%04o for inode %lu\n", mode, inode->i_ino);
+	return -EIO;
 }
 
 static int hfsplus_file_open(struct inode *inode, struct file *file)
@@ -516,7 +536,9 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd)
 		}
 		hfs_bnode_read(fd->bnode, &entry, fd->entryoffset,
 					sizeof(struct hfsplus_cat_folder));
-		hfsplus_get_perms(inode, &folder->permissions, 1);
+		res = hfsplus_get_perms(inode, &folder->permissions, 1);
+		if (res)
+			goto out;
 		set_nlink(inode, 1);
 		inode->i_size = 2 + be32_to_cpu(folder->valence);
 		inode_set_atime_to_ts(inode, hfsp_mt2ut(folder->access_date));
@@ -545,7 +567,9 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd)
 
 		hfsplus_inode_read_fork(inode, HFSPLUS_IS_RSRC(inode) ?
 					&file->rsrc_fork : &file->data_fork);
-		hfsplus_get_perms(inode, &file->permissions, 0);
+		res = hfsplus_get_perms(inode, &file->permissions, 0);
+		if (res)
+			goto out;
 		set_nlink(inode, 1);
 		if (S_ISREG(inode->i_mode)) {
 			if (file->permissions.dev)
-- 
2.51.0