[PATCH] hw/usb/hcd-ohci: check for MPS=0 to avoid infinite loop

[PATCH] hw/usb/hcd-ohci: check for MPS=0 to avoid infinite loop

[Jenny Guanni Qu]

When a guest sets MaxPacketSize to 0 in an OHCI Endpoint Descriptor,
ohci_service_td() transfers 0 bytes per iteration. The Transfer
Descriptor never completes because CBP never advances toward BE,
causing ohci_service_ed_list() to loop indefinitely and hang QEMU.

Add a check for MPS==0 after extracting the field from ED flags.
If MPS is zero, call ohci_die() to reset the controller and return
an error, preventing the infinite loop.

Fixes: CVE-2026-3890
Reported-by: Jenny Guanni Qu <qguanni@gmail.com>
Signed-off-by: Jenny Guanni Qu <qguanni@gmail.com>
---
 hw/usb/hcd-ohci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 1aeed9286f..b93a99593e 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -956,6 +956,10 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
         if (len && dir != OHCI_TD_DIR_IN) {
             /* The endpoint may not allow us to transfer it all now */
             pktlen = (ed->flags & OHCI_ED_MPS_MASK) >> OHCI_ED_MPS_SHIFT;
+            if (pktlen == 0) {
+                ohci_die(ohci);
+                return 1;
+            }
             if (pktlen > len) {
                 pktlen = len;
             }
-- 
2.34.1

Re: [PATCH] hw/usb/hcd-ohci: check for MPS=0 to avoid infinite loop

[Peter Maydell]

On Wed, 18 Mar 2026 at 19:36, Jenny Guanni Qu <qguanni@gmail.com> wrote:
>
> When a guest sets MaxPacketSize to 0 in an OHCI Endpoint Descriptor,
> ohci_service_td() transfers 0 bytes per iteration. The Transfer
> Descriptor never completes because CBP never advances toward BE,
> causing ohci_service_ed_list() to loop indefinitely and hang QEMU.
>
> Add a check for MPS==0 after extracting the field from ED flags.
> If MPS is zero, call ohci_die() to reset the controller and return
> an error, preventing the infinite loop.
>
> Fixes: CVE-2026-3890
> Reported-by: Jenny Guanni Qu <qguanni@gmail.com>
> Signed-off-by: Jenny Guanni Qu <qguanni@gmail.com>
> ---
>  hw/usb/hcd-ohci.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index 1aeed9286f..b93a99593e 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -956,6 +956,10 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
>          if (len && dir != OHCI_TD_DIR_IN) {
>              /* The endpoint may not allow us to transfer it all now */
>              pktlen = (ed->flags & OHCI_ED_MPS_MASK) >> OHCI_ED_MPS_SHIFT;
> +            if (pktlen == 0) {
> +                ohci_die(ohci);
> +                return 1;
> +            }

The OHCI spec doesn't (as far as I can tell) say what the
hardware should do for this situation, so I guess "flag it
as an error" is a reasonable choice. I think it would be
useful to note in a comment that this is our best-guess, though:

/*
 * The OHCI spec doesn't say what to do if the guest hands us
 * an endpoint descriptor which specifies a MaximumPacketSize
 * of zero, which would mean we can never actually make forward
 * progress transferring data to it. We choose to treat it as
 * an error.
 */

With something like that,

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

(Or if I've missed something in the spec, let me know...)

thanks
-- PMM

Re: [PATCH] hw/usb/hcd-ohci: check for MPS=0 to avoid infinite loop

[Guanni Qu]

Thanks Peter. v2 sent with your suggested comment added.

On Thu, Mar 19, 2026 at 6:34 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 18 Mar 2026 at 19:36, Jenny Guanni Qu <qguanni@gmail.com> wrote:
> >
> > When a guest sets MaxPacketSize to 0 in an OHCI Endpoint Descriptor,
> > ohci_service_td() transfers 0 bytes per iteration. The Transfer
> > Descriptor never completes because CBP never advances toward BE,
> > causing ohci_service_ed_list() to loop indefinitely and hang QEMU.
> >
> > Add a check for MPS==0 after extracting the field from ED flags.
> > If MPS is zero, call ohci_die() to reset the controller and return
> > an error, preventing the infinite loop.
> >
> > Fixes: CVE-2026-3890
> > Reported-by: Jenny Guanni Qu <qguanni@gmail.com>
> > Signed-off-by: Jenny Guanni Qu <qguanni@gmail.com>
> > ---
> >  hw/usb/hcd-ohci.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> > index 1aeed9286f..b93a99593e 100644
> > --- a/hw/usb/hcd-ohci.c
> > +++ b/hw/usb/hcd-ohci.c
> > @@ -956,6 +956,10 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
> >          if (len && dir != OHCI_TD_DIR_IN) {
> >              /* The endpoint may not allow us to transfer it all now */
> >              pktlen = (ed->flags & OHCI_ED_MPS_MASK) >> OHCI_ED_MPS_SHIFT;
> > +            if (pktlen == 0) {
> > +                ohci_die(ohci);
> > +                return 1;
> > +            }
>
> The OHCI spec doesn't (as far as I can tell) say what the
> hardware should do for this situation, so I guess "flag it
> as an error" is a reasonable choice. I think it would be
> useful to note in a comment that this is our best-guess, though:
>
> /*
>  * The OHCI spec doesn't say what to do if the guest hands us
>  * an endpoint descriptor which specifies a MaximumPacketSize
>  * of zero, which would mean we can never actually make forward
>  * progress transferring data to it. We choose to treat it as
>  * an error.
>  */
>
> With something like that,
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> (Or if I've missed something in the spec, let me know...)
>
> thanks
> -- PMM