Re: [PATCH 02/48] xfs: add log recovery item validation design document

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Oct 13, 2026 at 05:00:52PM +1100, Dave Chinner wrote:
> Add a design document describing a systematic approach to validating log
> items during journal recovery. Log item data read from the journal cannot
> be fully trusted - corruption from torn writes, hardware errors or
> software bugs can produce invalid type codes, wrong region counts,
> truncated regions or internally inconsistent format structures that the
> current recovery code largely uses unchecked.
> 
> The document describes a three-layer validation scheme (generic region
> header decode, per-region type-specific validation, and full cross-region
> item validation), the treatment of the transaction header as a validated
> region type, and a set of additional safety fixes for the generic region
> assembly code. It also lays out a phased implementation plan.
> 
> Changes:
> - Add Documentation/filesystems/xfs/xfs-log-recovery-validation-design.rst
> - Add the new document to the XFS documentation index
> 
> Created-by-AI: Claude Opus 4.8 (1M context)
>
> Prompt: Let's start by converting the existing verification doc to kernel
> .rst format and moving it to Documentation/filesystems/xfs/ and adding it
> to the relevant doc index file. The file should be tracked in a new patch.

Eheheh.

> Refinements:
> - Reordered the Phase 1 implementation plan so the ops-struct validation
>   infrastructure (min_regions/max_regions/min_hdr_len fields and the
>   validate_region callback) is added first (Patch 1), followed by the
>   transaction header ops entry that uses it (Patch 2). Downstream patches
>   renumbered accordingly so each patch builds cleanly on its own.
> ---
>  Documentation/filesystems/xfs/index.rst       |   1 +
>  .../xfs-log-recovery-validation-design.rst    | 477 ++++++++++++++++++
>  2 files changed, 478 insertions(+)
>  create mode 100644 Documentation/filesystems/xfs/xfs-log-recovery-validation-design.rst
> 
> diff --git a/Documentation/filesystems/xfs/index.rst b/Documentation/filesystems/xfs/index.rst
> index ab66c57a5d18..231c1382b27a 100644
> --- a/Documentation/filesystems/xfs/index.rst
> +++ b/Documentation/filesystems/xfs/index.rst
> @@ -9,6 +9,7 @@ XFS Filesystem Documentation
>     :numbered:
>  
>     xfs-delayed-logging-design
> +   xfs-log-recovery-validation-design
>     xfs-maintainer-entry-profile
>     xfs-self-describing-metadata
>     xfs-online-fsck-design
> diff --git a/Documentation/filesystems/xfs/xfs-log-recovery-validation-design.rst b/Documentation/filesystems/xfs/xfs-log-recovery-validation-design.rst
> new file mode 100644
> index 000000000000..188b1bebf171
> --- /dev/null
> +++ b/Documentation/filesystems/xfs/xfs-log-recovery-validation-design.rst

The document sounds pretty reasonable for a first pass at hardening the
log recovery code.  This is a good start.

<snip>

> +Layer 3: Full item validation (type-specific, cross-region)
> +-----------------------------------------------------------

<snip>

> +Other intent/done items
> +    similar field-level validation

But please be more specific.

> +ICREATE
> +    - Already has validation in
> +      ``xlog_recover_icreate_commit_pass2()``, move the checks to
> +      ``validate_item``
> +
> +QUOTAOFF
> +    - Verify ``qf_flags`` contains only valid quota flags
> +

<snip>

> +Phase 3: Full item validation
> +-----------------------------
> +
> +Patch M: Add validate_item callback infrastructure
> +    - Add the callback to the ops struct
> +    - Call it from ``xlog_recover_reorder_trans()`` after ops verification
> +    - Return ``-EFSCORRUPTED`` on failure, aborting recovery
> +
> +Patches M+1 to M+N: Implement validate_item for each item type
> +    - Move existing validation out of ``commit_pass2`` into
> +      ``validate_item`` where possible (e.g. ICREATE field checks)
> +    - Add new cross-region validation
> +    - Order: simple types first, complex types last
> +    - INODE validation is the most complex (fork format vs region size)
> +
> +Phase 4: Cleanup
> +----------------
> +
> +- Remove redundant validation from ``commit_pass1``/``commit_pass2`` that
> +  is now covered by ``validate_region``/``validate_item``
> +- Add ``ASSERT()``\ s in commit handlers to verify validation has run
> +- Review and update error messages for consistency

Why is the plan for splitting scope across M+N patches relevant for a
design document that will live in the kernel?

--D