[PATCH v2] cxl/mbox: bound the Get Supported Logs entry count by the payload

[Gaobin Huang]

cxl_enumerate_cmds() iterates gsl->entries entries of gsl->entry[] using
the count the device put in the Get Supported Logs response.  The response
is only checked with .min_out = 2, so a device may report more entries
than it delivered and the driver reads past the end of the buffer.  This
is probe time, so it happens on every boot of a machine with such a
device, without any host action.

cxl_get_gsl() can already tell the caller how much arrived, because
__cxl_pci_mbox_send_cmd() records the copied byte count in
mbox_cmd.size_out.  Derive the number of entries that fit from it and stop
the loop there.
Guard the subtraction too: min_out is smaller than the response header, so
a two byte response would otherwise wrap the size_t arithmetic and leave
the bound with nothing to do.

Like 1/2, this is hardening against a device that does not honour the
protocol rather than a fix for a regression: an honest device never reports
more entries than it returned, so no existing hardware is affected.

Reproduced on the tree this series is based on with a QEMU Type-3 device
that reports 0xffff entries while writing one.  The 2048 byte buffer holds
102 entries, so a claim of 102 is still inside it and 103 is not:

	BUG: KASAN: slab-out-of-bounds in cxl_enumerate_cmds+0x1e1/0x870
	Read of size 4 at addr ffff8880036de810 by task kworker/u8:4/48
	 which belongs to the cache kmalloc-2k of size 2048
	The buggy address is located 16 bytes to the right of

The Read of size 4 is gsl->entry[i].size.  With the bound in place the
same device logs

	GSL: device claimed 65535 entries but the payload holds 1

and enumeration continues with the entries that are present.

This is the cross-check get_supported_features() already performs in
drivers/cxl/core/features.c, where a device supplied count is compared
against the retrieved length before the entries are used.

Signed-off-by: Gaobin Huang <huanggaobin23@xxxxxxxxxx>
---
v1 -> v2.

1/2 of the v1 series (the event record count and cxl_clear_event_record) is
dropped.  Anisa Su posted a fix for the same bug in the same function on
2026-08-31:

  [PATCH v2 2/4] cxl/events: Validate the record count reported by the device
  https://lore.kernel.org/linux-cxl/20260901002912.958-3-anisa.su@xxxxxxxxxxx/

It bounds the count with the same expression (struct_size() against
mbox_cmd.size_out) and carries the same Fixes: commit (6ebe28f9ec72), so this
is a duplicate, and hers is the better of the two: it fails the command rather
than clamping, which also bounds the per-log loop.  Clamping leaves nr_rec
non-zero, so `} while (nr_rec);` keeps issuing Get and Clear Event Records to a
device that never clears them -- a hang, which is worse than the read it fixes.
No reason to post both.

The patch kept here (formerly 2/2) is not in her series: it is the Get
Supported Logs entry count during command enumeration, a different function and
a different response.

Changes from v1 to this patch, from Jonathan Cameron's review:
- drop the Fixes: tag; this is hardening against a device that does not honour
  the protocol, not a fix for a regression, and the commit message says so now
  rather than leaving it to be inferred.
- use struct_offset(gsl, entry) and name it gsl_hdr_size so it is not read as a
  pointer to the header.
- fail the command when the response is shorter than the header instead of
  deriving a zero bound from the subtraction.  That also removes the ternary
  and the underflow it was guarding against, so the comment about wrapping went
  with it.  -EIO because that is what cxl_internal_send_cmd() returns for a
  payload size mismatch.
- add the missing blank line before return ret in cxl_get_gsl().

Anisa Su is on the Cc list, as she asked on the v1 thread.

 drivers/cxl/core/mbox.c | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 55828a836..e7daa23de 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -794,7 +794,8 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel)
 	set_features_cap(cxl_mbox, ro_cmds, wr_cmds);
 }
 
-static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct cxl_memdev_state *mds)
+static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct cxl_memdev_state *mds,
+						       size_t *len)
 {
 	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
 	struct cxl_mbox_get_supported_logs *ret;
@@ -818,6 +819,7 @@ static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct cxl_memdev_state *
 		return ERR_PTR(rc);
 	}
 
+	*len = mbox_cmd.size_out;	/* bytes actually received */
 
 	return ret;
 }
@@ -849,18 +851,47 @@ int cxl_enumerate_cmds(struct cxl_memdev_state *mds)
 	struct cxl_mbox_get_supported_logs *gsl;
 	struct device *dev = mds->cxlds.dev;
 	struct cxl_mem_command *cmd;
+	size_t gsl_len, gsl_hdr_size, max_entries;
 	int i, rc;
 
-	gsl = cxl_get_gsl(mds);
+	gsl = cxl_get_gsl(mds, &gsl_len);
 	if (IS_ERR(gsl))
 		return PTR_ERR(gsl);
 
+	/*
+	 * The device chooses the reported payload length and min_out only
+	 * requires the entry count field on its own (2 bytes), so a response
+	 * shorter than the header is reachable.  There is nothing to enumerate
+	 * in that case: fail rather than derive a bound from an underflowed
+	 * subtraction.
+	 */
+	gsl_hdr_size = struct_offset(gsl, entry);
+	if (gsl_len < gsl_hdr_size) {
+		dev_err(dev,
+		"GSL: response of %zu bytes is too short for the header\n",
+		gsl_len);
+		return -EIO;
+	}
+
+	max_entries = (gsl_len - gsl_hdr_size) / sizeof(gsl->entry[0]);
+
 	rc = -ENOENT;
 	for (i = 0; i < le16_to_cpu(gsl->entries); i++) {
-		u32 size = le32_to_cpu(gsl->entry[i].size);
-		uuid_t uuid = gsl->entry[i].uuid;
+		u32 size;
+		uuid_t uuid;
 		u8 *log;
 
+		if (i >= max_entries) {
+			dev_warn_ratelimited(dev,
+					     "GSL: device claimed %u entries but the payload holds %zu\n",
+					     le16_to_cpu(gsl->entries),
+					     max_entries);
+			break;
+		}
+
+		size = le32_to_cpu(gsl->entry[i].size);
+		uuid = gsl->entry[i].uuid;
+
 		dev_dbg(dev, "Found LOG type %pU of size %d", &uuid, size);
 
 		if (!uuid_equal(&uuid, &log_uuid[CEL_UUID]))

base-commit: 999811aca000b0d3d1c838c60dc9db7c72eb0c73
-- 
2.34.1