[PATCH v3 0/4] xfs: close crash window in attr dabtree inactivation

[Long Li <leo.lilong@huawei.com>]

Problem
-------

xfs_attr3_node_inactive() cancels all child blocks in the attr dabtree
via xfs_trans_binval(), relying on a subsequent full attr bmap truncation
in xfs_attr_inactive() to ensure log recovery never reaches the root
node.  The child block cancellations and the bmap truncation are
committed in separate transactions, creating a narrow but real crash
window.

If the system shuts down after the cancellations commit but before the
bmap truncation commits, the attr bmap survives recovery intact and
xlog_recover_process_iunlinks() retries inactivation.  It attempts to
read the root node via the attr bmap: if the root node was not replayed,
reading the unreplayed root block maybe triggers a metadata verification
failure immediately; if it was replayed, following its child pointers to
unreplayed child blocks triggers the same failure.

Zhang Yi previously attempted to solve this problem [1], but Dave gave
different advice. Following the approach suggested by Dave Chinner in
review.

[1] https://patchwork.kernel.org/project/xfs/patch/20230613030434.2944173-3-yi.zhang@huaweicloud.com/

Fix
---

The fix addresses the atomicity gap at two levels:

(1) In xfs_attr3_node_inactive(), each child block invalidation and the
    removal of its reference from the parent node are placed in the same
    transaction, so no parent node ever holds a pointer to a cancelled
    block at any crash point.  After all children are removed, the empty
    root node is converted to an empty leaf in the same transaction.
    This conversion is necessary because xfs_da3_node_verify() rejects
    a node block with count == 0, whereas xfs_attr3_leaf_verify()
    explicitly allows it during recovery to accommodate the transient
    state from the shortform-to-leaf promotion path.

(2) In xfs_attr_inactive(), the attr fork is truncated in two phases.
    First the child extents are removed.  Then the root block is
    invalidated and the attr bmap is truncated to zero in a single
    transaction.  These two operations in the second phase must be
    atomic: as long as the attr bmap has any non-zero length, recovery
    can follow it to the root block, so the root block invalidation must
    commit together with the bmap-to-zero truncation.


Changes
-------

v2->v3:
 1) Rewrite Description of xfs_attr3_node_entry_remove and xfs_attr3_leaf_init.
 2) Fixed corner case issue where root leaf block has remote value block.
 3) Reduced the return exit of xfs_attr3_root_inactive().

v1->v2:
 1) Rename xfs_da3_node_entry_remove to xfs_attr3_node_entry_remove.
 2) Initializing the structure value when declaring a structure in a function.
 3) An assertion is added to xfs_attr3_leaf_init to check whether tp is not null.
 4) Fixed syntax style issues.

Long Li (4):
  xfs: only assert new size for datafork during truncate extents
  xfs: factor out xfs_attr3_node_entry_remove
  xfs: factor out xfs_attr3_leaf_init
  xfs: close crash window in attr dabtree inactivation

 fs/xfs/libxfs/xfs_attr_leaf.c | 22 ++++++++
 fs/xfs/libxfs/xfs_attr_leaf.h |  3 ++
 fs/xfs/libxfs/xfs_da_btree.c  | 53 +++++++++++++++----
 fs/xfs/libxfs/xfs_da_btree.h  |  2 +
 fs/xfs/xfs_attr_inactive.c    | 99 +++++++++++++++++++++--------------
 fs/xfs/xfs_inode.c            |  3 +-
 6 files changed, 130 insertions(+), 52 deletions(-)

-- 
2.39.2