Re: CVS commit: src/sys/dev/usb

[Nick Hudson <nick.hudson%gmx.co.uk@localhost>]

On 26/11/2025 04:36, Taylor R Campbell wrote:
> > Module Name:    src
> > Committed By:   manu
> > Date:           Fri Oct 10 12:08:54 UTC 2025
> >
> > Modified Files:
> >          src/sys/dev/usb: ucom.c
> >
> > Log Message:
> > Fix ucom input queue corruption on open-after-close
> >
> > When closing ucom(4) while a transfer was in progress, ucomreadcb()
> > is called with USBD_CANCELLED. Previously, the ucom_buffer was not
> > removed from sc->sc_ibuff_empty, leading to queue corruption when
> > reopening the device: ucomopen() calls ucomsubmitread(), which
> > queues the ucom_buffer again.
> >
> > We fix this by making sure the ucom_buffer is removed from queue
> > on USBD_CANCELLED or USBD_IOERROR.
> >
> > Approved by Nick Hudson, who rewrote the comment.
>
> Thanks, this looks good!
>
> I think it would also benefit from an assertion, just in case we ever
> expand UCOM_IN_BUFFS beyond 1:
>
>   	ub = SIMPLEQ_FIRST(&sc->sc_ibuff_empty);
>   	SIMPLEQ_REMOVE_HEAD(&sc->sc_ibuff_empty, ub_link);
> +	KASSERT(xfer == ub->ub_xfer);

I committed this and it has been pulled up to -10 and -11, and -9 is 
pending.

> (It's also not immediately obvious that it doesn't leak memory -- I
> think it doesn't because ucom_detach iterates over all sc_ibuff
> elements, whether empty or full, but the change left me wondering and
> I had to go reading to verify.)

It doesn't leak memory, but ucomsubmitread can leak ucom_buffs such that 
there are none to used any more if usbd_transfer fails.