* [PULL 1/5] hw/cxl: Validate Set Feature payload bounds
2026-07-27 12:52 [PULL 0/5] Various fixes for 11.1-rc2 Thomas Huth
@ 2026-07-27 12:52 ` Thomas Huth
2026-07-27 12:52 ` [PULL 2/5] hw/usb/dev-uas: Fix guest-triggerable heap OOB access Thomas Huth
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2026-07-27 12:52 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi; +Cc: Feifan Qian, Jia Jia
From: Feifan Qian <bea1e@proton.me>
cmd_features_set_feature() derives bytes_to_copy from the mailbox input
length and uses hdr->offset as the destination offset into per-feature
write attribute buffers.
The patrol scrub and ECS paths already reject writes where hdr->offset
plus bytes_to_copy exceeds the destination structure. Add the same check
to the soft PPR, hard PPR and memory sparing feature paths before
copying into their write attribute buffers.
Without the check, a malformed Set Feature request can write past the
selected write attribute object and corrupt adjacent CXL type 3 device
state.
Fixes: 5e5a86bab830 ("hw/cxl: Add support for Maintenance command and Post Package Repair (PPR)")
Fixes: da5cafdc4ddd ("hw/cxl: Add emulation for memory sparing control feature")
Signed-off-by: Feifan Qian <bea1e@proton.me>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3458
Reported-by: Jia Jia <physicalmtea@gmail.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/cxl/cxl-mailbox-utils.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 20e0b7e476e..ec18338b423 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -1813,6 +1813,10 @@ 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;
@@ -1832,6 +1836,10 @@ 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;
@@ -1851,6 +1859,10 @@ 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;
@@ -1869,6 +1881,10 @@ 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;
@@ -1887,6 +1903,10 @@ 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;
@@ -1905,6 +1925,10 @@ 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.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PULL 2/5] hw/usb/dev-uas: Fix guest-triggerable heap OOB access
2026-07-27 12:52 [PULL 0/5] Various fixes for 11.1-rc2 Thomas Huth
2026-07-27 12:52 ` [PULL 1/5] hw/cxl: Validate Set Feature payload bounds Thomas Huth
@ 2026-07-27 12:52 ` Thomas Huth
2026-07-27 12:52 ` [PULL 3/5] hw/ide/core: Fix possible crash via NULL pointer in ide_cancel_dma_sync() Thomas Huth
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2026-07-27 12:52 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi; +Cc: Tristan Madani, Peter Maydell
From: Thomas Huth <thuth@redhat.com>
The stream ID is under control of the guest, and some spots in the
code currently use it for indexing into the status3[] array without
checking it for being in range first, so the code accesses the heap
beyond the limit of the status3 array.
Since our status delivery code depends on having a valid stream ID,
we must not try to generate a fake sense code in this situation.
Simply log a guest error and return early in usb_uas_command().
And to make sure that we really cannot access the status3[] array
beyond its limit anymore, add some assert() statements in the
affected functions, too.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3612
Reported-by: Reported-by: huntr bubble
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3986
Reported-by: Tristan Madani <tristan@talencesecurity.com>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20260720134809.573757-1-thuth@redhat.com>
---
hw/usb/dev-uas.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 8576dfec96f..963c0433b38 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -362,6 +362,7 @@ static void usb_uas_send_status_bh(void *opaque)
while ((st = QTAILQ_FIRST(&uas->results)) != NULL) {
if (uas_using_streams(uas)) {
+ assert(st->stream <= UAS_MAX_STREAMS);
p = uas->status3[st->stream];
uas->status3[st->stream] = NULL;
} else {
@@ -383,8 +384,14 @@ static void usb_uas_send_status_bh(void *opaque)
static void usb_uas_queue_status(UASDevice *uas, UASStatus *st, int length)
{
- USBPacket *p = uas_using_streams(uas) ?
- uas->status3[st->stream] : uas->status2;
+ USBPacket *p;
+
+ if (uas_using_streams(uas)) {
+ assert(st->stream <= UAS_MAX_STREAMS);
+ p = uas->status3[st->stream];
+ } else {
+ p = uas->status2;
+ }
st->length += length;
QTAILQ_INSERT_TAIL(&uas->results, st, next);
@@ -700,14 +707,22 @@ static void usb_uas_command(UASDevice *uas, uas_iu *iu)
uint16_t tag = be16_to_cpu(iu->hdr.tag);
size_t cdb_len = sizeof(iu->command.cdb) + iu->command.add_cdb_length;
+ if (uas_using_streams(uas) && tag > UAS_MAX_STREAMS) {
+ /*
+ * Our status delivery only works with valid tags, so in case the
+ * stream ID is out of bounds, we have to return immediately here
+ * without sending a fake sense_code_INVALID_TAG to the guest.
+ */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "invalid tag 0x%x for USB UAS command\n", tag);
+ return;
+ }
+
if (iu->command.add_cdb_length > 0) {
qemu_log_mask(LOG_UNIMP, "additional adb length not yet supported\n");
goto unsupported_len;
}
- if (uas_using_streams(uas) && tag > UAS_MAX_STREAMS) {
- goto invalid_tag;
- }
req = usb_uas_find_request(uas, tag);
if (req) {
goto overlapped_tag;
@@ -744,10 +759,6 @@ unsupported_len:
usb_uas_queue_fake_sense(uas, tag, sense_code_INVALID_PARAM_VALUE);
return;
-invalid_tag:
- usb_uas_queue_fake_sense(uas, tag, sense_code_INVALID_TAG);
- return;
-
overlapped_tag:
usb_uas_queue_fake_sense(uas, tag, sense_code_OVERLAPPED_COMMANDS);
return;
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PULL 3/5] hw/ide/core: Fix possible crash via NULL pointer in ide_cancel_dma_sync()
2026-07-27 12:52 [PULL 0/5] Various fixes for 11.1-rc2 Thomas Huth
2026-07-27 12:52 ` [PULL 1/5] hw/cxl: Validate Set Feature payload bounds Thomas Huth
2026-07-27 12:52 ` [PULL 2/5] hw/usb/dev-uas: Fix guest-triggerable heap OOB access Thomas Huth
@ 2026-07-27 12:52 ` Thomas Huth
2026-07-27 12:52 ` [PULL 4/5] hw/usb/core: Avoid possible assert() in do_parameter() --> usb_packet_copy() Thomas Huth
2026-07-27 12:52 ` [PULL 5/5] hw/usb/hcd-xhci: Check return value of xhci_xfer_create_sgl() for errors Thomas Huth
4 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2026-07-27 12:52 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi; +Cc: Alexander Bulekov
From: Thomas Huth <thuth@redhat.com>
ide_cancel_dma_sync() is called with a "IDEState *s" for one of the
two IDE drives on a bus (primary or secondary drive) to cancel all
pending DMA transfers on the drive. The code then checks
s->bus->dma->aiocb to see whether there is any IO in flight on the
*bus* and then calls blk_drain(s->blk) to wait for its completion.
However, s->bus->dma->aiocb might belong to the other drive on the
bus, and if there is no disk attached to the current drive, s->blk
is NULL. Since blk_drain() does not check its parameter for a NULL
pointer, QEMU can crash in such a case.
To fix the problem, we have to check that "blk" is not NULL before
calling blk_drain(). And we have to call blk_drain() for both drives,
otherwise the assert(s->bus->dma->aiocb == NULL) statement after
the blk_drain() might trigger if the IO in flight belongs to the
the other drive.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/905
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4052
Reported-by: dong ling
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20260721070216.82984-1-thuth@redhat.com>
---
hw/ide/core.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 4c1ee19d8e8..fb9bf11b455 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -741,10 +741,17 @@ void ide_cancel_dma_sync(IDEState *s)
* In the future we'll be able to safely cancel the I/O if the
* whole DMA operation will be submitted to disk with a single
* aio operation with preadv/pwritev.
+ *
+ * Note: s->bus->dma->aiocb might belong to the adjacent IDEState,
+ * so we have to drain both drives to get it cleared.
*/
if (s->bus->dma->aiocb) {
trace_ide_cancel_dma_sync_remaining();
- blk_drain(s->blk);
+ for (int i = 0; i < 2; i++) {
+ if (s->bus->ifs[i].blk) {
+ blk_drain(s->bus->ifs[i].blk);
+ }
+ }
assert(s->bus->dma->aiocb == NULL);
}
}
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PULL 4/5] hw/usb/core: Avoid possible assert() in do_parameter() --> usb_packet_copy()
2026-07-27 12:52 [PULL 0/5] Various fixes for 11.1-rc2 Thomas Huth
` (2 preceding siblings ...)
2026-07-27 12:52 ` [PULL 3/5] hw/ide/core: Fix possible crash via NULL pointer in ide_cancel_dma_sync() Thomas Huth
@ 2026-07-27 12:52 ` Thomas Huth
2026-07-27 12:52 ` [PULL 5/5] hw/usb/hcd-xhci: Check return value of xhci_xfer_create_sgl() for errors Thomas Huth
4 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2026-07-27 12:52 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi; +Cc: Yunhe Wang
From: Thomas Huth <thuth@redhat.com>
usb_packet_copy() uses assert(p->actual_length + bytes <= iov->size)
to make sure that there is enough space in the the iov. This assert()
can be triggered from do_parameter() if the guest programs the XHCI
in a weird way. Avoid the hard error by checking for the condition
in do_parameter() first and signalling a USB_RET_STALL to the guest,
just like it is done for another error condition here already some
lines earlier.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3746
Reported-by: Yunhe Wang <yunhewwww@163.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20260721185140.247775-1-thuth@redhat.com>
---
hw/usb/core.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/hw/usb/core.c b/hw/usb/core.c
index d71204c5c80..43653c26cae 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -26,6 +26,7 @@
#include "qemu/osdep.h"
#include "hw/usb/usb.h"
#include "qemu/iov.h"
+#include "qemu/log.h"
#include "trace.h"
void usb_pick_speed(USBPort *port)
@@ -288,6 +289,15 @@ static void do_parameter(USBDevice *s, USBPacket *p)
p->status = USB_RET_STALL;
return;
}
+ if ((p->pid == USB_TOKEN_OUT || p->pid == USB_TOKEN_IN) &&
+ setup_len > p->iov.size) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "xhci: setup state param length %u > iov size %zu\n",
+ setup_len, p->iov.size);
+ p->status = USB_RET_STALL;
+ return;
+ }
+
s->setup_len = setup_len;
if (p->pid == USB_TOKEN_OUT) {
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PULL 5/5] hw/usb/hcd-xhci: Check return value of xhci_xfer_create_sgl() for errors
2026-07-27 12:52 [PULL 0/5] Various fixes for 11.1-rc2 Thomas Huth
` (3 preceding siblings ...)
2026-07-27 12:52 ` [PULL 4/5] hw/usb/core: Avoid possible assert() in do_parameter() --> usb_packet_copy() Thomas Huth
@ 2026-07-27 12:52 ` Thomas Huth
4 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2026-07-27 12:52 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi; +Cc: Feifan Qian, Peter Maydell
From: Thomas Huth <thuth@redhat.com>
xhci_xfer_create_sgl() can fail if a guest programmed the XHCI in
a weird way. The current code ignores this error, and this triggers
an assert() shortly afterwards:
hw/usb/core.c:612: usb_packet_copy:
Assertion `p->actual_length + bytes <= iov->size' failed.
Fix it by handling the error correctly (i.e. return with an error to
the caller).
While we're at it, change the DPRINTF statements in xhci_xfer_create_sgl()
into proper qemu_log_mask() statements, so we have a better way to detect
this situation.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3786
Reported-by: Feifan Qian <bea1e@proton.me>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20260724110933.629791-1-thuth@redhat.com>
---
hw/usb/hcd-xhci.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 569386b8cf1..d342aa2739e 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1464,7 +1464,8 @@ static int xhci_xfer_create_sgl(XHCITransfer *xfer, int in_xfer)
switch (TRB_TYPE(*trb)) {
case TR_DATA:
if ((!(trb->control & TRB_TR_DIR)) != (!in_xfer)) {
- DPRINTF("xhci: data direction mismatch for TR_DATA\n");
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "xhci: data direction mismatch for TR_DATA\n");
goto err;
}
/* fallthrough */
@@ -1474,7 +1475,8 @@ static int xhci_xfer_create_sgl(XHCITransfer *xfer, int in_xfer)
chunk = trb->status & 0x1ffff;
if (trb->control & TRB_TR_IDT) {
if (chunk > 8 || in_xfer) {
- DPRINTF("xhci: invalid immediate data TRB\n");
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "xhci: invalid immediate data TRB\n");
goto err;
}
qemu_sglist_add(&xfer->sgl, trb->addr, chunk);
@@ -1617,7 +1619,9 @@ static int xhci_setup_packet(XHCITransfer *xfer)
}
}
- xhci_xfer_create_sgl(xfer, dir == USB_TOKEN_IN); /* Also sets int_req */
+ if (xhci_xfer_create_sgl(xfer, dir == USB_TOKEN_IN) < 0) { /* Also sets int_req */
+ return -1;
+ }
usb_packet_setup(&xfer->packet, dir, ep, xfer->streamid,
xfer->trbs[0].addr, false, xfer->int_req);
if (usb_packet_map(&xfer->packet, &xfer->sgl)) {
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread