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

[Shuah Khan <skhan@linuxfoundation.org>]

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@linuxfoundation.org>

thanks,
-- Shuah