@samitouri / QOSamiQemu / commits / b7520c9a59

hw/usb/dev-uas: Fix guest-triggerable heap OOB access

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>

Thomas Huth committed Jul 20, 2026 at 15:48 UTC b7520c9a59128d7e616876a511821c70edab2a90
1 file changed +20 -9
hw/usb/dev-uas.c
+20 -9
@@ -362,6 +362,7 @@ static void usb_uas_send_status_bh(void *opaque)
362
363 while ((st = QTAILQ_FIRST(&uas->results)) != NULL) {
364 if (uas_using_streams(uas)) {
365 + assert(st->stream <= UAS_MAX_STREAMS);
366 p = uas->status3[st->stream];
367 uas->status3[st->stream] = NULL;
368 } else {
@@ -383,8 +384,14 @@ static void usb_uas_send_status_bh(void *opaque)
384
385 static void usb_uas_queue_status(UASDevice *uas, UASStatus *st, int length)
386 {
386 - USBPacket *p = uas_using_streams(uas) ?
387 - uas->status3[st->stream] : uas->status2;
387 + USBPacket *p;
388 +
389 + if (uas_using_streams(uas)) {
390 + assert(st->stream <= UAS_MAX_STREAMS);
391 + p = uas->status3[st->stream];
392 + } else {
393 + p = uas->status2;
394 + }
395
396 st->length += length;
397 QTAILQ_INSERT_TAIL(&uas->results, st, next);
@@ -700,14 +707,22 @@ static void usb_uas_command(UASDevice *uas, uas_iu *iu)
707 uint16_t tag = be16_to_cpu(iu->hdr.tag);
708 size_t cdb_len = sizeof(iu->command.cdb) + iu->command.add_cdb_length;
709
710 + if (uas_using_streams(uas) && tag > UAS_MAX_STREAMS) {
711 + /*
712 + * Our status delivery only works with valid tags, so in case the
713 + * stream ID is out of bounds, we have to return immediately here
714 + * without sending a fake sense_code_INVALID_TAG to the guest.
715 + */
716 + qemu_log_mask(LOG_GUEST_ERROR,
717 + "invalid tag 0x%x for USB UAS command\n", tag);
718 + return;
719 + }
720 +
721 if (iu->command.add_cdb_length > 0) {
722 qemu_log_mask(LOG_UNIMP, "additional adb length not yet supported\n");
723 goto unsupported_len;
724 }
725
708 - if (uas_using_streams(uas) && tag > UAS_MAX_STREAMS) {
709 - goto invalid_tag;
710 - }
726 req = usb_uas_find_request(uas, tag);
727 if (req) {
728 goto overlapped_tag;
@@ -744,10 +759,6 @@ unsupported_len:
759 usb_uas_queue_fake_sense(uas, tag, sense_code_INVALID_PARAM_VALUE);
760 return;
761
747 -invalid_tag:
748 - usb_uas_queue_fake_sense(uas, tag, sense_code_INVALID_TAG);
749 - return;
750 -
762 overlapped_tag:
763 usb_uas_queue_fake_sense(uas, tag, sense_code_OVERLAPPED_COMMANDS);
764 return;