* [PATCH v3 1/3] xfs: verify log item headers when they are decoded during recovery
2026-07-19 11:29 [PATCH v3 0/3] xfs: add a log item verification layer to recovery Weiming Shi
@ 2026-07-19 11:29 ` Weiming Shi
2026-07-19 11:29 ` [PATCH v3 2/3] xfs: verify recovered log items are complete before replaying them Weiming Shi
2026-07-19 11:29 ` [PATCH v3 3/3] xfs: add an inode log item recovery verifier Weiming Shi
2 siblings, 0 replies; 4+ messages in thread
From: Weiming Shi @ 2026-07-19 11:29 UTC (permalink / raw)
To: Carlos Maiolino, Darrick J . Wong
Cc: linux-xfs, linux-kernel, xmei5, Weiming Shi, Dave Chinner
Log recovery rebuilds each log item in xlog_recover_add_to_trans() from
the ophdr regions in the journal. The first region carries the item's
format header, and recovery reads the type from its first two bytes and
the region count from the next two to size and populate the item's
ri_buf[] array. Those regions are later cast back to their format
structures and used to drive replay. This is the first point at which
recovery trusts data read from the log, and the geometry it reads here is
used to build the structures every later check and decode step relies on,
so the header should be verified comprehensively here rather than trusted
until something dereferences it.
Do that in xlog_recover_verify_item_header(): require the region to be
large enough to hold the type and count fields before they are read,
resolve the item type against the known xlog_recover_item_ops (rejecting
unrecognised types), and bound the declared region count within the
minimum and maximum a log item of that type is formatted with. A lower
bound matters as much as an upper one: an item that declares fewer
regions than its replay code indexes still passes the completeness check
added later in the series and then reads past its ri_buf[] array, e.g. a
dquot item with qlf_size == 1 still reaches ri_buf[1] in
xlog_recover_dquot_commit_pass2(). Record both bounds in new min_regions
and max_regions fields on xlog_recover_item_ops, where a max_regions of 0
keeps the generic XLOG_MAX_REGIONS_IN_ITEM ceiling used by buffer items.
Resolving the type here also lets us set item->ri_ops at decode time, so
xlog_recover_reorder_trans() no longer has to look it up and can no longer
meet an unrecognised item.
While decoding the continuation of a region in
xlog_recover_add_to_cont_trans(), also reject a continuation that has no
started region to extend, instead of indexing ri_buf[-1] on a
never-populated item.
Suggested-by: Dave Chinner <dgc@kernel.org>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
fs/xfs/libxfs/xfs_log_recover.h | 8 ++++
fs/xfs/xfs_attr_item.c | 4 ++
fs/xfs/xfs_bmap_item.c | 4 ++
fs/xfs/xfs_dquot_item_recover.c | 4 ++
fs/xfs/xfs_exchmaps_item.c | 4 ++
fs/xfs/xfs_extfree_item.c | 8 ++++
fs/xfs/xfs_icreate_item.c | 2 +
fs/xfs/xfs_inode_item_recover.c | 2 +
fs/xfs/xfs_log_recover.c | 80 ++++++++++++++++++++-------------
fs/xfs/xfs_refcount_item.c | 8 ++++
fs/xfs/xfs_rmap_item.c | 8 ++++
11 files changed, 101 insertions(+), 31 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
index 9e712e62369c..77f51afcbd9c 100644
--- a/fs/xfs/libxfs/xfs_log_recover.h
+++ b/fs/xfs/libxfs/xfs_log_recover.h
@@ -24,6 +24,14 @@ enum xlog_recover_reorder {
struct xlog_recover_item_ops {
uint16_t item_type; /* XFS_LI_* type code. */
+ /*
+ * Bounds on the item's declared region count, checked before the
+ * region array is allocated. max_regions 0 means no fixed maximum, so
+ * the generic XLOG_MAX_REGIONS_IN_ITEM ceiling applies (buffers).
+ */
+ unsigned int min_regions;
+ unsigned int max_regions;
+
/*
* Help sort recovered log items into the order required to replay them
* correctly. Log item types that always use XLOG_REORDER_ITEM_LIST do
diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index a53ee2b0c54a..cec7c1bb3694 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -1189,6 +1189,8 @@ static const struct xfs_item_ops xfs_attri_item_ops = {
const struct xlog_recover_item_ops xlog_attri_item_ops = {
.item_type = XFS_LI_ATTRI,
+ .max_regions = 5,
+ .min_regions = 2,
.commit_pass2 = xlog_recover_attri_commit_pass2,
};
@@ -1203,5 +1205,7 @@ static const struct xfs_item_ops xfs_attrd_item_ops = {
const struct xlog_recover_item_ops xlog_attrd_item_ops = {
.item_type = XFS_LI_ATTRD,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_attrd_commit_pass2,
};
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 89f6e79a955f..8659428a51bb 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -684,6 +684,8 @@ xlog_recover_bui_commit_pass2(
const struct xlog_recover_item_ops xlog_bui_item_ops = {
.item_type = XFS_LI_BUI,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_bui_commit_pass2,
};
@@ -716,5 +718,7 @@ xlog_recover_bud_commit_pass2(
const struct xlog_recover_item_ops xlog_bud_item_ops = {
.item_type = XFS_LI_BUD,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_bud_commit_pass2,
};
diff --git a/fs/xfs/xfs_dquot_item_recover.c b/fs/xfs/xfs_dquot_item_recover.c
index fe419b28de22..5882c688ecc1 100644
--- a/fs/xfs/xfs_dquot_item_recover.c
+++ b/fs/xfs/xfs_dquot_item_recover.c
@@ -178,6 +178,8 @@ xlog_recover_dquot_commit_pass2(
const struct xlog_recover_item_ops xlog_dquot_item_ops = {
.item_type = XFS_LI_DQUOT,
+ .max_regions = 2,
+ .min_regions = 2,
.ra_pass2 = xlog_recover_dquot_ra_pass2,
.commit_pass2 = xlog_recover_dquot_commit_pass2,
};
@@ -211,6 +213,8 @@ xlog_recover_quotaoff_commit_pass1(
const struct xlog_recover_item_ops xlog_quotaoff_item_ops = {
.item_type = XFS_LI_QUOTAOFF,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass1 = xlog_recover_quotaoff_commit_pass1,
/* nothing to commit in pass2 */
};
diff --git a/fs/xfs/xfs_exchmaps_item.c b/fs/xfs/xfs_exchmaps_item.c
index c3745d33e54e..d3f0fb677341 100644
--- a/fs/xfs/xfs_exchmaps_item.c
+++ b/fs/xfs/xfs_exchmaps_item.c
@@ -576,6 +576,8 @@ xlog_recover_xmi_commit_pass2(
const struct xlog_recover_item_ops xlog_xmi_item_ops = {
.item_type = XFS_LI_XMI,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_xmi_commit_pass2,
};
@@ -607,5 +609,7 @@ xlog_recover_xmd_commit_pass2(
const struct xlog_recover_item_ops xlog_xmd_item_ops = {
.item_type = XFS_LI_XMD,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_xmd_commit_pass2,
};
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index 2266d56e37dc..dccc3d159268 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -889,6 +889,8 @@ xlog_recover_efi_commit_pass2(
const struct xlog_recover_item_ops xlog_efi_item_ops = {
.item_type = XFS_LI_EFI,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_efi_commit_pass2,
};
@@ -941,6 +943,8 @@ xlog_recover_rtefi_commit_pass2(
const struct xlog_recover_item_ops xlog_rtefi_item_ops = {
.item_type = XFS_LI_EFI_RT,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_rtefi_commit_pass2,
};
@@ -984,6 +988,8 @@ xlog_recover_efd_commit_pass2(
const struct xlog_recover_item_ops xlog_efd_item_ops = {
.item_type = XFS_LI_EFD,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_efd_commit_pass2,
};
@@ -1025,5 +1031,7 @@ xlog_recover_rtefd_commit_pass2(
const struct xlog_recover_item_ops xlog_rtefd_item_ops = {
.item_type = XFS_LI_EFD_RT,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_rtefd_commit_pass2,
};
diff --git a/fs/xfs/xfs_icreate_item.c b/fs/xfs/xfs_icreate_item.c
index 95b0eba242e9..9afe58f7bfd4 100644
--- a/fs/xfs/xfs_icreate_item.c
+++ b/fs/xfs/xfs_icreate_item.c
@@ -255,6 +255,8 @@ xlog_recover_icreate_commit_pass2(
const struct xlog_recover_item_ops xlog_icreate_item_ops = {
.item_type = XFS_LI_ICREATE,
+ .max_regions = 1,
+ .min_regions = 1,
.reorder = xlog_recover_icreate_reorder,
.commit_pass2 = xlog_recover_icreate_commit_pass2,
};
diff --git a/fs/xfs/xfs_inode_item_recover.c b/fs/xfs/xfs_inode_item_recover.c
index 169a8fe3bf0a..6b6ac92964d0 100644
--- a/fs/xfs/xfs_inode_item_recover.c
+++ b/fs/xfs/xfs_inode_item_recover.c
@@ -599,6 +599,8 @@ xlog_recover_inode_commit_pass2(
const struct xlog_recover_item_ops xlog_inode_item_ops = {
.item_type = XFS_LI_INODE,
+ .max_regions = 4,
+ .min_regions = 2,
.ra_pass2 = xlog_recover_inode_ra_pass2,
.commit_pass2 = xlog_recover_inode_commit_pass2,
};
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 09e6678ca487..41fa029054d3 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -1828,12 +1828,12 @@ static const struct xlog_recover_item_ops *xlog_recover_item_ops[] = {
static const struct xlog_recover_item_ops *
xlog_find_item_ops(
- struct xlog_recover_item *item)
+ unsigned short item_type)
{
unsigned int i;
for (i = 0; i < ARRAY_SIZE(xlog_recover_item_ops); i++)
- if (ITEM_TYPE(item) == xlog_recover_item_ops[i]->item_type)
+ if (item_type == xlog_recover_item_ops[i]->item_type)
return xlog_recover_item_ops[i];
return NULL;
@@ -1888,14 +1888,13 @@ xlog_find_item_ops(
* but for all other items there may be specific ordering that we need to
* preserve.
*/
-STATIC int
+STATIC void
xlog_recover_reorder_trans(
struct xlog *log,
struct xlog_recover *trans,
int pass)
{
struct xlog_recover_item *item, *n;
- int error = 0;
LIST_HEAD(sort_list);
LIST_HEAD(cancel_list);
LIST_HEAD(buffer_list);
@@ -1906,22 +1905,6 @@ xlog_recover_reorder_trans(
list_for_each_entry_safe(item, n, &sort_list, ri_list) {
enum xlog_recover_reorder fate = XLOG_REORDER_ITEM_LIST;
- item->ri_ops = xlog_find_item_ops(item);
- if (!item->ri_ops) {
- xfs_warn(log->l_mp,
- "%s: unrecognized type of log operation (%d)",
- __func__, ITEM_TYPE(item));
- ASSERT(0);
- /*
- * return the remaining items back to the transaction
- * item list so they can be freed in caller.
- */
- if (!list_empty(&sort_list))
- list_splice_init(&sort_list, &trans->r_itemq);
- error = -EFSCORRUPTED;
- break;
- }
-
if (item->ri_ops->reorder)
fate = item->ri_ops->reorder(item);
@@ -1954,7 +1937,6 @@ xlog_recover_reorder_trans(
list_splice_tail(&inode_buffer_list, &trans->r_itemq);
if (!list_empty(&cancel_list))
list_splice_tail(&cancel_list, &trans->r_itemq);
- return error;
}
void
@@ -2039,9 +2021,7 @@ xlog_recover_commit_trans(
hlist_del_init(&trans->r_list);
- error = xlog_recover_reorder_trans(log, trans, pass);
- if (error)
- return error;
+ xlog_recover_reorder_trans(log, trans, pass);
list_for_each_entry_safe(item, next, &trans->r_itemq, ri_list) {
trace_xfs_log_recover_item_recover(log, trans, item, pass);
@@ -2130,6 +2110,10 @@ xlog_recover_add_to_cont_trans(
item = list_entry(trans->r_itemq.prev, struct xlog_recover_item,
ri_list);
+ /* the continuation has to extend a region we have already started */
+ if (XFS_IS_CORRUPT(log->l_mp, item->ri_cnt == 0 || !item->ri_buf))
+ return -EFSCORRUPTED;
+
old_ptr = item->ri_buf[item->ri_cnt-1].iov_base;
old_len = item->ri_buf[item->ri_cnt-1].iov_len;
@@ -2143,6 +2127,43 @@ xlog_recover_add_to_cont_trans(
return 0;
}
+/*
+ * Validate a newly decoded item header before recovery trusts it to size and
+ * assemble the item's regions: resolve the item type and bound the declared
+ * region count.
+ */
+STATIC int
+xlog_recover_verify_item_header(
+ struct xlog *log,
+ struct xlog_recover_item *item,
+ char *ptr,
+ int len)
+{
+ struct xfs_inode_log_format *in_f = (struct xfs_inode_log_format *)ptr;
+ const struct xlog_recover_item_ops *ops;
+ unsigned int max_regions;
+
+ /* The type and region count fields have to be present to be read. */
+ if (XFS_IS_CORRUPT(log->l_mp,
+ len < offsetofend(struct xfs_inode_log_format, ilf_size)))
+ return -EFSCORRUPTED;
+
+ ops = xlog_find_item_ops(in_f->ilf_type);
+ if (XFS_IS_CORRUPT(log->l_mp, !ops))
+ return -EFSCORRUPTED;
+ item->ri_ops = ops;
+
+ max_regions = ops->max_regions ? ops->max_regions :
+ XLOG_MAX_REGIONS_IN_ITEM;
+ if (XFS_IS_CORRUPT(log->l_mp,
+ in_f->ilf_size == 0 ||
+ in_f->ilf_size < ops->min_regions ||
+ in_f->ilf_size > max_regions))
+ return -EFSCORRUPTED;
+
+ 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
@@ -2166,6 +2187,7 @@ xlog_recover_add_to_trans(
struct xfs_inode_log_format *in_f; /* any will do */
struct xlog_recover_item *item;
char *ptr;
+ int error;
if (!len)
return 0;
@@ -2211,14 +2233,10 @@ xlog_recover_add_to_trans(
}
if (item->ri_total == 0) { /* first region to be added */
- if (in_f->ilf_size == 0 ||
- in_f->ilf_size > XLOG_MAX_REGIONS_IN_ITEM) {
- xfs_warn(log->l_mp,
- "bad number of regions (%d) in inode log format",
- in_f->ilf_size);
- ASSERT(0);
+ error = xlog_recover_verify_item_header(log, item, ptr, len);
+ if (error) {
kvfree(ptr);
- return -EFSCORRUPTED;
+ return error;
}
item->ri_total = in_f->ilf_size;
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index 8bccf89a7766..ecd2be4d425b 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -741,6 +741,8 @@ xlog_recover_cui_commit_pass2(
const struct xlog_recover_item_ops xlog_cui_item_ops = {
.item_type = XFS_LI_CUI,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_cui_commit_pass2,
};
@@ -796,6 +798,8 @@ xlog_recover_rtcui_commit_pass2(
const struct xlog_recover_item_ops xlog_rtcui_item_ops = {
.item_type = XFS_LI_CUI_RT,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_rtcui_commit_pass2,
};
@@ -828,6 +832,8 @@ xlog_recover_cud_commit_pass2(
const struct xlog_recover_item_ops xlog_cud_item_ops = {
.item_type = XFS_LI_CUD,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_cud_commit_pass2,
};
@@ -858,5 +864,7 @@ xlog_recover_rtcud_commit_pass2(
const struct xlog_recover_item_ops xlog_rtcud_item_ops = {
.item_type = XFS_LI_CUD_RT,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_rtcud_commit_pass2,
};
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index 2a3a73a8566d..201c8cea74f5 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -770,6 +770,8 @@ xlog_recover_rui_commit_pass2(
const struct xlog_recover_item_ops xlog_rui_item_ops = {
.item_type = XFS_LI_RUI,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_rui_commit_pass2,
};
@@ -825,6 +827,8 @@ xlog_recover_rtrui_commit_pass2(
const struct xlog_recover_item_ops xlog_rtrui_item_ops = {
.item_type = XFS_LI_RUI_RT,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_rtrui_commit_pass2,
};
@@ -857,6 +861,8 @@ xlog_recover_rud_commit_pass2(
const struct xlog_recover_item_ops xlog_rud_item_ops = {
.item_type = XFS_LI_RUD,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_rud_commit_pass2,
};
@@ -887,5 +893,7 @@ xlog_recover_rtrud_commit_pass2(
const struct xlog_recover_item_ops xlog_rtrud_item_ops = {
.item_type = XFS_LI_RUD_RT,
+ .max_regions = 1,
+ .min_regions = 1,
.commit_pass2 = xlog_recover_rtrud_commit_pass2,
};
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v3 2/3] xfs: verify recovered log items are complete before replaying them
2026-07-19 11:29 [PATCH v3 0/3] xfs: add a log item verification layer to recovery Weiming Shi
2026-07-19 11:29 ` [PATCH v3 1/3] xfs: verify log item headers when they are decoded during recovery Weiming Shi
@ 2026-07-19 11:29 ` Weiming Shi
2026-07-19 11:29 ` [PATCH v3 3/3] xfs: add an inode log item recovery verifier Weiming Shi
2 siblings, 0 replies; 4+ messages in thread
From: Weiming Shi @ 2026-07-19 11:29 UTC (permalink / raw)
To: Carlos Maiolino, Darrick J . Wong
Cc: linux-xfs, linux-kernel, xmei5, Weiming Shi, Dave Chinner
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@asu.edu>
Suggested-by: Dave Chinner <dgc@kernel.org>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v3 3/3] xfs: add an inode log item recovery verifier
2026-07-19 11:29 [PATCH v3 0/3] xfs: add a log item verification layer to recovery Weiming Shi
2026-07-19 11:29 ` [PATCH v3 1/3] xfs: verify log item headers when they are decoded during recovery Weiming Shi
2026-07-19 11:29 ` [PATCH v3 2/3] xfs: verify recovered log items are complete before replaying them Weiming Shi
@ 2026-07-19 11:29 ` Weiming Shi
2 siblings, 0 replies; 4+ messages in thread
From: Weiming Shi @ 2026-07-19 11:29 UTC (permalink / raw)
To: Carlos Maiolino, Darrick J . Wong
Cc: linux-xfs, linux-kernel, xmei5, Weiming Shi, Dave Chinner
The previous patches let each log item type validate its own formatted
structures through an xlog_recover_item_ops->verify method, run once the
item is fully decoded. Add the first verifier, for inode items.
Log recovery previously validated a recovered inode item's structures
inside the pass2 decode and replay code, one open-coded check at a time,
which was hard to read and to audit for what was still unchecked.
xlog_recover_inode_verify() instead checks in one place that the core and
each fork region implied by ilf_fields is declared, that the log dinode is
present and large enough, that its version matches the mount, that
di_forkoff is within the literal area, and that the verbatim-copied fork
regions fit their destination fork.
Because the log dinode checks now run before pass2, drop the equivalent
open-coded checks (the log dinode magic and the dead di_forkoff bound)
from xlog_recover_inode_commit_pass2(). The checks that need the on-disk
inode buffer (its magic, the LSN and di_flushiter replay-ordering
decisions, the di_mode/di_format consistency, and the final
xfs_dinode_verify()) cannot be hoisted and stay in pass2.
This covers the self-contained log dinode structure. The btree-root fork
formats are converted from a larger in-core form on replay and their
record count is not bounded here yet; clamping xfs_bmbt_to_bmdr() and the
rt btree converters against the destination fork is left as follow-up.
Suggested-by: Dave Chinner <dgc@kernel.org>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
fs/xfs/xfs_inode_item_recover.c | 87 +++++++++++++++++++++++++++------
1 file changed, 71 insertions(+), 16 deletions(-)
diff --git a/fs/xfs/xfs_inode_item_recover.c b/fs/xfs/xfs_inode_item_recover.c
index 6b6ac92964d0..8308f064cecc 100644
--- a/fs/xfs/xfs_inode_item_recover.c
+++ b/fs/xfs/xfs_inode_item_recover.c
@@ -367,13 +367,6 @@ xlog_recover_inode_commit_pass2(
goto out_release;
}
ldip = item->ri_buf[1].iov_base;
- if (XFS_IS_CORRUPT(mp, ldip->di_magic != XFS_DINODE_MAGIC)) {
- xfs_alert(mp,
- "%s: Bad inode log record, rec ptr "PTR_FMT", ino %lld",
- __func__, item, in_f->ilf_ino);
- error = -EFSCORRUPTED;
- goto out_release;
- }
/*
* If the inode has an LSN in it, recover the inode only if the on-disk
@@ -462,15 +455,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,
@@ -597,10 +581,81 @@ xlog_recover_inode_commit_pass2(
return error;
}
+/*
+ * Validate the log dinode and fork regions of a decoded inode item. Checks
+ * that need the on-disk inode buffer stay in xlog_recover_inode_commit_pass2().
+ */
+STATIC int
+xlog_recover_inode_verify(
+ struct xlog *log,
+ struct xlog_recover_item *item)
+{
+ struct xfs_mount *mp = log->l_mp;
+ struct xfs_inode_log_format *in_f;
+ struct xfs_inode_log_format in_f_buf;
+ struct xfs_log_dinode *ldip;
+ unsigned int litino = XFS_LITINO(mp);
+ unsigned int dsize, asize;
+ int attr_index;
+ int error;
+
+ if (item->ri_buf[0].iov_len == sizeof(struct xfs_inode_log_format)) {
+ in_f = item->ri_buf[0].iov_base;
+ } else {
+ in_f = &in_f_buf;
+ error = xfs_inode_item_format_convert(&item->ri_buf[0], in_f);
+ if (error)
+ return error;
+ }
+
+ /* The inode core is always logged as the log dinode in ri_buf[1]. */
+ if (XFS_IS_CORRUPT(mp, in_f->ilf_size < 2) ||
+ XFS_IS_CORRUPT(mp,
+ item->ri_buf[1].iov_len < xfs_log_dinode_size(mp)))
+ return -EFSCORRUPTED;
+
+ ldip = item->ri_buf[1].iov_base;
+ if (XFS_IS_CORRUPT(mp, ldip->di_magic != XFS_DINODE_MAGIC) ||
+ XFS_IS_CORRUPT(mp, !xfs_dinode_good_version(mp, ldip->di_version)) ||
+ XFS_IS_CORRUPT(mp, ldip->di_forkoff >= (litino >> 3)))
+ return -EFSCORRUPTED;
+
+ if (ldip->di_forkoff) {
+ dsize = ldip->di_forkoff << 3;
+ asize = litino - (ldip->di_forkoff << 3);
+ } else {
+ dsize = litino;
+ asize = 0;
+ }
+
+ /*
+ * Btree-root forks are logged in a larger in-core form and converted on
+ * replay, so their region is not bounded by the on-disk fork size here.
+ */
+ if (in_f->ilf_fields & XFS_ILOG_DFORK) {
+ if (XFS_IS_CORRUPT(mp, in_f->ilf_size < 3))
+ return -EFSCORRUPTED;
+ if ((in_f->ilf_fields & XFS_ILOG_DFORK) != XFS_ILOG_DBROOT &&
+ XFS_IS_CORRUPT(mp, item->ri_buf[2].iov_len > dsize))
+ return -EFSCORRUPTED;
+ }
+ if (in_f->ilf_fields & XFS_ILOG_AFORK) {
+ attr_index = (in_f->ilf_fields & XFS_ILOG_DFORK) ? 3 : 2;
+ if (XFS_IS_CORRUPT(mp, in_f->ilf_size < attr_index + 1))
+ return -EFSCORRUPTED;
+ if ((in_f->ilf_fields & XFS_ILOG_AFORK) != XFS_ILOG_ABROOT &&
+ XFS_IS_CORRUPT(mp, item->ri_buf[attr_index].iov_len > asize))
+ return -EFSCORRUPTED;
+ }
+
+ return 0;
+}
+
const struct xlog_recover_item_ops xlog_inode_item_ops = {
.item_type = XFS_LI_INODE,
.max_regions = 4,
.min_regions = 2,
+ .verify = xlog_recover_inode_verify,
.ra_pass2 = xlog_recover_inode_ra_pass2,
.commit_pass2 = xlog_recover_inode_commit_pass2,
};
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread