On Thu, 16 Apr 2026 15:07:02 -0500 Aaron Esau <[email protected]> wrote:
> From: Aaron Esau <[email protected]> > > 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: [email protected] > 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 <[email protected]> > Signed-off-by: Aaron Esau <[email protected]> There is an alternative patch that I think covers these same cases from Jia Jia. +CC https://lore.kernel.org/qemu-devel/[email protected]/ I prefer the use of a helper like Jia Jia has done. If you have time to take a look at that patch it would be much appreciated. Jonathan > --- > 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;
