hw/usb/host-libusb.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
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>
---
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 fcf48c0193..6ab75e2feb 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -810,7 +810,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);
@@ -822,8 +822,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);
--
2.17.1
Hi, Just wanted to ping this. Patchwork link is here: https://patchwork.kernel.org/project/qemu-devel/patch/20210201213021.500277-1-rosbrookn@ainfosec.com/. Thanks, NR On Mon, Feb 1, 2021 at 4:30 PM Nick Rosbrook <rosbrookn@gmail.com> wrote: > > 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> > --- > 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 fcf48c0193..6ab75e2feb 100644 > --- a/hw/usb/host-libusb.c > +++ b/hw/usb/host-libusb.c > @@ -810,7 +810,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); > @@ -822,8 +822,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); > -- > 2.17.1 >
On Thu, Feb 11, 2021 at 11:05:41AM -0500, Nick Rosbrook wrote: > Hi, > > Just wanted to ping this. Patchwork link is here: > https://patchwork.kernel.org/project/qemu-devel/patch/20210201213021.500277-1-rosbrookn@ainfosec.com/. Pull request sent now. Not much usb activity these days ... thanks, Gerd
On Thu, Feb 18, 2021 at 12:52:51PM +0100, Gerd Hoffmann wrote: > On Thu, Feb 11, 2021 at 11:05:41AM -0500, Nick Rosbrook wrote: > > Hi, > > > > Just wanted to ping this. Patchwork link is here: > > https://patchwork.kernel.org/project/qemu-devel/patch/20210201213021.500277-1-rosbrookn@ainfosec.com/. > > Pull request sent now. > Not much usb activity these days ... Thanks! NR
© 2016 - 2024 Red Hat, Inc.