The CBW structure is 31 bytes, so CBW DATAOUT packets must be at least
31 bytes. QEMU enforces exactly 31 bytes, but this is inconsistent with
how it handles CSW packets (where it allows greater than or equal to 13
bytes) despite wording in the spec[*] being similar for both packet
types: "shall end as a short packet with exactly 31 bytes transferred".

  [*] USB MSD Bulk-Only Transport 1.0

For consistency, and on the principle of being tolerant in accepting
input, relax the CBW size check.

Alternatively, both checks could be tightened to exact. Or a message
could be printed warning of possible guest error if size is not exact,
but still accept the packets.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/usb/dev-storage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 7bc2f7664b2..fe8955bf212 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -407,7 +407,7 @@ static bool try_get_valid_cbw(USBPacket *p, struct usb_msd_cbw *cbw)
 {
     uint32_t sig;
 
-    if (p->iov.size != CBW_SIZE) {
+    if (p->iov.size < CBW_SIZE) {
         qemu_log_mask(LOG_GUEST_ERROR, "usb-msd: Bad CBW size %zu\n",
                                        p->iov.size);
         return false;
-- 
2.47.1

Am 02.05.2025 um 05:30 hat Nicholas Piggin geschrieben:
> The CBW structure is 31 bytes, so CBW DATAOUT packets must be at least
> 31 bytes. QEMU enforces exactly 31 bytes, but this is inconsistent with
> how it handles CSW packets (where it allows greater than or equal to 13
> bytes) despite wording in the spec[*] being similar for both packet
> types: "shall end as a short packet with exactly 31 bytes transferred".
> 
>   [*] USB MSD Bulk-Only Transport 1.0
> 
> For consistency, and on the principle of being tolerant in accepting
> input, relax the CBW size check.
> 
> Alternatively, both checks could be tightened to exact. Or a message
> could be printed warning of possible guest error if size is not exact,
> but still accept the packets.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

This doesn't look right to me.

CBW is a message from the host to the device. The device must fully
validate the data in it (see "6.2 Valid and Meaningful CBW"). My
understanding is that a wrong CBW size is an error.

CSW is a message from the device to the host, i.e. the iovec doesn't
really have any content when we get it. It's essentially just a buffer
in which usb-storage has to construct a valid CSW (of the exact size
13). If the buffer is larger than it has to be, that's a different case
than receiving a CBW of the wrong size. I'm not entirely sure what the
mechanism is to send exactly 13 bytes, but I assume it's related to
p->actual_length, which is updated in usb_packet_copy().

Actually, if we reject too small buffers, why do we even need the MIN()
in usb_msd_send_status()? Shouldn't len be an unconditional CSW_SIZE?

Kevin