From 0dbe4768b95eef1d0fa55a5cfd6ec7a6ffa58634 Mon Sep 17 00:00:00 2001 From: Nick Rosbrook <rosbrookn@gmail.com> Date: Mon, 1 Feb 2021 16:30:21 -0500 Subject: [PATCH 1/2] usb-host: use correct altsetting in usb_host_ep_update In order to keep track of the alternate setting that should be used for a given interface, the USBDevice struct keeps an array of alternate setting values, which is indexed by the interface number. In usb_host_set_interface, when this array is updated, usb_host_ep_update is called as a result. However, when usb_host_ep_update accesses the active libusb_config_descriptor, it indexes udev->altsetting with the loop variable, rather than the interface number. With the simple trace backend enable, this behavior can be seen: [...] usb_xhci_xfer_start 0.440 pid=1215 xfer=0x5596a4b85930 slotid=0x1 epid=0x1 streamid=0x0 usb_packet_state_change 1.703 pid=1215 bus=0x1 port=b'1' ep=0x0 p=0x5596a4b85938 o=b'undef' n=b'setup' usb_host_req_control 2.269 pid=1215 bus=0x1 addr=0x5 p=0x5596a4b85938 req=0x10b value=0x1 index=0xd usb_host_set_interface 0.449 pid=1215 bus=0x1 addr=0x5 interface=0xd alt=0x1 usb_host_parse_config 2542.648 pid=1215 bus=0x1 addr=0x5 value=0x2 active=0x1 usb_host_parse_interface 1.804 pid=1215 bus=0x1 addr=0x5 num=0xc alt=0x0 active=0x1 usb_host_parse_endpoint 2.012 pid=1215 bus=0x1 addr=0x5 ep=0x2 dir=b'in' type=b'int' active=0x1 usb_host_parse_interface 1.598 pid=1215 bus=0x1 addr=0x5 num=0xd alt=0x0 active=0x1 usb_host_req_emulated 3.593 pid=1215 bus=0x1 addr=0x5 p=0x5596a4b85938 status=0x0 usb_packet_state_change 2.550 pid=1215 bus=0x1 port=b'1' ep=0x0 p=0x5596a4b85938 o=b'setup' n=b'complete' usb_xhci_xfer_success 4.298 pid=1215 xfer=0x5596a4b85930 bytes=0x0 [...] In particular, it is seen that although usb_host_set_interface sets the alternate setting of interface 0xd to 0x1, usb_host_ep_update uses 0x0 as the alternate setting due to using the incorrect index to udev->altsetting. Fix this problem by getting the interface number from the active libusb_config_descriptor, and then using that as the index to udev->altsetting. Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com> Message-Id: <20210201213021.500277-1-rosbrookn@ainfosec.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/usb/host-libusb.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c index 7dde3d1206..354713a77d 100644 --- a/hw/usb/host-libusb.c +++ b/hw/usb/host-libusb.c @@ -836,7 +836,7 @@ static void usb_host_ep_update(USBHostDevice *s) struct libusb_ss_endpoint_companion_descriptor *endp_ss_comp; #endif uint8_t devep, type; - int pid, ep; + int pid, ep, alt; int rc, i, e; usb_ep_reset(udev); @@ -848,8 +848,20 @@ static void usb_host_ep_update(USBHostDevice *s) conf->bConfigurationValue, true); for (i = 0; i < conf->bNumInterfaces; i++) { - assert(udev->altsetting[i] < conf->interface[i].num_altsetting); - intf = &conf->interface[i].altsetting[udev->altsetting[i]]; + /* + * The udev->altsetting array indexes alternate settings + * by the interface number. Get the 0th alternate setting + * first so that we can grab the interface number, and + * then correct the alternate setting value if necessary. + */ + intf = &conf->interface[i].altsetting[0]; + alt = udev->altsetting[intf->bInterfaceNumber]; + + if (alt != 0) { + assert(alt < conf->interface[i].num_altsetting); + intf = &conf->interface[i].altsetting[alt]; + } + trace_usb_host_parse_interface(s->bus_num, s->addr, intf->bInterfaceNumber, intf->bAlternateSetting, true); From 6ba5a437ad48f10931592f649b5b7375968f362d Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann <kraxel@redhat.com> Date: Tue, 16 Feb 2021 15:49:39 +0100 Subject: [PATCH 2/2] usb/pcap: set flag_setup Without that wireshark complains about invalid control setup data for non-control transfers. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-Id: <20210216144939.841873-1-kraxel@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/usb/pcap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/usb/pcap.c b/hw/usb/pcap.c index 4350989d3a..dbff00be25 100644 --- a/hw/usb/pcap.c +++ b/hw/usb/pcap.c @@ -127,6 +127,7 @@ static void do_usb_pcap_ctrl(FILE *fp, USBPacket *p, bool setup) .xfer_type = usbmon_xfer_type[USB_ENDPOINT_XFER_CONTROL], .epnum = in ? 0x80 : 0, .devnum = dev->addr, + .flag_setup = setup ? 0 : '-', .flag_data = '=', .length = dev->setup_len, }; @@ -169,6 +170,7 @@ static void do_usb_pcap_data(FILE *fp, USBPacket *p, bool setup) .xfer_type = usbmon_xfer_type[p->ep->type], .epnum = usbmon_epnum(p), .devnum = p->ep->dev->addr, + .flag_setup = '-', .flag_data = '=', .length = p->iov.size, };