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

Hongling Zeng posted 1 patch 1 month, 1 week ago
There is a newer version of this series
fs/xfs/xfs_buf_item_recover.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
[PATCH] xfs: fix array bounds checking in log recovery
Posted by Hongling Zeng 1 month, 1 week ago
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>
---
 fs/xfs/xfs_buf_item_recover.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
index 123456789abc..defghijklmnop 100644
--- a/fs/xfs/xfs_buf_item_recover.c
+++ b/fs/xfs/xfs_buf_item_recover.c
@@ -483,6 +483,14 @@ xlog_recover_do_reg_buffer(
 			if (bit == -1)
 				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);
@@ -687,6 +695,16 @@ 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