[PATCH v2 0/2] usb-host: validate and release configuration descriptors

[PATCH v2 0/2] usb-host: validate and release configuration descriptors

[Junjie Cao]

The libusb host backend reads interface numbers from the active
configuration descriptor while opening a device. An out-of-range number
can index past USBDevice.altsetting. The backend also leaves an allocated
descriptor behind on endpoint parse and interface claim failures.

Changes in v2:
- Use g_autoptr for configuration descriptors in usb_host_ep_update() and
  usb_host_claim_interfaces() (Marc-André).
- Collected tags from Marc-André.

v1: https://lore.kernel.org/qemu-devel/20261004042722.6937-1-junjie.cao@linux.dev/

Junjie Cao (2):
  usb-host: validate interface number before indexing altsetting
  usb-host: free configuration descriptors on error paths

 hw/usb/host-libusb.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)


base-commit: d7a65d1793d691d356a56833620f7d1e6f5d653b
-- 
2.43.0

[PATCH v2 1/2] usb-host: validate interface number before indexing altsetting

[Junjie Cao]

From: Junjie Cao <junjie.cao@intel.com>

usb_host_ep_update() indexes USBDevice.altsetting with the interface
number supplied by the active configuration descriptor. A descriptor
with bInterfaceNumber >= USB_MAX_INTERFACES reads past that array while
the host device is opened. Skip such interfaces before the lookup.

Record the descriptor's interface number on its endpoints as well. The
loop index identifies the descriptor's position in the configuration,
which need not equal bInterfaceNumber.

Fixes: 0dbe4768b95 ("usb-host: use correct altsetting in usb_host_ep_update")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3975
Cc: qemu-stable@nongnu.org
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/usb/host-libusb.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 629505c6d3..fd4a16ee8a 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -895,6 +895,11 @@ static void usb_host_ep_update(USBHostDevice *s)
          * then correct the alternate setting value if necessary.
          */
         intf = &conf->interface[i].altsetting[0];
+        if (intf->bInterfaceNumber >= USB_MAX_INTERFACES) {
+            trace_usb_host_parse_error(s->bus_num, s->addr,
+                                       "invalid interface number");
+            continue;
+        }
         alt = udev->altsetting[intf->bInterfaceNumber];
 
         if (alt != 0) {
@@ -930,7 +935,7 @@ static void usb_host_ep_update(USBHostDevice *s)
             usb_ep_set_max_packet_size(udev, pid, ep,
                                        endp->wMaxPacketSize);
             usb_ep_set_type(udev, pid, ep, type);
-            usb_ep_set_ifnum(udev, pid, ep, i);
+            usb_ep_set_ifnum(udev, pid, ep, intf->bInterfaceNumber);
             usb_ep_set_halted(udev, pid, ep, 0);
 #ifdef HAVE_STREAMS
             if (type == LIBUSB_TRANSFER_TYPE_BULK &&
-- 
2.43.0

[PATCH v2 2/2] usb-host: free configuration descriptors on error paths

[Junjie Cao]

From: Junjie Cao <junjie.cao@intel.com>

usb_host_ep_update() returns before freeing the active configuration
descriptor when an endpoint address is invalid or duplicated.
usb_host_claim_interfaces() also returns without freeing it when fewer
interfaces can be claimed than the descriptor declares. Each failure
leaves one descriptor allocated.

Fixes: 2b2325ff64 ("use libusb for usb-host")
Cc: qemu-stable@nongnu.org
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/usb/host-libusb.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index fd4a16ee8a..cd566b91a3 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -68,6 +68,10 @@ typedef struct USBHostRequest USBHostRequest;
 typedef struct USBHostIsoXfer USBHostIsoXfer;
 typedef struct USBHostIsoRing USBHostIsoRing;
 
+typedef struct libusb_config_descriptor libusb_config_descriptor;
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(libusb_config_descriptor,
+                            libusb_free_config_descriptor)
+
 struct USBAutoFilter {
     uint32_t bus_num;
     uint32_t addr;
@@ -869,7 +873,7 @@ static void usb_host_ep_update(USBHostDevice *s)
         [USB_ENDPOINT_XFER_INT]     = "int",
     };
     USBDevice *udev = USB_DEVICE(s);
-    struct libusb_config_descriptor *conf;
+    g_autoptr(libusb_config_descriptor) conf = NULL;
     const struct libusb_interface_descriptor *intf;
     const struct libusb_endpoint_descriptor *endp;
 #ifdef HAVE_STREAMS
@@ -948,8 +952,6 @@ static void usb_host_ep_update(USBHostDevice *s)
 #endif
         }
     }
-
-    libusb_free_config_descriptor(conf);
 }
 
 static int usb_host_open(USBHostDevice *s, libusb_device *dev, int hostfd)
@@ -1343,7 +1345,7 @@ static void usb_host_attach_kernel(USBHostDevice *s)
 static int usb_host_claim_interfaces(USBHostDevice *s, int configuration)
 {
     USBDevice *udev = USB_DEVICE(s);
-    struct libusb_config_descriptor *conf;
+    g_autoptr(libusb_config_descriptor) conf = NULL;
     int rc, i, claimed;
 
     for (i = 0; i < USB_MAX_INTERFACES; i++) {
@@ -1381,7 +1383,6 @@ static int usb_host_claim_interfaces(USBHostDevice *s, int configuration)
     udev->ninterfaces   = conf->bNumInterfaces;
     udev->configuration = configuration;
 
-    libusb_free_config_descriptor(conf);
     return USB_RET_SUCCESS;
 }
 
-- 
2.43.0