Re: [PATCH v4 15/22] usb/msd: Allow CBW packet size greater than 31

[Kevin Wolf <kwolf@redhat.com>]

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