All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] usb-host: validate and release configuration descriptors
@ 2026-10-04  4:27 Junjie Cao
  2026-10-04  4:27 ` [PATCH 1/2] usb-host: validate interface number before indexing altsetting Junjie Cao
  2026-10-04  4:27 ` [PATCH 2/2] usb-host: free configuration descriptors on error paths Junjie Cao
  0 siblings, 2 replies; 6+ messages in thread
From: Junjie Cao @ 2026-10-04  4:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: thuth, kraxel, philmd, 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.

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

 hw/usb/host-libusb.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] usb-host: validate interface number before indexing altsetting
  2026-10-04  4:27 [PATCH 0/2] usb-host: validate and release configuration descriptors Junjie Cao
@ 2026-10-04  4:27 ` Junjie Cao
  2026-10-04 16:42   ` Marc-André Lureau
  2026-10-04  4:27 ` [PATCH 2/2] usb-host: free configuration descriptors on error paths Junjie Cao
  1 sibling, 1 reply; 6+ messages in thread
From: Junjie Cao @ 2026-10-04  4:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: thuth, kraxel, philmd, junjie.cao, qemu-stable

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>
---
 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



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] usb-host: free configuration descriptors on error paths
  2026-10-04  4:27 [PATCH 0/2] usb-host: validate and release configuration descriptors Junjie Cao
  2026-10-04  4:27 ` [PATCH 1/2] usb-host: validate interface number before indexing altsetting Junjie Cao
@ 2026-10-04  4:27 ` Junjie Cao
  2026-10-04 16:34   ` Marc-André Lureau
  1 sibling, 1 reply; 6+ messages in thread
From: Junjie Cao @ 2026-10-04  4:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: thuth, kraxel, philmd, junjie.cao, qemu-stable

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>
---
 hw/usb/host-libusb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index fd4a16ee8a..491302feb7 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -921,12 +921,12 @@ static void usb_host_ep_update(USBHostDevice *s)
             if (ep == 0) {
                 trace_usb_host_parse_error(s->bus_num, s->addr,
                                            "invalid endpoint address");
-                return;
+                goto out;
             }
             if (usb_ep_get_type(udev, pid, ep) != USB_ENDPOINT_XFER_INVALID) {
                 trace_usb_host_parse_error(s->bus_num, s->addr,
                                            "duplicate endpoint address");
-                return;
+                goto out;
             }
 
             trace_usb_host_parse_endpoint(s->bus_num, s->addr, ep,
@@ -949,6 +949,7 @@ static void usb_host_ep_update(USBHostDevice *s)
         }
     }
 
+out:
     libusb_free_config_descriptor(conf);
 }
 
@@ -1375,6 +1376,7 @@ static int usb_host_claim_interfaces(USBHostDevice *s, int configuration)
         }
     }
     if (claimed != conf->bNumInterfaces) {
+        libusb_free_config_descriptor(conf);
         return USB_RET_STALL;
     }
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] usb-host: free configuration descriptors on error paths
  2026-10-04  4:27 ` [PATCH 2/2] usb-host: free configuration descriptors on error paths Junjie Cao
@ 2026-10-04 16:34   ` Marc-André Lureau
  2026-10-05  5:45     ` Junjie Cao
  0 siblings, 1 reply; 6+ messages in thread
From: Marc-André Lureau @ 2026-10-04 16:34 UTC (permalink / raw)
  To: Junjie Cao; +Cc: qemu-devel, thuth, kraxel, philmd, qemu-stable

Hi

On Sun, Oct 4, 2026 at 4:11 PM Junjie Cao <junjie.cao@linux.dev> wrote:
>
> 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>

Although it's an opportunity to introduce g_auto support:
typedef struct libusb_config_descriptor libusb_config_descriptor;
G_DEFINE_AUTOPTR_CLEANUP_FUNC(libusb_config_descriptor,
                              libusb_free_config_descriptor)

Then declaration should become:
g_autoptr(libusb_config_descriptor) conf = NULL;

And we can drop free paths. Up to you if you want to make a v2


> ---
>  hw/usb/host-libusb.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
> index fd4a16ee8a..491302feb7 100644
> --- a/hw/usb/host-libusb.c
> +++ b/hw/usb/host-libusb.c
> @@ -921,12 +921,12 @@ static void usb_host_ep_update(USBHostDevice *s)
>              if (ep == 0) {
>                  trace_usb_host_parse_error(s->bus_num, s->addr,
>                                             "invalid endpoint address");
> -                return;
> +                goto out;
>              }
>              if (usb_ep_get_type(udev, pid, ep) != USB_ENDPOINT_XFER_INVALID) {
>                  trace_usb_host_parse_error(s->bus_num, s->addr,
>                                             "duplicate endpoint address");
> -                return;
> +                goto out;
>              }
>
>              trace_usb_host_parse_endpoint(s->bus_num, s->addr, ep,
> @@ -949,6 +949,7 @@ static void usb_host_ep_update(USBHostDevice *s)
>          }
>      }
>
> +out:
>      libusb_free_config_descriptor(conf);
>  }
>
> @@ -1375,6 +1376,7 @@ static int usb_host_claim_interfaces(USBHostDevice *s, int configuration)
>          }
>      }
>      if (claimed != conf->bNumInterfaces) {
> +        libusb_free_config_descriptor(conf);
>          return USB_RET_STALL;
>      }
>
> --
> 2.43.0
>
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] usb-host: validate interface number before indexing altsetting
  2026-10-04  4:27 ` [PATCH 1/2] usb-host: validate interface number before indexing altsetting Junjie Cao
@ 2026-10-04 16:42   ` Marc-André Lureau
  0 siblings, 0 replies; 6+ messages in thread
From: Marc-André Lureau @ 2026-10-04 16:42 UTC (permalink / raw)
  To: Junjie Cao; +Cc: qemu-devel, thuth, kraxel, philmd, qemu-stable

Hi

On Sun, Oct 4, 2026 at 4:11 PM Junjie Cao <junjie.cao@linux.dev> wrote:
>
> 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>
> ---
>  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;
> +        }

unlikely to happen, but better be safe.

>          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);

ack, it happens to match generally by accident

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>              usb_ep_set_halted(udev, pid, ep, 0);
>  #ifdef HAVE_STREAMS
>              if (type == LIBUSB_TRANSFER_TYPE_BULK &&
> --
> 2.43.0
>
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] usb-host: free configuration descriptors on error paths
  2026-10-04 16:34   ` Marc-André Lureau
@ 2026-10-05  5:45     ` Junjie Cao
  0 siblings, 0 replies; 6+ messages in thread
From: Junjie Cao @ 2026-10-05  5:45 UTC (permalink / raw)
  To: marcandre.lureau
  Cc: qemu-devel, thuth, kraxel, philmd, qemu-stable, junjie.cao

Thanks for the review. Will send v2 with the g_autoptr conversion.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-05  5:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-04  4:27 [PATCH 0/2] usb-host: validate and release configuration descriptors Junjie Cao
2026-10-04  4:27 ` [PATCH 1/2] usb-host: validate interface number before indexing altsetting Junjie Cao
2026-10-04 16:42   ` Marc-André Lureau
2026-10-04  4:27 ` [PATCH 2/2] usb-host: free configuration descriptors on error paths Junjie Cao
2026-10-04 16:34   ` Marc-André Lureau
2026-10-05  5:45     ` Junjie Cao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.