Linux XFS filesystem development
 help / color / mirror / Atom feed
* Re: [PATCH] xfs: check the region index against ri_total in buffer
  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
  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
  1 sibling, 1 reply; 4+ messages in thread
From: Carlos Maiolino @ 2026-11-09 17:09 UTC (permalink / raw)
  To: Guikun Yang; +Cc: Darrick J. Wong, linux-xfs

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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs: check the region index against ri_total in buffer
  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
@ 2026-11-09 21:41 ` Dave Chinner
  1 sibling, 0 replies; 4+ messages in thread
From: Dave Chinner @ 2026-11-09 21:41 UTC (permalink / raw)
  To: Guikun Yang; +Cc: Carlos Maiolino, Darrick J. Wong, linux-xfs

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.
> 
> 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(+)

NACK. Not the way to fix these problems.

-- 
Dave Chinner
dgc@kernel.org

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] xfs: check the region index against ri_total in buffer
@ 2026-11-10  6:00 Guikun Yang
  2026-11-09 17:09 ` Carlos Maiolino
  2026-11-09 21:41 ` [PATCH] xfs: check the region index against ri_total in buffer Dave Chinner
  0 siblings, 2 replies; 4+ messages in thread
From: Guikun Yang @ 2026-11-10  6:00 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: Darrick J. Wong, linux-xfs

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.

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2] xfs: check the region index against ri_total in buffer log item recovery
  2026-11-09 17:09 ` Carlos Maiolino
@ 2026-11-10 15:30   ` Guikun Yang
  0 siblings, 0 replies; 4+ messages in thread
From: Guikun Yang @ 2026-11-10 15:30 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: Darrick J. Wong, linux-xfs, linux-kernel

Hi Carlos,

That was my mistake. I have fixed it: the version below is generated
with git diff from a clean tree and applies cleanly. No content
changes.

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.

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.

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>
Signed-off-by: Guikun Yang <yangguikun19@qq.com>
---
 fs/xfs/xfs_buf_item_recover.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
index 57929f115..ea749a8ab 100644
--- a/fs/xfs/xfs_buf_item_recover.c
+++ b/fs/xfs/xfs_buf_item_recover.c
@@ -486,6 +486,12 @@ 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);
 		/*
@@ -675,6 +681,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++;


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-09 21:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox