[PATCH] xfs: validate inode log item regions during recovery

[Weiming Shi <bestswngs@gmail.com>]

xlog_recover_inode_commit_pass2() replays an XFS_LI_INODE item straight
out of the log: it dereferences ri_buf[1] as the log dinode and copies the
data and attr forks from ri_buf[2]/ri_buf[attr_index], but never checks
that those regions were logged or that the dinode geometry agrees with the
mount. ri_buf is sized by the item's ilf_size, bounded only to
[1, XLOG_MAX_REGIONS_IN_ITEM]; the number of regions actually filled in
(ri_cnt) and the fork copies are guarded only by ASSERT()s, which compile
out in production.

A crafted image mounted for log recovery can thus hit a NULL dereference of
an absent region, an out-of-bounds read from a di_version or di_forkoff
that disagrees with the mount, or an overflow of the inode fork by an
oversized local or extent region. An item declaring a second region that
is never logged faults on the log dinode:

 KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
 RIP: xlog_recover_inode_commit_pass2 (fs/xfs/xfs_inode_item_recover.c:370)
 Call Trace:
  xlog_recover_items_pass2 (fs/xfs/xfs_log_recover.c:2011)
  xlog_recover_commit_trans (fs/xfs/xfs_log_recover.c:2078)
  xlog_recovery_process_trans (fs/xfs/xfs_log_recover.c:2328)
  xlog_recover_process_data (fs/xfs/xfs_log_recover.c:2502)
  xlog_recover (fs/xfs/xfs_log_recover.c:3486)
  xfs_log_mount (fs/xfs/xfs_log.c:667)
  xfs_mountfs (fs/xfs/xfs_mount.c:1039)
  xfs_fs_fill_super (fs/xfs/xfs_super.c:1965)
  get_tree_bdev_flags (fs/super.c:1680)
  vfs_get_tree (fs/super.c:1803)
  __x64_sys_mount (fs/namespace.c:4433)

Validate the regions before use: require ri_cnt to cover the regions
ilf_fields implies, the log dinode to be present and at least a log dinode
in size, and di_version to match the mount. Replace the di_forkoff check,
which can never fire (di_forkoff is a u8 of 8-byte units, sb_inodesize a
byte count of at least 256), with xfs_dinode_verify_forkoff() (exported
here), and bound the local and extent fork copies against the destination
fork. The btree-root formats are logged in their larger in-core form and
converted on the way in, so they are not bounded by the on-disk fork size.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Xiang Mei <xmei5@asu.edu>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 fs/xfs/libxfs/xfs_inode_buf.c   |  2 +-
 fs/xfs/libxfs/xfs_inode_buf.h   |  2 ++
 fs/xfs/xfs_inode_item_recover.c | 59 ++++++++++++++++++++++++---------
 3 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 336ef843f2fe..71311c4fc2ed 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -479,7 +479,7 @@ xfs_dinode_verify_fork(
 	return NULL;
 }
 
-static xfs_failaddr_t
+xfs_failaddr_t
 xfs_dinode_verify_forkoff(
 	struct xfs_dinode	*dip,
 	struct xfs_mount	*mp)
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index f3624532b023..d120675f6c07 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -27,6 +27,8 @@ int	xfs_inode_from_disk(struct xfs_inode *ip, struct xfs_dinode *from);
 
 xfs_failaddr_t xfs_dinode_verify(struct xfs_mount *mp, xfs_ino_t ino,
 			   struct xfs_dinode *dip);
+xfs_failaddr_t xfs_dinode_verify_forkoff(struct xfs_dinode *dip,
+			   struct xfs_mount *mp);
 xfs_failaddr_t xfs_dinode_verify_metadir(struct xfs_mount *mp,
 		struct xfs_dinode *dip, uint16_t mode, uint16_t flags,
 		uint64_t flags2);
diff --git a/fs/xfs/xfs_inode_item_recover.c b/fs/xfs/xfs_inode_item_recover.c
index 169a8fe3bf0a..8ebc8eaf9e03 100644
--- a/fs/xfs/xfs_inode_item_recover.c
+++ b/fs/xfs/xfs_inode_item_recover.c
@@ -366,6 +366,13 @@ xlog_recover_inode_commit_pass2(
 		error = -EFSCORRUPTED;
 		goto out_release;
 	}
+	/* ri_buf[1] may be absent or shorter than a log dinode */
+	if (XFS_IS_CORRUPT(mp, item->ri_cnt < 2) ||
+	    XFS_IS_CORRUPT(mp,
+			   item->ri_buf[1].iov_len < xfs_log_dinode_size(mp))) {
+		error = -EFSCORRUPTED;
+		goto out_release;
+	}
 	ldip = item->ri_buf[1].iov_base;
 	if (XFS_IS_CORRUPT(mp, ldip->di_magic != XFS_DINODE_MAGIC)) {
 		xfs_alert(mp,
@@ -374,6 +381,11 @@ xlog_recover_inode_commit_pass2(
 		error = -EFSCORRUPTED;
 		goto out_release;
 	}
+	/* the size gate and fork offsets assume the mount's inode version */
+	if (XFS_IS_CORRUPT(mp, !xfs_dinode_good_version(mp, ldip->di_version))) {
+		error = -EFSCORRUPTED;
+		goto out_release;
+	}
 
 	/*
 	 * If the inode has an LSN in it, recover the inode only if the on-disk
@@ -462,15 +474,6 @@ xlog_recover_inode_commit_pass2(
 	if (error)
 		goto out_release;
 
-	if (unlikely(ldip->di_forkoff > mp->m_sb.sb_inodesize)) {
-		XFS_CORRUPTION_ERROR("Bad log dinode fork offset",
-				XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
-		xfs_alert(mp,
-			"Bad inode 0x%llx, di_forkoff 0x%x",
-			in_f->ilf_ino, ldip->di_forkoff);
-		error = -EFSCORRUPTED;
-		goto out_release;
-	}
 	isize = xfs_log_dinode_size(mp);
 	if (unlikely(item->ri_buf[1].iov_len > isize)) {
 		XFS_CORRUPTION_ERROR("Bad log dinode size", XFS_ERRLEVEL_LOW,
@@ -495,6 +498,30 @@ xlog_recover_inode_commit_pass2(
 	xfs_log_dinode_to_disk(ldip, dip, current_lsn);
 
 	fields = in_f->ilf_fields;
+
+	/* forks are otherwise ASSERT-only; check di_forkoff and presence */
+	if (fields & (XFS_ILOG_DFORK | XFS_ILOG_AFORK)) {
+		if (XFS_IS_CORRUPT(mp, xfs_dinode_verify_forkoff(dip, mp) != NULL)) {
+			error = -EFSCORRUPTED;
+			goto out_release;
+		}
+	}
+	if (fields & XFS_ILOG_DFORK) {
+		if (XFS_IS_CORRUPT(mp, item->ri_cnt < 3) ||
+		    XFS_IS_CORRUPT(mp, item->ri_buf[2].iov_base == NULL)) {
+			error = -EFSCORRUPTED;
+			goto out_release;
+		}
+	}
+	if (fields & XFS_ILOG_AFORK) {
+		attr_index = (fields & XFS_ILOG_DFORK) ? 3 : 2;
+
+		if (XFS_IS_CORRUPT(mp, item->ri_cnt < attr_index + 1) ||
+		    XFS_IS_CORRUPT(mp, item->ri_buf[attr_index].iov_base == NULL)) {
+			error = -EFSCORRUPTED;
+			goto out_release;
+		}
+	}
 	if (fields & XFS_ILOG_DEV)
 		xfs_dinode_put_rdev(dip, in_f->ilf_u.ilfu_rdev);
 
@@ -510,6 +537,10 @@ xlog_recover_inode_commit_pass2(
 	switch (fields & XFS_ILOG_DFORK) {
 	case XFS_ILOG_DDATA:
 	case XFS_ILOG_DEXT:
+		if (XFS_IS_CORRUPT(mp, len > XFS_DFORK_DSIZE(dip, mp))) {
+			error = -EFSCORRUPTED;
+			goto out_release;
+		}
 		memcpy(XFS_DFORK_DPTR(dip), src, len);
 		break;
 
@@ -533,11 +564,6 @@ xlog_recover_inode_commit_pass2(
 	 * transaction.
 	 */
 	if (in_f->ilf_fields & XFS_ILOG_AFORK) {
-		if (in_f->ilf_fields & XFS_ILOG_DFORK) {
-			attr_index = 3;
-		} else {
-			attr_index = 2;
-		}
 		len = item->ri_buf[attr_index].iov_len;
 		src = item->ri_buf[attr_index].iov_base;
 		ASSERT(len == xlog_calc_iovec_len(in_f->ilf_asize));
@@ -546,7 +572,10 @@ xlog_recover_inode_commit_pass2(
 		case XFS_ILOG_ADATA:
 		case XFS_ILOG_AEXT:
 			dest = XFS_DFORK_APTR(dip);
-			ASSERT(len <= XFS_DFORK_ASIZE(dip, mp));
+			if (XFS_IS_CORRUPT(mp, len > XFS_DFORK_ASIZE(dip, mp))) {
+				error = -EFSCORRUPTED;
+				goto out_release;
+			}
 			memcpy(dest, src, len);
 			break;
 
-- 
2.43.0