Linux XFS filesystem development
 help / color / mirror / Atom feed
From: Carlos Maiolino <cem@kernel.org>
To: Guikun Yang <yangguikun19@qq.com>
Cc: "Darrick J. Wong" <djwong@kernel.org>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH] xfs: check the region index against ri_total in buffer
Date: Mon, 9 Nov 2026 18:09:51 +0100	[thread overview]
Message-ID: <avH-JslaJ4xlof4c@cronus.toxiclabs.cc> (raw)
In-Reply-To: <tencent_ADBE1F465A9C5A4CFBA28A7D6AA39C527509@qq.com>

On Tue, Nov 10, 2026 at 02:00:00PM +0800, Guikun Yang wrote:
> Both xlog_recover_do_reg_buffer() and xlog_recover_do_inode_buffer()
> walk the buffer data map run by run and index item->ri_buf[] with a
> counter that is advanced once per run. The number of runs in the
> on-disk bitmap has no relationship to the number of regions the log
> item actually carries (ri_total, from blf_size), and the only bound
> on the index is an ASSERT - either the mid-loop ASSERT on ri_buf[i]
> or the trailing ASSERT(i == item->ri_total) - both of which compile
> away on production kernels.

Great... Your patch does not even apply to the tree...

> 
> A crafted buffer log item with a data map that contains more runs
> than blf_size claims regions walks the index past the end of ri_buf[]
> and reads ri_buf[i].iov_base out of bounds. The destination side of
> do_reg_buffer() is already validated for real since the CVE-2026-80536
> fix ("... the only thing bounding the copy is an ASSERT(), which
> compiles away on production kernels"); the source index was left to
> the ASSERTs in both functions.
> 
> Validate the index before each ri_buf[] use, in both functions:
> 
>   xlog_recover_do_reg_buffer(): i >= item->ri_total at the top of the
>   loop body, reported as a bad buffer log item.
> 
>   xlog_recover_do_inode_buffer(): item_index >= item->ri_total right
>   after the run-advance loop.
> 
> Tested with two crafted dirty-log images (mount(2) alone): one hits
> do_reg_buffer (16 runs, blf_size 2), one hits do_inode_buffer
> (alternating map, blf_size 2). Both previously produced
> KASAN: slab-out-of-bounds; with this patch both fail recovery with
> the new message, and the unmodified images still mount.
> 
> Reported-by: Guikun Yang <yangguikun19@qq.com>  (bug reports of 2026-11-09, <c40-xfs-inodebuf-item-index-20261109@qq.com> and <c41-xfs-regbuf-run-index-20261109@qq.com>)
> Signed-off-by: Guikun Yang <yangguikun19@qq.com>
> ---
>  fs/xfs/xfs_buf_item_recover.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
> index 1111111..2222222 100644
> --- a/fs/xfs/xfs_buf_item_recover.c
> +++ b/fs/xfs/xfs_buf_item_recover.c
> @@ -479,6 +479,13 @@ xlog_recover_do_reg_buffer(
>  		nbits = xfs_contig_bits(buf_f->blf_data_map,
>  					buf_f->blf_map_size, bit);
>  		ASSERT(nbits > 0);
> +		if (XFS_IS_CORRUPT(mp, i >= item->ri_total)) {
> +			xfs_alert(mp,
> +"Bad buffer log item dirty bitmap (region %d, %d regions) at daddr 0x%llx.",
> +				i, item->ri_total, xfs_buf_daddr(bp));
> +			return -EFSCORRUPTED;
> +		}
>  		ASSERT(item->ri_buf[i].iov_base != NULL);
>  		ASSERT(item->ri_buf[i].iov_len % XFS_BLF_CHUNK == 0);
>  		/*
> @@ -688,6 +695,13 @@ xlog_recover_do_inode_buffer(
>  			nbits = xfs_contig_bits(buf_f->blf_data_map,
>  						buf_f->blf_map_size, bit);
>  			ASSERT(nbits > 0);
> +			if (XFS_IS_CORRUPT(mp, item_index >= item->ri_total)) {
> +				xfs_alert(mp,
> +"Bad inode buffer log item dirty bitmap (region %d, %d regions) at daddr 0x%llx.",
> +					item_index, item->ri_total,
> +					xfs_buf_daddr(bp));
> +				return -EFSCORRUPTED;
> +			}
>  			reg_buf_offset = bit << XFS_BLF_SHIFT;
>  			reg_buf_bytes = nbits << XFS_BLF_SHIFT;
>  			item_index++;
> 
> Fixes: 658fa68b6f34 ("xfs: refactor log recovery buffer item dispatch for pass2 commit functions")
> --
> 2.43.0
> 

  reply	other threads:[~2026-11-09 17:09 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-10  6:00 [PATCH] xfs: check the region index against ri_total in buffer Guikun Yang
2026-11-09 17:09 ` Carlos Maiolino [this message]
2026-11-10 15:30   ` [PATCH v2] xfs: check the region index against ri_total in buffer log item recovery Guikun Yang
2026-11-09 21:41 ` [PATCH] xfs: check the region index against ri_total in buffer Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=avH-JslaJ4xlof4c@cronus.toxiclabs.cc \
    --to=cem@kernel.org \
    --cc=djwong@kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=yangguikun19@qq.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox