[PATCH] hw/usb/dev-mtp: Fix assert crash on undersized BULK IN

Tommaso Califano posted 1 patch 1 week, 3 days ago
hw/usb/dev-mtp.c | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH] hw/usb/dev-mtp: Fix assert crash on undersized BULK IN
Posted by Tommaso Califano 1 week, 3 days ago
The EP_DATA_IN result path serializes the queued response as a 12
byte MTP container header followed by one 4 byte word per result
argument (12 + 4 * argc bytes).  The entry check at the top of the
function only verifies that the guest-provided transfer buffer holds
at least the 12 byte header.

If the queued result carries arguments, e.g. RES_SESSION_ALREADY_OPEN
which returns the existing session ID, the second usb_packet_copy()
writes past the end of the guest-declared buffer and trips the
p->actual_length + bytes <= iov->size assertion, aborting QEMU.  A
guest can control the transfer size freely, e.g. TBYTES in the qTD
of an EHCI BULK IN.

A device must not send more data than the initiator signaled it would
accept. Stall the transfer instead, and keep the result queued so that
a subsequent BULK IN with a sufficient buffer size delivers the full
response.

Fixes: 840a178c94db ("usb: mtp filesharing")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3386
Signed-off-by: Tommaso Califano <tcalifano@redhat.com>
---
 hw/usb/dev-mtp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index c2d36a73cf..cbc2be3317 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1905,6 +1905,13 @@ static void usb_mtp_handle_data(USBDevice *dev, USBPacket *p)
         } else if (s->result != NULL) {
             MTPControl *r = s->result;
             int length = sizeof(container) + r->argc * sizeof(uint32_t);
+
+            if (p->iov.size < length) {
+                trace_usb_mtp_stall(s->dev.addr,
+                                    "packet too small to send response");
+                p->status = USB_RET_STALL;
+                return;
+            }
             if (r->code == RES_OK) {
                 trace_usb_mtp_success(s->dev.addr, r->trans,
                                       (r->argc > 0) ? r->argv[0] : 0,
-- 
2.56.0
Re: [PATCH] hw/usb/dev-mtp: Fix assert crash on undersized BULK IN
Posted by Junjie Cao 1 week, 2 days ago
From: Junjie Cao <junjie.cao@intel.com>

The check is correctly placed before either copy, and the early return leaves
s->result queued so a later BULK IN with sufficient space can deliver the
response.

Reviewed-by: Junjie Cao <junjie.cao@intel.com>
Re: [PATCH] hw/usb/dev-mtp: Fix assert crash on undersized BULK IN
Posted by Luigi Leonardi 1 week, 2 days ago
On Mon, Oct 12, 2026 at 04:08:38PM +0200, Tommaso Califano wrote:
>The EP_DATA_IN result path serializes the queued response as a 12
>byte MTP container header followed by one 4 byte word per result
>argument (12 + 4 * argc bytes).  The entry check at the top of the
>function only verifies that the guest-provided transfer buffer holds
>at least the 12 byte header.
>
>If the queued result carries arguments, e.g. RES_SESSION_ALREADY_OPEN
>which returns the existing session ID, the second usb_packet_copy()
>writes past the end of the guest-declared buffer and trips the
>p->actual_length + bytes <= iov->size assertion, aborting QEMU.  A
>guest can control the transfer size freely, e.g. TBYTES in the qTD
>of an EHCI BULK IN.
>
>A device must not send more data than the initiator signaled it would
>accept. Stall the transfer instead, and keep the result queued so that
>a subsequent BULK IN with a sufficient buffer size delivers the full
>response.
>
>Fixes: 840a178c94db ("usb: mtp filesharing")
>Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3386
>Signed-off-by: Tommaso Califano <tcalifano@redhat.com>
>---

code LGTM!

Reviewed-by: Luigi Leonardi <leonardi@redhat.com>

Thanks,
Luigi