Re: [PATCH 1/2] usbip: clear actual_length after a failed receive

[Shuah Khan]

On 11/10/26 14:29, Alan Stern wrote:
> On Tue, Nov 10, 2026 at 11:57:34AM -0700, Shuah Khan wrote:
> > On 10/29/26 22:13, Cheng Yongkang wrote:
> > > Hi Shuah,
> > >
> > > Without this change the URB is still completed to usbfs, with status
> > > -EPROTO but with the peer-supplied actual_length intact:
> > > >     vhci_recv_ret_submit()
> > >       usbip_pack_pdu() -> usbip_pack_ret_submit():
> > >           urb->actual_length = rpdu->actual_length;   (usbip_common.c:471)
> > >       usbip_recv_xbuff():
> > >           size = urb->actual_length;                  (usbip_common.c:849)
> > >           if (size > urb->transfer_buffer_length)     (usbip_common.c:856)
> > >                   goto error;  /* rejects the packet, returns < 0 */
> > >       urb->status = -EPROTO; goto error;
> > >     error:
> > >       usb_hcd_giveback_urb(hcd, urb, urb->status);    (vhci_rx.c:107)
> > >         -> urb->complete(urb)                         (hcd.c:1657)
> > >         -> usbfs async_completed()                    (devio.c:1911)
> > >
> > > usbfs copies data on reap whenever actual_length is non-zero, and does
> > > not look at the URB status:
> >
> > It sounds like fixing usbfs to check the URB status before copying
> > sounds like the right fix.

Cheng, I missed that the you sent usbfs patch to bound the length. Thank
you for both patches.

> No, that's not possible.  A transfer may terminate with an error partway
> through, after some data has been sent/received.  The URB status would
> then be a negative error code, but actual_length would still need to
> reflect the amount of data that usbfs has to copy back to userspace (for
> an IN transfer) or tell userspace was sent (for an OUT transfer).
>
> In general, the USB core relies on HCDs to set urb->actual_length
> correctly even when there was an error.
>
> > Changing vhci to zero out a filed in an URB that it received, itsn't
> > something we want to do. Why not fix this in usbfs and that would work
> > for other invalid lengths?
>
> HCDs are _supposed_ to set the actual_length field in URBs.  It's not
> a mistake to do so.

Alan, Thank you for weighing on this.

Greg, please pick this patch up.

Reviewed-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>

thanks,
-- Shuah