Re: [PATCH] xfs: fix array bounds checking in log recovery

[Carlos Maiolino <cem@kernel.org>]

On Mon, Aug 24, 2026 at 05:45:35PM +0800, Hongling Zeng wrote:
> The log recovery code increments array indices based on bits set in
> blf_data_map without verifying that the indices stay within the bounds
> of the item->ri_buf array.
> 
> Since blf_data_map is from untrusted log data, a malicious log could set
> many bits while ri_total is small, causing array index overflow.
> 
> This can result in an out-of-bounds access during log recovery, causing
> a kernel crash or memory corruption.
> 
> This patch adds array bounds checking for ri_buf access in
> xlog_recover_do_reg_buffer() and xlog_recover_do_inode_buffer().
> 
> The check in xlog_recover_do_inode_buffer() is placed right before the
> actual array access, not after incrementing the index, to avoid false
> positives when processing the last valid region.
> 
> Fixes: 1094d3f12363 ("xfs: refactor log recovery buffer item dispatch for pass2 commit functions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

Hi,

is this a re-send? A v2? Please specify why you are sending the same
patche again. This saves a lot of time trying to understand why there
are two patches with the same subject in the list.

> ---
>  fs/xfs/xfs_buf_item_recover.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
> index 57929f115055..7148716366ba 100644
> --- a/fs/xfs/xfs_buf_item_recover.c
> +++ b/fs/xfs/xfs_buf_item_recover.c
> @@ -485,6 +485,19 @@ xlog_recover_do_reg_buffer(
>  			break;
>  		nbits = xfs_contig_bits(buf_f->blf_data_map,
>  					buf_f->blf_map_size, bit);
> +
> +		/*
> +		 * The bitmap can have more bits set than there are regions
> +		 * in ri_buf, so we must check array bounds before using the
> +		 * index to access ri_buf[i].
> +		 */
> +		if (XFS_IS_CORRUPT(mp, i >= item->ri_total)) {
> +			xfs_alert(mp,
> +		"Buffer log item index (%d) exceeds allocated regions (%d).",
> +					i, item->ri_total);
> +			return -EFSCORRUPTED;
> +		}
> +
>  		ASSERT(nbits > 0);
>  		ASSERT(item->ri_buf[i].iov_base != NULL);
>  		ASSERT(item->ri_buf[i].iov_len % XFS_BLF_CHUNK == 0);
> @@ -688,6 +701,19 @@ xlog_recover_do_inode_buffer(
>  		if (next_unlinked_offset < reg_buf_offset)
>  			continue;
>  
> +		/*
> +		 * Check array bounds here (right before accessing ri_buf)
> +		 * rather than after incrementing item_index. This avoids
> +		 * incorrectly rejecting logs when item_index reaches
> +		 * ri_total after processing the final valid region.
> +		 */
> +		if (XFS_IS_CORRUPT(mp, item_index >= item->ri_total)) {
> +			xfs_alert(mp,
> +		"Inode buffer log item index (%d) exceeds allocated regions (%d).",
> +				item_index, item->ri_total);
> +			return -EFSCORRUPTED;
> +		}
> +
>  		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.25.1
> 
>