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

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

[Henry Martin]

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.

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;
 	fkp = xfs_bmbt_key_addr(mp, rblock, 1);
 	tkp = xfs_bmdr_key_addr(dblock, 1);
 	fpp = xfs_bmap_broot_ptr_addr(mp, rblock, 1, rblocklen);
diff --git a/fs/xfs/libxfs/xfs_rtrefcount_btree.c b/fs/xfs/libxfs/xfs_rtrefcount_btree.c
index dcc89b8e149b0..c6b351b7fbcdb 100644
--- a/fs/xfs/libxfs/xfs_rtrefcount_btree.c
+++ b/fs/xfs/libxfs/xfs_rtrefcount_btree.c
@@ -696,6 +696,19 @@ xfs_rtrefcountbt_to_disk(
 	dblock->bb_level = rblock->bb_level;
 	dblock->bb_numrecs = rblock->bb_numrecs;
 
+	/* Corrupted log block: refuse the conversion instead of copying
+	 * past the end of the inode fork area.
+	 */
+	numrecs = be16_to_cpu(rblock->bb_numrecs);
+	maxrecs = xfs_rtrefcountbt_droot_maxrecs(dblocklen,
+						 be16_to_cpu(rblock->bb_level) == 0);
+	if (numrecs > maxrecs) {
+		xfs_warn(mp, "rtrefcountbt log recovery: numrecs %u > max %u",
+			 numrecs, maxrecs);
+		memset(dblock, 0, dblocklen);
+		return;
+	}
+
 	if (be16_to_cpu(rblock->bb_level) > 0) {
 		maxrecs = xfs_rtrefcountbt_droot_maxrecs(dblocklen, false);
 		fkp = xfs_rtrefcount_key_addr(rblock, 1);
diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.c b/fs/xfs/libxfs/xfs_rtrmap_btree.c
index a15e460a1ec7f..29d948b42af66 100644
--- a/fs/xfs/libxfs/xfs_rtrmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rtrmap_btree.c
@@ -929,6 +929,18 @@ xfs_rtrmapbt_to_disk(
 	dblock->bb_numrecs = rblock->bb_numrecs;
 	numrecs = be16_to_cpu(rblock->bb_numrecs);
 
+	/* Corrupted log block: refuse the conversion instead of copying
+	 * past the end of the inode fork area.
+	 */
+	maxrecs = xfs_rtrmapbt_droot_maxrecs(dblocklen,
+					     be16_to_cpu(rblock->bb_level) == 0);
+	if (numrecs > maxrecs) {
+		xfs_warn(mp, "rtrmapbt log recovery: numrecs %u > max %u",
+			 numrecs, maxrecs);
+		memset(dblock, 0, dblocklen);
+		return;
+	}
+
 	if (be16_to_cpu(rblock->bb_level) > 0) {
 		maxrecs = xfs_rtrmapbt_droot_maxrecs(dblocklen, false);
 		fkp = xfs_rtrmap_key_addr(rblock, 1);
-- 
2.43.7

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

[Dave Chinner]

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

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

[Jonathan Corbet]

Dave Chinner <dgc@kernel.org> writes:

> 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.

Just to be obnoxious...this runs counter to the advice we have in
Documentation/process/security-bugs.rst, where reporters are requested
to include a proposed fix as well.  It seems we're sending a bit of a
mixed message...

jon

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

[Dave Chinner]

On Wed, Oct 28, 2026 at 04:43:37PM -0600, Jonathan Corbet wrote:
> Dave Chinner <dgc@kernel.org> writes:
> 
> > 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.
> 
> Just to be obnoxious...this runs counter to the advice we have in
> Documentation/process/security-bugs.rst, where reporters are requested
> to include a proposed fix as well.  It seems we're sending a bit of a
> mixed message...

These aren't security bugs. Bugs that result from maliciously
corrupted filesystem images (i.e. fuzzed) are explicitly stated as
not covered by the kernel security threat models. IOWs, they are
just -bugs-, and bugs that just don't ever happen in the real world.

The journal records are CRC'd and location stamped, so any sort of
real world storage error (e.g.  media corruption, misdirected write,
etc) will already be caught and rejected. Hence to get to these
issues, someone has to corrupt the journal stuctures and manually
rewrite the CRCs to get them to pass verification before we are
exposed to any of these issues.

Really, though, as someone who has learnt how to make LLMs generate
decent code, I'm disappointed in how frequently I'm to tell random
people using LLMs that their code is complete shite. LLMs don't
generate bad code when they are well directed by a human.  Bad code
is a sign of a poor coder (i.e. a human failing), regardless of
whether an LLM generated the code or not.

However, regardless of the skill and capability of the human driving
the LLM, LLMs will still generate decent bug reports. I'll take a
good bug report over bad code every day of the week.

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

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

[Theodore Tso]

On Thu, Oct 29, 2026 at 10:34:44AM -0500, Dave Chinner wrote:
> These aren't security bugs. Bugs that result from maliciously
> corrupted filesystem images (i.e. fuzzed) are explicitly stated as
> not covered by the kernel security threat models. IOWs, they are
> just -bugs-, and bugs that just don't ever happen in the real world.

Well, there are people/companies who might be silly/stupid enough to
think that containers are a real security isolation layer[1], and who
think that they can let random users mount untrusted file systems in a
container, and not have to worry about one company gaining access to
PII belonging to another company because both companies are using a
particular cloud hosting provider which uses containers and allows
their customers to mount file systems as (container) root, with
multiple customers sharing the some physical host.

As Dave has said, *all* of the file system maintainers have explicitly
disclaimed this security model; from our perspective the block device
is inside the trusted computing base (TCB), and if someone wants to
dedicaste one or more full-time engineers to be file system developers
who have contributed a large number of improvements to the file
system, and who understand the context that they can competently drive
LLM's to create architecturally sound patches, I *might* be willing to
change my stance for ext4.  But those hosting providers' engineers
would have do most of the work to try to make this security model work
--- and it might be that using fuse2fs, and running that fuse server
in a sandbox (e.g. seccomp-bpf, etc.) might be a much simpler /
cheaper solution in the end.

Cheers,

					- Ted

[1] There's a *reason* why Google Kubernetes Engine (GKE) uses VM's
and not containers as a security isolation layer.  :-)