Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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



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

(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.)

Was there a PR to detail the symptoms and track pullups?  Seems to me
this should be pulled up to all applicable branches, if it hasn't been
already.


Home | Main Index | Thread Index | Old Index