[PATCH v3 2/3] xfs: verify recovered log items are complete before replaying them

[Weiming Shi]

xlog_recover_add_to_trans() assembles each recovered log item into an
ri_buf[] of ri_total slots, where ri_total is the region count the item's
format header declared, and counts the regions actually logged in ri_cnt.
Nothing checked that ri_cnt reached ri_total once the item was fully
decoded, so a crafted or truncated log can present an item whose header
declares more regions than were logged. The trailing ri_buf[] slots stay
NULL, and the reorder, readahead and replay code then dereference them.

For example, an XFS_LI_INODE item declaring two regions but logging only
the format region leaves ri_buf[1] NULL, and mount-time recovery faults
dereferencing it as 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)
  __x64_sys_mount (fs/namespace.c:4433)

An item is fully decoded once every region its header declared has
arrived. That happens when the next item's header starts (the current
item is closed out in xlog_recover_add_to_trans()) or, for the last item
in the transaction, when the commit record arrives
(xlog_recover_commit_trans()). Verify the item at both of those points
with xlog_recover_verify_item(): confirm it received all its declared
regions, and run the item type's verifier if one is provided. Item types
opt in with a new xlog_recover_item_ops->verify method; the first, for
inode items, is added in the next patch.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Xiang Mei <xmei5@xxxxxxx>
Suggested-by: Dave Chinner <dgc@xxxxxxxxxx>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@xxxxxxxxx>
---
 fs/xfs/libxfs/xfs_log_recover.h |  9 ++++++++
 fs/xfs/xfs_log_recover.c        | 41 ++++++++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
index 77f51afcbd9c..c7c93c65f60a 100644
--- a/fs/xfs/libxfs/xfs_log_recover.h
+++ b/fs/xfs/libxfs/xfs_log_recover.h
@@ -41,6 +41,15 @@ struct xlog_recover_item_ops {
 	 */
 	enum xlog_recover_reorder (*reorder)(struct xlog_recover_item *item);
 
+	/*
+	 * Comprehensively validate a fully decoded item's formatted log
+	 * structures, if provided: the region count and sizes the item type
+	 * requires, and any header fields that can be checked without the
+	 * on-disk buffer.  Called once the item is complete, before it is
+	 * queued for replay.  Returning an error aborts recovery.
+	 */
+	int (*verify)(struct xlog *log, struct xlog_recover_item *item);
+
 	/* Start readahead for pass2, if provided. */
 	void (*ra_pass2)(struct xlog *log, struct xlog_recover_item *item);
 
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 41fa029054d3..956599b73b16 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -1997,6 +1997,9 @@ xlog_recover_items_pass2(
 	return error;
 }
 
+STATIC int xlog_recover_verify_item(struct xlog *log,
+		struct xlog_recover_item *item);
+
 /*
  * Perform the transaction.
  *
@@ -2021,6 +2024,18 @@ xlog_recover_commit_trans(
 
 	hlist_del_init(&trans->r_list);
 
+	/*
+	 * The final item is completed by the commit record rather than by a
+	 * following item, so no decode step has verified it yet; do so now.
+	 */
+	if (!list_empty(&trans->r_itemq)) {
+		item = list_entry(trans->r_itemq.prev,
+				  struct xlog_recover_item, ri_list);
+		error = xlog_recover_verify_item(log, item);
+		if (error)
+			return error;
+	}
+
 	xlog_recover_reorder_trans(log, trans, pass);
 
 	list_for_each_entry_safe(item, next, &trans->r_itemq, ri_list) {
@@ -2164,6 +2179,25 @@ xlog_recover_verify_item_header(
 	return 0;
 }
 
+/*
+ * A fully decoded item has received all its declared regions.  Check it is
+ * complete and run the type's verifier, if any, before it is queued for
+ * replay.
+ */
+STATIC int
+xlog_recover_verify_item(
+	struct xlog			*log,
+	struct xlog_recover_item	*item)
+{
+	if (XFS_IS_CORRUPT(log->l_mp,
+			item->ri_total == 0 || item->ri_cnt != item->ri_total))
+		return -EFSCORRUPTED;
+
+	if (item->ri_ops->verify)
+		return item->ri_ops->verify(log, item);
+	return 0;
+}
+
 /*
  * The next region to add is the start of a new region.  It could be
  * a whole region or it could be the first part of a new region.  Because
@@ -2226,7 +2260,12 @@ xlog_recover_add_to_trans(
 			  ri_list);
 	if (item->ri_total != 0 &&
 	     item->ri_total == item->ri_cnt) {
-		/* tail item is in use, get a new one */
+		/* the tail item is complete; verify it before starting a new one */
+		error = xlog_recover_verify_item(log, item);
+		if (error) {
+			kvfree(ptr);
+			return error;
+		}
 		xlog_recover_add_item(&trans->r_itemq);
 		item = list_entry(trans->r_itemq.prev,
 					struct xlog_recover_item, ri_list);
-- 
2.43.0