* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.