[PATCH] xfs: bound logged region access in inode buffer recovery

Hongling Zeng <[email protected]>
Newsgroups org.kernel.vger.linux-xfs,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>
xlog_recover_do_inode_buffer() reads the logged di_next_unlinked field
from a log record buffer at a computed offset:

	logged_nextp = item->ri_buf[item_index].iov_base +
			next_unlinked_offset - reg_buf_offset;
	*buffer_nextp = *logged_nextp;

The only protection against reading past the log record buffer are
ASSERT()s, which compile away on non-DEBUG kernels.  The existing
XFS_IS_CORRUPT(*logged_nextp == 0) check also dereferences the pointer
before validating that the computed offset lies within the logged region.

A crafted log record can make the computed offset exceed iov_len, causing
an out-of-bounds read from the log record buffer during inode buffer
recovery.

Convert the relevant ASSERT-only checks into runtime corruption checks and
verify that the logged di_next_unlinked field lies entirely within the log
iovec before dereferencing it.

Fixes: 1094d3f12363 ("xfs: refactor log recovery buffer item dispatch for pass2 commit functions")
Cc: [email protected]
Signed-off-by: Hongling Zeng <[email protected]>
---
 fs/xfs/xfs_buf_item_recover.c | 40 ++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
index 240deb3f7827..bfaf20b8e48d 100644
--- a/fs/xfs/xfs_buf_item_recover.c
+++ b/fs/xfs/xfs_buf_item_recover.c
@@ -639,6 +639,9 @@ xlog_recover_do_inode_buffer(
 	int				inodes_per_buf;
 	xfs_agino_t			*logged_nextp;
 	xfs_agino_t			*buffer_nextp;
+	size_t				buf_size;
+	size_t				iov_len;
+	size_t				rel_off;
 
 	trace_xfs_log_recover_buf_inode_buf(mp->m_log, buf_f);
 
@@ -689,17 +692,48 @@ xlog_recover_do_inode_buffer(
 		if (next_unlinked_offset < reg_buf_offset)
 			continue;
 
+		buf_size = BBTOB(bp->b_length);
+		if (XFS_IS_CORRUPT(mp, reg_buf_bytes > buf_size ||
+			   reg_buf_offset > buf_size - reg_buf_bytes)) {
+			xfs_alert(mp,
+	"Bad inode buffer log bitmap region (off %d, len %d, buf_size %zu).",
+				reg_buf_offset, reg_buf_bytes, buf_size);
+			return -EFSCORRUPTED;
+		}
+
+		if (XFS_IS_CORRUPT(mp,
+				item->ri_buf[item_index].iov_base == NULL)) {
+			xfs_alert(mp, "NULL inode buffer log record.");
+			return -EFSCORRUPTED;
+		}
+
+		iov_len = item->ri_buf[item_index].iov_len;
+		if (XFS_IS_CORRUPT(mp, iov_len < reg_buf_bytes)) {
+			xfs_alert(mp,
+	"Bad inode buffer log record length (iov_len %zu, region len %d).",
+				iov_len, reg_buf_bytes);
+			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));
 
+		rel_off = next_unlinked_offset - reg_buf_offset;
+		if (XFS_IS_CORRUPT(mp, rel_off > iov_len ||
+			   sizeof(xfs_agino_t) > iov_len - rel_off)) {
+			xfs_alert(mp,
+	"Bad inode buffer log record offset (rel_off %zu, iov_len %zu).",
+				rel_off, iov_len);
+			return -EFSCORRUPTED;
+		}
+
 		/*
 		 * The current logged region contains a copy of the
 		 * current di_next_unlinked field.  Extract its value
-		 * and copy it to the buffer copy.
+		 * and copy it to the on disk inode buffer.
 		 */
-		logged_nextp = item->ri_buf[item_index].iov_base +
-				next_unlinked_offset - reg_buf_offset;
+		logged_nextp = item->ri_buf[item_index].iov_base + rel_off;
 		if (XFS_IS_CORRUPT(mp, *logged_nextp == 0)) {
 			xfs_alert(mp,
 		"Bad inode buffer log record (ptr = "PTR_FMT", bp = "PTR_FMT"). "
-- 
2.25.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.