Re: hw/usb/hcd-ohci: Fix #1510, #303: pid not IN or OUT

[Thomas Huth <thuth@redhat.com>]

On 07/05/2024 22.20, Cord Amfmgm wrote:
> 
> 
> On Wed, Apr 24, 2024 at 3:43 PM Cord Amfmgm <dmamfmgm@gmail.com 
> <mailto:dmamfmgm@gmail.com>> wrote:
> 
>     On Thu, Apr 18, 2024 at 10:43 AM Michael Tokarev <mjt@tls.msk.ru
>     <mailto:mjt@tls.msk.ru>> wrote:
> 
>         06.02.2024 10:13, Cord Amfmgm wrote:
>          > This changes the ohci validation to not assert if invalid
>          > data is fed to the ohci controller. The poc suggested in
>          > https://bugs.launchpad.net/qemu/+bug/1907042
>         <https://bugs.launchpad.net/qemu/+bug/1907042>
>          > and then migrated to bug #303 does the following to
>          > feed it a SETUP pid and EndPt of 1:
>          >
>          >          uint32_t MaxPacket = 64;
>          >          uint32_t TDFormat = 0;
>          >          uint32_t Skip = 0;
>          >          uint32_t Speed = 0;
>          >          uint32_t Direction = 0;  /* #define OHCI_TD_DIR_SETUP 0 */
>          >          uint32_t EndPt = 1;
>          >          uint32_t FuncAddress = 0;
>          >          ed->attr = (MaxPacket << 16) | (TDFormat << 15) | (Skip
>         << 14)
>          >                     | (Speed << 13) | (Direction << 11) | (EndPt
>         << 7)
>          >                     | FuncAddress;
>          >          ed->tailp = /*TDQTailPntr= */ 0;
>          >          ed->headp = ((/*TDQHeadPntr= */ &td[0]) & 0xfffffff0)
>          >                     | (/* ToggleCarry= */ 0 << 1);
>          >          ed->next_ed = (/* NextED= */ 0 & 0xfffffff0)
>          >
>          > qemu-fuzz also caught the same issue in #1510. They are
>          > both fixed by this patch.
>          >
>          > The if (td.cbp > td.be <http://td.be>) logic in ohci_service_td()
>         causes an
>          > ohci_die(). My understanding of the OHCI spec 4.3.1.2
>          > Table 4-2 allows td.cbp to be one byte more than td.be
>         <http://td.be> to
>          > signal the buffer has zero length. The new check in qemu
>          > appears to have been added since qemu-4.2. This patch
>          > includes both fixes since they are located very close
>          > together.
>          >
>          > Signed-off-by: David Hubbard <dmamfmgm@gmail.com
>         <mailto:dmamfmgm@gmail.com>>

Your Signed-off-by line does not match the From: line ... could you please 
fix this? (see 
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#patch-emails-must-include-a-signed-off-by-line 
, too)

>         Wonder if this got lost somehow.  Or is it not needed?
> 
>         Thanks,
> 
>         /mjt
> 
> 
>     Friendly ping! Gerd, can you chime in with how you would like to
>     approach this? I still need this patch to unblock my qemu workflow -
>     custom OS development.
> 
> 
> Can I please ask for an update on this? I'm attempting to figure out if this 
> patch has been rejected and I need to resubmit / rework it at HEAD?

Looks like it's hard to find someone who still can review OHCI patches these 
days...

Anyway, I tried to get the reproducer running that had been added to the 
original patch (installed an Ubuntu 18.04 guest and compiled and ran that 
ohci_poc program in it), but so far, I failed. Could you please provide 
detailed steps how you can still produce this issue with the latest version 
of QEMU, please?

  Thanks,
   Thomas