Re: [PATCH] xfs: fix out-of-bounds write from crafted btree root numrecs during log recovery

[Dave Chinner <dgc@kernel.org>]

On Tue, Oct 27, 2026 at 11:17:03PM +0800, Henry Martin wrote:
> xfs_bmbt_to_bmdr() converts a logged in-memory bmap btree root block
> into its on-disk short form, and it trusts the source block's record
> count for the copy sizes:
> 
> 	dblock->bb_level = rblock->bb_level;
> 	dblock->bb_numrecs = rblock->bb_numrecs;
> 	dmxr = xfs_bmdr_maxrecs(dblocklen, 0);
> 	...
> 	dmxr = be16_to_cpu(dblock->bb_numrecs);
> 	memcpy(tkp, fkp, sizeof(*fkp) * dmxr);
> 	memcpy(tpp, fpp, sizeof(*fpp) * dmxr);
> 
> The capacity bound (dmxr from xfs_bmdr_maxrecs) is computed and then
> immediately overwritten with the attacker-controlled numrecs from the
> source block.  During log recovery, xlog_recover_inode_dbroot() and
> the XFS_ILOG_DBROOT/XFS_ILOG_ABROOT paths pass a raw log iovec as the
> source block, so a crafted log item with numrecs up to 0xffff makes the
> two memcpy calls copy up to 512KB each (8 * 65535) out of the log
> buffer and into the inode fork area (a few hundred bytes) and past the
> end of the inode buffer page: a page-granularity out-of-bounds write
> with fully attacker-controlled length and content, before any dinode
> verification runs (xfs_dinode_verify() only sees the result
> afterwards).
> 
> The same pattern exists in xfs_rtrmapbt_to_disk() and
> xfs_rtrefcountbt_to_disk(), which convert rt rmap and refcount btree
> roots from log blocks using the source numrecs unbounded.
> 
> Validate that the source numrecs fits within the destination capacity
> before converting, in all three conversion helpers.
> 
> This issue was discovered by Tencent CodeBuddy Security.

No it wasn't. 

I've already proposed journal recovery validation infrastructure to
knock out all these 'llm plays whack-a-mole-with-corrupted-logs"
bugs once and for all. The first version of that was posted a couple
of weeks ago:

https://lore.kernel.org/linux-xfs/20261013060204.3138690-1-dgc@kernel.org/T/#t

Sashiko flagged this specific bb_numrecs issue in review top the
"[PATCH 41/48] xfs: validate logged btree fork roots during recovery"
two weeks ago:

| The comment above mentions that the region size has already been bounds
| checked against the inode fork size. However, looking at the callers in
| xlog_dinode_verify_dfork() and xlog_dinode_verify_afork(), is this check
| actually performed for the XFS_DINODE_FMT_BTREE case?
|
| If a malicious log provides a large ilfp->ilf_dsize and bb_numrecs, this
| could pass the validation here (since numrecs will match the expanded size),
| but exceed the on-disk fork size.
|
| Could this later result in an out-of-bounds memcpy in xfs_bmbt_to_bmdr()
| when the replayed bb_numrecs exceeds the maximum allowed records for the
| on-disk fork?

So, known issue, and I've already fixed this in my current working
tree. As such, fuzzed garbage like this is rejected before any btree
root is recovered now.

Also:

> Fixes: 658fa68b6f34 ("xfs: refactor log recovery inode item dispatch for pass2 commit functions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_bmap_btree.c       | 13 ++++++++++++-
>  fs/xfs/libxfs/xfs_rtrefcount_btree.c | 13 +++++++++++++
>  fs/xfs/libxfs/xfs_rtrmap_btree.c     | 12 ++++++++++++
>  3 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
> index 758a4b1ccf5bb..118e8ab35f1d1 100644
> --- a/fs/xfs/libxfs/xfs_bmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_bmap_btree.c
> @@ -148,6 +148,7 @@ xfs_bmbt_to_bmdr(
>  	xfs_bmdr_block_t	*dblock,
>  	int			dblocklen)
>  {
> +	int			maxrecs;
>  	int			dmxr;
>  	xfs_bmbt_key_t		*fkp;
>  	__be64			*fpp;
> @@ -167,7 +168,17 @@ xfs_bmbt_to_bmdr(
>  	ASSERT(rblock->bb_level != 0);
>  	dblock->bb_level = rblock->bb_level;
>  	dblock->bb_numrecs = rblock->bb_numrecs;
> -	dmxr = xfs_bmdr_maxrecs(dblocklen, 0);
> +	maxrecs = xfs_bmdr_maxrecs(dblocklen, 0);
> +	if (be16_to_cpu(dblock->bb_numrecs) > maxrecs) {
> +		/* Corrupted log block: refuse the conversion instead of
> +		 * copying past the end of the inode fork area.
> +		 */
> +		xfs_warn(mp, "bmbt log recovery: numrecs %d > max %d",
> +			 be16_to_cpu(dblock->bb_numrecs), maxrecs);
> +		memset(dblock, 0, dblocklen);
> +		return;
> +	}
> +	dmxr = maxrecs;

This is typical of poorly directed LLM code fixes. i.e. Slap a
bandaid on the symptom without understanding how the problem should
be fixed, so it actually makes the problem -worse- rather than
better. But we fixed the out-of-bounds copy, so it must be good!

Unless you know how to fix the discovered problem correctly, don't
send what your LLM proposed for review. It will be garbage, and all
that will do is annoy whoever has to review it. Just report the bug,
let the subsystem developers fix it.

-Dave.
-- 
Dave Chinner
dgc@kernel.org