[Qemu-devel] [PATCH 0/1] USB CCID packet handling issue

jjelen@redhat.com posted 1 patch 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180516115544.3897-1-jjelen@redhat.com
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test s390x passed
hw/usb/dev-smartcard-reader.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
[Qemu-devel] [PATCH 0/1] USB CCID packet handling issue
Posted by jjelen@redhat.com 5 years, 10 months ago
From: Jakub Jelen <jjelen@redhat.com>

while working on libcacard, I encountered an issue with USB "packet
fragmentation" (for the lack of better knowledge how to call it) in the
CCID code.

It happens if I try to send exactly 34 B from the virtualized Smart
Card to the guest. These 34 B are packed into 64 B payload on USB
layer, which is the boundary where the "fragmentation" of USB 1.1
packets should happen.

In the guest, the packet is delivered, but not "committed", I see it in
wireshark with 3 seconds delay and with -ENOENT URB status, while pcscd
times out while waiting for this packet.

If I send one byte less or more, the packet(s) go through as expected.

I debugged up to the function, that is handling the data passing to the
USB emulation layer [1].

From the debug logs, I can see that there is something clearly wrong.
For 63 B message, I can see the following:

usb-ccid: ccid_bulk_in_copy_to_guest: 64/63 req/act to guest (BULK_IN)
usb-ccid: ccid_bulk_in_copy_to_guest: returning short (EREMOTEIO) 63 <
64

But if I bump the length to 64 B, I see one correct message sending the
data and then hundreds of messages like this (assuming until the caller
timer times out):

usb-ccid: ccid_bulk_in_copy_to_guest: 64/64 req/act to guest (BULK_IN)
usb-ccid: ccid_bulk_in_copy_to_guest: returning short (EREMOTEIO) 0 <
64
usb-ccid: ccid_bulk_in_copy_to_guest: returning short (EREMOTEIO) 0 <
64
...

From the USB specification (8.5.3.2 Variable-length Data Stage), the
following section seems to explain the issue:

>  When all of the data structure is returned to the host, the function
> should indicate that the Data stage is ended by returning a packet
> that is shorter than the MaxPacketSize for thepipe.  If the data
> structure is an exact multiple of wMaxPacketSize for the pipe, the
> function will return a zero-length packet to indicate the end of the
> Data stage.

The code does not have any check for the full packet size and it is not
sending zero-length packet in that case, only NAKs, which means we can
not send data now, the host retries and then comes the timeout.

The attached patch address the issue for me and was already reviewed by
Gerd Hoffmann.

Jakub Jelen (1):
  hw/usb/dev-smartcard-reader: Handle 64 B USB packets

 hw/usb/dev-smartcard-reader.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

-- 
2.14.3