[PATCH 1/2] cxl/mbox: clamp the event record count to the received payload

[Gaobin Huang]

cxl_mem_get_records_log() walks payload->records[] using the record_count
the device wrote into that same payload, and cxl_clear_event_record()
repeats the walk with the same unvalidated value, reading record handles
past the end of the mailbox buffer.  The handle it reads is then handed
back to the device in the Clear Event Records payload.

The command is issued with .size_out = cxl_mbox->payload_size and only

	.min_out = struct_size(payload, records, 0)

enforced, so the count is never compared against what the device actually
returned.  __cxl_pci_mbox_send_cmd() already knows that number: it stores
the bytes it copied into the driver buffer in mbox_cmd.size_out.  Derive
the record bound from it, and pass the validated count to
cxl_clear_event_record() instead of letting it re-read the raw field.

Seen with a QEMU Type-3 device that returns one record in a 2048 byte
buffer while claiming more, so records[16] is the first access outside it:

	BUG: KASAN: slab-out-of-bounds in cxl_clear_event_record+0x1ad/0x2e0
	Read of size 2 at addr ffff888003306834 by task irq/27-0000:35:/58
	 which belongs to the cache kmalloc-2k of size 2048
	The buggy address is located 52 bytes to the right of
	 allocated 2048-byte region

A sweep of the claimed count agrees: 16 stays inside the allocation and 17
does not.  With the clamp in place the same device logs

	Event log '4': device claimed 4096 records but the payload holds 1

and the log is drained normally.

Fixes: 6ebe28f9ec72 ("cxl/mem: Read, trace, and clear events on driver load")
Signed-off-by: Gaobin Huang <huanggaobin23@xxxxxxxxxx>
---
 drivers/cxl/core/mbox.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 55828a836..a8f51bf0f 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -992,11 +992,13 @@ static void __cxl_event_trace_record(struct cxl_memdev *cxlmd,
 
 static int cxl_clear_event_record(struct cxl_memdev_state *mds,
 				  enum cxl_event_log_type log,
-				  struct cxl_get_event_payload *get_pl)
+				  struct cxl_get_event_payload *get_pl,
+				  u16 nr_rec)
 {
 	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
 	struct cxl_mbox_clear_event_payload *payload;
-	u16 total = le16_to_cpu(get_pl->record_count);
+	/* count validated by cxl_mem_get_records_log(), not re-read here */
+	u16 total = nr_rec;
 	u8 max_handles = CXL_CLEAR_EVENT_MAX_HANDLES;
 	size_t pl_size = struct_size(payload, handles, max_handles);
 	struct cxl_mbox_cmd mbox_cmd;
@@ -1070,6 +1072,7 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
 	struct cxl_get_event_payload *payload;
 	u8 log_type = type;
 	u16 nr_rec;
+	size_t max_recs;
 
 	mutex_lock(&mds->event.log_lock);
 	payload = mds->event.buf;
@@ -1093,7 +1096,20 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
 			break;
 		}
 
+		/*
+		 * The record count is device-supplied.  Never walk records[]
+		 * past the payload the device actually returned.
+		 */
+		max_recs = (mbox_cmd.size_out -
+			    offsetof(struct cxl_get_event_payload, records)) /
+			   sizeof(struct cxl_event_record_raw);
 		nr_rec = le16_to_cpu(payload->record_count);
+		if (nr_rec > max_recs) {
+			dev_warn_ratelimited(dev,
+					     "Event log '%d': device claimed %u records but the payload holds %zu\n",
+					     type, nr_rec, max_recs);
+			nr_rec = max_recs;
+		}
 		if (!nr_rec)
 			break;
 
@@ -1104,7 +1120,7 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
 		if (payload->flags & CXL_GET_EVENT_FLAG_OVERFLOW)
 			trace_cxl_overflow(cxlmd, type, payload);
 
-		rc = cxl_clear_event_record(mds, type, payload);
+		rc = cxl_clear_event_record(mds, type, payload, nr_rec);
 		if (rc) {
 			dev_err_ratelimited(dev,
 				"Event log '%d': Failed to clear events : %d",
-- 
2.34.1