[PATCH v3 0/3] xfs: add a log item verification layer to recovery

[Weiming Shi]

Log recovery rebuilds each log item from the ophdr regions in the journal
and then casts those regions back to their format structures to drive
replay. The region layout it trusts to do this - the item type, the region
count, and the region sizes - all come from the log, so a crafted or
corrupt image can present a malformed item that recovery dereferences.
The reported case is an XFS_LI_INODE item that declares two regions but
logs only one, leaving ri_buf[1] NULL and faulting mount-time recovery in
xlog_recover_inode_commit_pass2().

v2 rejected an item whose logged region count did not reach the declared
count with a single check in xlog_recover_commit_trans(). As Dave pointed
out, that only found the one missing check and left the comprehensive
verification of the recovered item structure undone, at the wrong layer.
v3 instead verifies each item where it is first decoded from the log, in
xlog_recover_add_to_trans(), before that data is used to size, build,
decode or replay anything:

  - When an item header is decoded, resolve the item type and bound the
    region count it declares within the minimum and maximum a log item of
    that type is formatted with. The upper bound keeps the region array
    from being oversized; the lower bound stops an item that declares fewer
    regions than its replay code indexes, e.g. a dquot with qlf_size == 1
    that still reaches ri_buf[1]. Reject a continuation that has no region
    to extend.

  - When an item is fully decoded (the next item's header arrives, or the
    commit record does), confirm it received all its declared regions and
    run the item type's verifier, if it has one.

  - Add the first per-type verifier, for inode items, and drop the
    equivalent open-coded checks that were scattered through the inode
    pass2 path.

The generic checks bound and complete the region layout - the item type,
the region count within a per-type [min, max], and that every declared
region arrived - for every item type. The per-region size and structure
checks are type-specific and live in the ->verify method; this series adds
the inode verifier, and the other types can grow their own the same way.

Not addressed here are the ->verify checks for the remaining types,
including the non-header region size checks and the content-driven indices
that the region count alone does not bound. Two such cases are known: the
btree-root fork formats convert from a larger in-core form on replay, so
their region size does not bound bb_numrecs in xfs_bmbt_to_bmdr() and the
rt converters; and buffer replay walks ri_buf[] by the blf_data_map bit
runs rather than by the region count. A buffer ->verify and a bb_numrecs
clamp are left as follow-ups.

v3:
  - Reworked from the single completeness check in
    xlog_recover_commit_trans() into a verification layer at the
    region-assembly boundary in xlog_recover_add_to_trans(), as Dave
    suggested: a header check at decode time (item type known, region count
    within [min, max]) and a completeness/structure check when the item is
    fully decoded, with per-type verifiers hanging off
    xlog_recover_item_ops. Added the inode verifier and a continuation
    underflow guard.

v2:
  - Dropped the piecemeal per-check validation in the inode pass2 path.

Weiming Shi (3):
  xfs: verify log item headers when they are decoded during recovery
  xfs: verify recovered log items are complete before replaying them
  xfs: add an inode log item recovery verifier

 fs/xfs/libxfs/xfs_log_recover.h |  17 +++++
 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 |  89 ++++++++++++++++++-----
 fs/xfs/xfs_log_recover.c        | 121 +++++++++++++++++++++++---------
 fs/xfs/xfs_refcount_item.c      |   8 +++
 fs/xfs/xfs_rmap_item.c          |   8 +++
 11 files changed, 221 insertions(+), 48 deletions(-)


base-commit: 8c13415c8a4383447c21ec832b20b3b283f0e01a
-- 
2.43.0