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

[Hongling Zeng <zhongling0719@126.com>]

在 2026年09月01日 17:49, Carlos Maiolino 写道:
> On Tue, Sep 01, 2026 at 05:39:57PM +0800, Hongling Zeng wrote:
>> 在 2026年09月01日 15:06, Carlos Maiolino 写道:
>>> 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.
>> Hi ,
>> This is the same patch version, but the previous submission failed to
>> apply because my local repository was out of date. I updated my tree and
>> resubmitted the patch with the same fix.
>>
>> Sorry for the confusion.
> So this is not the same version, it's a rebase against top of the tree
> and that should be properly described.
>
> I'll be updating for-next soon this week, please rebase it again on top
> of for-next once I publish it and re-send it.
>
> Cheers,
> Carlos
> |
> |
|Thanks for the clarification.

Got it — this is a rebase, not a new version. I’ll wait for your updated
for-next tree, rebase the fix on top of it, and resend it with an
appropriate changelog.

Sorry for the confusion.

Thanks,
Hongling|
>> Thanks,
>> Hongling
>>>> ---
>>>>    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
>>>>
>>>>
>>