[PATCH 0/2] hw/cxl: Fix two OOB access bugs in CXL mailbox commands

[PATCH 0/2] hw/cxl: Fix two OOB access bugs in CXL mailbox commands

[Aaron Esau]

Two fixes for CXL Type-3 device emulation in hw/cxl/cxl-mailbox-utils.c:

Patch 1: cmd_logs_get_log() performs pointer arithmetic on a struct
array using a byte offset, reading past the end of cel_log. Fix the
memmove to use byte-based pointer arithmetic (cast to uint8_t *).

Patch 2: cmd_features_set_feature() is missing bounds checks on six
Set Feature handlers (soft_ppr, hard_ppr, cacheline_sparing,
row_sparing, bank_sparing, rank_sparing). A guest-controlled offset
and payload length can overflow the small write-attribute structs
into adjacent CXLType3Dev fields. Add the same bounds check already
present in the patrol_scrub and ecs cases.

Aaron Esau (2):
  hw/cxl: fix OOB read in Get Log command due to incorrect pointer
    arithmetic
  hw/cxl: add missing bounds checks in Set Feature for PPR and sparing

 hw/cxl/cxl-mailbox-utils.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

-- 
2.53.0

[PATCH 1/2] hw/cxl: fix OOB read in Get Log command due to incorrect pointer arithmetic

[Aaron Esau]

From: Aaron Esau <aaron1esau@gmail.com>

The memmove in cmd_logs_get_log() uses cci->cel_log + get_log->offset,
which performs pointer arithmetic in units of sizeof(struct cel_log)
(4 bytes per element). However, per CXL r3.1 Section 8.2.9.5.2, the
offset field is a byte offset into the log.

The existing bounds check correctly treats offset as a byte value:

    (uint64_t)get_log->offset + get_log->length >= sizeof(cci->cel_log)

But the memmove reads from a position that is get_log->offset *
sizeof(cel_log[0]) bytes past the start, which can be well beyond the
array even when the bounds check passes. For example, offset=65536 and
length=512 passes the check (66048 < sizeof(cel_log)) but the memmove
reads from byte 262144 past cel_log, leaking adjacent heap data.

Fix by casting to uint8_t * before adding the byte offset, matching the
semantics assumed by the bounds check.

Cc: qemu-stable@nongnu.org
Fixes: 056172691b ("hw/cxl/device: Add log commands (8.2.9.4) + CEL")
Reported-by: Aaron Esau <aaron1esau@gmail.com>
Signed-off-by: Aaron Esau <aaron1esau@gmail.com>
---
 hw/cxl/cxl-mailbox-utils.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index d8ba7e8625..0adf1e72c8 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -1217,7 +1217,8 @@ static CXLRetCode cmd_logs_get_log(const struct cxl_cmd *cmd,
     /* Store off everything to local variables so we can wipe out the payload */
     *len_out = get_log->length;
 
-    memmove(payload_out, cci->cel_log + get_log->offset, get_log->length);
+    memmove(payload_out, (uint8_t *)cci->cel_log + get_log->offset,
+            get_log->length);
 
     return CXL_MBOX_SUCCESS;
 }
-- 
2.53.0

Re: [PATCH 1/2] hw/cxl: fix OOB read in Get Log command due to incorrect pointer arithmetic

[Michael Tokarev]

On 16.04.2026 23:07, Aaron Esau wrote:
> From: Aaron Esau <aaron1esau@gmail.com>
> 
> The memmove in cmd_logs_get_log() uses cci->cel_log + get_log->offset,
> which performs pointer arithmetic in units of sizeof(struct cel_log)
> (4 bytes per element). However, per CXL r3.1 Section 8.2.9.5.2, the
> offset field is a byte offset into the log.
> 
> The existing bounds check correctly treats offset as a byte value:
> 
>      (uint64_t)get_log->offset + get_log->length >= sizeof(cci->cel_log)

...

Ping?

Has this patchset been forgotten, or is it not needed anymore?
If it's needed, it would be nice if it lands in the master branch
in the next 10 days.

Thanks,

/mjt

Re: [PATCH 1/2] hw/cxl: fix OOB read in Get Log command due to incorrect pointer arithmetic

[Aaron Esau]

Hi mjt,

I haven't contributed to qemu before. Am I responsible for anything
beyond submitting the patch here, to get this merged into master?

- Aaron

On Wed, May 13, 2026 at 1:35 AM Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> On 16.04.2026 23:07, Aaron Esau wrote:
> > From: Aaron Esau <aaron1esau@gmail.com>
> >
> > The memmove in cmd_logs_get_log() uses cci->cel_log + get_log->offset,
> > which performs pointer arithmetic in units of sizeof(struct cel_log)
> > (4 bytes per element). However, per CXL r3.1 Section 8.2.9.5.2, the
> > offset field is a byte offset into the log.
> >
> > The existing bounds check correctly treats offset as a byte value:
> >
> >      (uint64_t)get_log->offset + get_log->length >= sizeof(cci->cel_log)
>
> ...
>
> Ping?
>
> Has this patchset been forgotten, or is it not needed anymore?
> If it's needed, it would be nice if it lands in the master branch
> in the next 10 days.
>
> Thanks,
>
> /mjt
>

[PATCH 2/2] hw/cxl: add missing bounds checks in Set Feature for PPR and sparing

[Aaron Esau]

From: Aaron Esau <aaron1esau@gmail.com>

cmd_features_set_feature() is missing bounds validation on the offset and
data length for several feature UUIDs before their memcpy calls. The
patrol_scrub and ecs cases correctly check:

    if ((uint32_t)hdr->offset + bytes_to_copy >
        sizeof(ct3d->..._wr_attrs))
        return CXL_MBOX_INVALID_PAYLOAD_LENGTH;

But the soft_ppr, hard_ppr, cacheline_sparing, row_sparing,
bank_sparing, and rank_sparing cases are missing this check. Since
bytes_to_copy is derived from the full mailbox payload length (up to
2048 bytes) and the write-attribute structs are only a few bytes each,
a guest can overflow into adjacent CXLType3Dev fields.

Add the same bounds check pattern to all six affected feature handlers.

Cc: qemu-stable@nongnu.org
Fixes: 5e5a86bab8 ("hw/cxl: Add support for Maintenance command and Post Package Repair (PPR)")
Fixes: da5cafdc4d ("hw/cxl: Add emulation for memory sparing control feature")
Reported-by: Aaron Esau <aaron1esau@gmail.com>
Signed-off-by: Aaron Esau <aaron1esau@gmail.com>
---
 hw/cxl/cxl-mailbox-utils.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 0adf1e72c8..5fabfcbbab 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -1814,6 +1814,11 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
             return CXL_MBOX_UNSUPPORTED;
         }
 
+        if ((uint32_t)hdr->offset + bytes_to_copy >
+            sizeof(ct3d->soft_ppr_wr_attrs)) {
+            return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
+        }
+
         memcpy((uint8_t *)&ct3d->soft_ppr_wr_attrs + hdr->offset,
                sppr_write_attrs, bytes_to_copy);
         set_feat_info->data_size += bytes_to_copy;
@@ -1833,6 +1838,11 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
             return CXL_MBOX_UNSUPPORTED;
         }
 
+        if ((uint32_t)hdr->offset + bytes_to_copy >
+            sizeof(ct3d->hard_ppr_wr_attrs)) {
+            return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
+        }
+
         memcpy((uint8_t *)&ct3d->hard_ppr_wr_attrs + hdr->offset,
                hppr_write_attrs, bytes_to_copy);
         set_feat_info->data_size += bytes_to_copy;
@@ -1852,6 +1862,11 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
             return CXL_MBOX_UNSUPPORTED;
         }
 
+        if ((uint32_t)hdr->offset + bytes_to_copy >
+            sizeof(ct3d->cacheline_sparing_wr_attrs)) {
+            return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
+        }
+
         memcpy((uint8_t *)&ct3d->cacheline_sparing_wr_attrs + hdr->offset,
                mem_sparing_write_attrs, bytes_to_copy);
         set_feat_info->data_size += bytes_to_copy;
@@ -1870,6 +1885,11 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
             return CXL_MBOX_UNSUPPORTED;
         }
 
+        if ((uint32_t)hdr->offset + bytes_to_copy >
+            sizeof(ct3d->row_sparing_wr_attrs)) {
+            return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
+        }
+
         memcpy((uint8_t *)&ct3d->row_sparing_wr_attrs + hdr->offset,
                mem_sparing_write_attrs, bytes_to_copy);
         set_feat_info->data_size += bytes_to_copy;
@@ -1888,6 +1908,11 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
             return CXL_MBOX_UNSUPPORTED;
         }
 
+        if ((uint32_t)hdr->offset + bytes_to_copy >
+            sizeof(ct3d->bank_sparing_wr_attrs)) {
+            return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
+        }
+
         memcpy((uint8_t *)&ct3d->bank_sparing_wr_attrs + hdr->offset,
                mem_sparing_write_attrs, bytes_to_copy);
         set_feat_info->data_size += bytes_to_copy;
@@ -1906,6 +1931,11 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
             return CXL_MBOX_UNSUPPORTED;
         }
 
+        if ((uint32_t)hdr->offset + bytes_to_copy >
+            sizeof(ct3d->rank_sparing_wr_attrs)) {
+            return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
+        }
+
         memcpy((uint8_t *)&ct3d->rank_sparing_wr_attrs + hdr->offset,
                mem_sparing_write_attrs, bytes_to_copy);
         set_feat_info->data_size += bytes_to_copy;
-- 
2.53.0