All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: bound inode buffer log item replay by logged regions
@ 2026-07-29 19:31 Weiming Shi
  2026-07-30  0:06 ` Dave Chinner
  0 siblings, 1 reply; 2+ messages in thread
From: Weiming Shi @ 2026-07-29 19:31 UTC (permalink / raw)
  To: Carlos Maiolino
  Cc: linux-xfs, linux-kernel, Kyle Zeng, Xiang Mei, Brian Foster,
	stable

xlog_recover_do_inode_buffer() advances item_index once for every run of
set bits in the logged dirty bitmap, but ri_buf[] is populated only for
the regions present in the recovered log item.  A crafted inode buffer
log item can therefore make the dirty bitmap describe more runs than the
item has logged regions, and recovery reads past the end of ri_buf[]
before dereferencing the bogus iov_base pointer.

The ASSERT() that checks iov_base compiles away without CONFIG_XFS_DEBUG,
and evaluating it would already perform the out-of-bounds ri_buf[] read.
Check item_index against ri_cnt before the first ri_buf[item_index]
access instead, and fail recovery with -EFSCORRUPTED.

Found with KASAN on a CONFIG_XFS_DEBUG=n build by mounting a crafted XFS
image whose log contains an inode buffer item with more dirty bitmap runs
than logged regions:

 BUG: KASAN: slab-out-of-bounds in xlog_recover_do_inode_buffer (fs/xfs/xfs_buf_item_recover.c:701)
 Read of size 8 at addr ffff88801387d160 by task mount
 The buggy address is located 0 bytes to the right of
  allocated 32-byte region [ffff88801387d140, ffff88801387d160)
  xlog_recover_do_inode_buffer (fs/xfs/xfs_buf_item_recover.c:701)
  xlog_recover_buf_commit_pass2 (fs/xfs/xfs_buf_item_recover.c:1130)
  ...
 Oops: general protection fault, probably for non-canonical address
 RIP: 0010:xlog_recover_do_inode_buffer (fs/xfs/xfs_buf_item_recover.c:703)

After this change the crafted image fails recovery with -EFSCORRUPTED and
the mount is refused.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Kyle Zeng <kylebot@openai.com>
Link: https://lore.kernel.org/linux-xfs/20260611212314.4610-1-kylebot@openai.com/
Reported-by: Xiang Mei <xmei5@asu.edu>
Suggested-by: Brian Foster <bfoster@redhat.com>
Link: https://lore.kernel.org/linux-xfs/ajGB9o5Ys6itU3Rh@bfoster/
Cc: stable@vger.kernel.org
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 fs/xfs/xfs_buf_item_recover.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
index 02b95b89d1b5..2c2d4a0027af 100644
--- a/fs/xfs/xfs_buf_item_recover.c
+++ b/fs/xfs/xfs_buf_item_recover.c
@@ -659,6 +659,13 @@ xlog_recover_do_inode_buffer(
 			reg_buf_offset = bit << XFS_BLF_SHIFT;
 			reg_buf_bytes = nbits << XFS_BLF_SHIFT;
 			item_index++;
+
+			if (XFS_IS_CORRUPT(mp, item_index >= item->ri_cnt)) {
+				xfs_alert(mp,
+	"Bad inode buffer log item dirty bitmap: more dirty regions than the %d logged, for buffer at daddr 0x%llx.",
+					item->ri_cnt - 1, xfs_buf_daddr(bp));
+				return -EFSCORRUPTED;
+			}
 		}
 
 		/*
@@ -669,7 +676,6 @@ xlog_recover_do_inode_buffer(
 		if (next_unlinked_offset < reg_buf_offset)
 			continue;
 
-		ASSERT(item->ri_buf[item_index].iov_base != NULL);
 		ASSERT((item->ri_buf[item_index].iov_len % XFS_BLF_CHUNK) == 0);
 		ASSERT((reg_buf_offset + reg_buf_bytes) <= BBTOB(bp->b_length));
 
-- 
2.43.0


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

* Re: [PATCH] xfs: bound inode buffer log item replay by logged regions
  2026-07-29 19:31 [PATCH] xfs: bound inode buffer log item replay by logged regions Weiming Shi
@ 2026-07-30  0:06 ` Dave Chinner
  0 siblings, 0 replies; 2+ messages in thread
From: Dave Chinner @ 2026-07-30  0:06 UTC (permalink / raw)
  To: Weiming Shi
  Cc: Carlos Maiolino, linux-xfs, linux-kernel, Kyle Zeng, Xiang Mei,
	Brian Foster, stable

On Wed, Jul 29, 2026 at 12:31:27PM -0700, Weiming Shi wrote:
> xlog_recover_do_inode_buffer() advances item_index once for every run of
> set bits in the logged dirty bitmap, but ri_buf[] is populated only for
> the regions present in the recovered log item.  A crafted inode buffer
> log item can therefore make the dirty bitmap describe more runs than the
> item has logged regions, and recovery reads past the end of ri_buf[]
> before dereferencing the bogus iov_base pointer.
> 
> The ASSERT() that checks iov_base compiles away without CONFIG_XFS_DEBUG,
> and evaluating it would already perform the out-of-bounds ri_buf[] read.
> Check item_index against ri_cnt before the first ri_buf[item_index]
> access instead, and fail recovery with -EFSCORRUPTED.
> 
> Found with KASAN on a CONFIG_XFS_DEBUG=n build by mounting a crafted XFS
> image whose log contains an inode buffer item with more dirty bitmap runs
> than logged regions:
> 
>  BUG: KASAN: slab-out-of-bounds in xlog_recover_do_inode_buffer (fs/xfs/xfs_buf_item_recover.c:701)
>  Read of size 8 at addr ffff88801387d160 by task mount
>  The buggy address is located 0 bytes to the right of
>   allocated 32-byte region [ffff88801387d140, ffff88801387d160)
>   xlog_recover_do_inode_buffer (fs/xfs/xfs_buf_item_recover.c:701)
>   xlog_recover_buf_commit_pass2 (fs/xfs/xfs_buf_item_recover.c:1130)
>   ...
>  Oops: general protection fault, probably for non-canonical address
>  RIP: 0010:xlog_recover_do_inode_buffer (fs/xfs/xfs_buf_item_recover.c:703)
> 
> After this change the crafted image fails recovery with -EFSCORRUPTED and
> the mount is refused.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Kyle Zeng <kylebot@openai.com>
> Link: https://lore.kernel.org/linux-xfs/20260611212314.4610-1-kylebot@openai.com/
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Suggested-by: Brian Foster <bfoster@redhat.com>
> Link: https://lore.kernel.org/linux-xfs/ajGB9o5Ys6itU3Rh@bfoster/
> Cc: stable@vger.kernel.org
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
>  fs/xfs/xfs_buf_item_recover.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
> index 02b95b89d1b5..2c2d4a0027af 100644
> --- a/fs/xfs/xfs_buf_item_recover.c
> +++ b/fs/xfs/xfs_buf_item_recover.c
> @@ -659,6 +659,13 @@ xlog_recover_do_inode_buffer(
>  			reg_buf_offset = bit << XFS_BLF_SHIFT;
>  			reg_buf_bytes = nbits << XFS_BLF_SHIFT;
>  			item_index++;
> +
> +			if (XFS_IS_CORRUPT(mp, item_index >= item->ri_cnt)) {
> +				xfs_alert(mp,
> +	"Bad inode buffer log item dirty bitmap: more dirty regions than the %d logged, for buffer at daddr 0x%llx.",
> +					item->ri_cnt - 1, xfs_buf_daddr(bp));
> +				return -EFSCORRUPTED;
> +			}
>  		}

Why are you trying to validate this here instead of via generic log
item validation infrastructure? I thought I already asked for this
sort of thing to be lifted to the initial pass of the log items in
the generic recovery code via per-item type validation methods
instead of being randomly distributed through the recovery code.

Cheers,

Dave.
-- 
Dave Chinner
dgc@kernel.org

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

end of thread, other threads:[~2026-07-30  0:06 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29 19:31 [PATCH] xfs: bound inode buffer log item replay by logged regions Weiming Shi
2026-07-30  0:06 ` 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.