Mikolaj Kucharski <[email protected]> wrote:
> On Mon, Apr 13, 2020 at 12:34:48PM -0600, Theo de Raadt wrote:
> > Mikolaj Kucharski <[email protected]> wrote:
> >
> > > Index: sys/dev/usb/ehci.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/dev/usb/ehci.c,v
> > > retrieving revision 1.210
> > > diff -u -p -u -r1.210 ehci.c
> > > --- sys/dev/usb/ehci.c 3 Apr 2020 20:11:47 -0000 1.210
> > > +++ sys/dev/usb/ehci.c 13 Apr 2020 18:04:51 -0000
> > > @@ -869,7 +869,9 @@ ehci_idone(struct usbd_xfer *xfer)
> > > struct ehci_xfer *ex = (struct ehci_xfer *)xfer;
> > > struct ehci_soft_qtd *sqtd;
> > > u_int32_t status = 0, nstatus = 0;
> > > - int actlen, cerr;
> > > + u_int32_t actlen, x_actlen;
> > > + int cerr;
> > > + u_int32_t x1, x2;
> > >
> > > #ifdef DIAGNOSTIC
> > > {
> > > @@ -903,8 +905,33 @@ ehci_idone(struct usbd_xfer *xfer)
> > > actlen += sqtd->len - EHCI_QTD_GET_BYTES(status);
> > > }
> > >
> > > + if (xfer->length < actlen) {
> > > + printf("XXX ehci_idone: len=%u, actlen=%u, xf_actlen=%u, "
> > > + "status=0x%x\n", xfer->length, actlen, xfer->actlen,
> > > status);
> >
> > actlen hasn't been intialized, so you've introduced a new bug and
> > aren't chasing the same bug. It is random stack garbage.
>
> With my diff actlen is initialized to 0 on line 891. It's existing code
> already in CVS, where that initialization exists on line 889. In my diff
> initialization of actlen is not visible as I didn't touch those lines,
> so it's outside of diffs context. Anyway, I don't see what you see.
Then you need to look at why this crazy value is composed here in this
loop:
actlen = 0;
for (sqtd = ex->sqtdstart; sqtd != NULL; sqtd = sqtd->nextqtd) {
usb_syncmem(&sqtd->dma, sqtd->offs, sizeof(sqtd->qtd),
BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
nstatus = letoh32(sqtd->qtd.qtd_status);
if (nstatus & EHCI_QTD_ACTIVE)
break;
status = nstatus;
/* halt is ok if descriptor is last, and complete */
if (sqtd->qtd.qtd_next == htole32(EHCI_LINK_TERMINATE) &&
EHCI_QTD_GET_BYTES(status) == 0)
status &= ~EHCI_QTD_HALTED;
if (EHCI_QTD_GET_PID(status) != EHCI_QTD_PID_SETUP)
actlen += sqtd->len - EHCI_QTD_GET_BYTES(status);
}
