Re: [PATCH] usbip: clear actual length after receive failure

[Shuah Khan <skhan@linuxfoundation.org>]

On 10/9/26 06:26, Sung Byeongchan wrote:
> A USB/IP peer supplies actual_length in RET_SUBMIT. usbip_recv_xbuff()
> rejects a value larger than the URB transfer buffer, but the VHCI error
> path gives the URB back with the peer-controlled length still present.
> usbfs subsequently uses urb->actual_length as its copy-to-userspace
> extent, reading beyond the transfer-buffer allocation.
> 
> On current mainline target 0c2669a9f4a1d, a 65-byte completion for a
> 64-byte URB produced a KASAN slab-out-of-bounds read in _copy_to_user in
> three fresh boots. INT_MAX reproduced the same first invalid consumer
> three times. The one-byte case modified only a boolean sentinel oracle;
> the copied value was not collected or characterized.
> 
> Clear actual_length when receiving the transfer buffer fails. No payload
> is valid on this error path, and zero prevents downstream consumers from
> reusing the rejected extent. The existing protocol-error path closes the
> broken transport.
> 
> The fixed kernel rejected both triggers without KASAN in three fresh
> boots each. Normal zero through buffer-length completions, OUT and ISO
> controls, high-bit counterexamples, disconnect cleanup, and a following
> valid transfer on a fresh transport remained functional.
> 
> This issue was found through manual source inspection assisted by an
> AI/LLM, then evaluated with local production-path dynamic validation.
> 
> Fixes: aabb5b833872 ("usbip: Fix error path of vhci_recv_ret_submit()")
> Assisted-by: LLM
> Signed-off-by: Sung Byeongchan <tjdqudcks0424@naver.com>
> ---
>   drivers/usb/usbip/vhci_rx.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c
> index a678e7c898375..741107a964660 100644
> --- a/drivers/usb/usbip/vhci_rx.c
> +++ b/drivers/usb/usbip/vhci_rx.c
> @@ -79,6 +79,7 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,
>   	/* recv transfer buffer */
>   	if (usbip_recv_xbuff(ud, urb) < 0) {
>   		urb->status = -EPROTO;
> +		urb->actual_length = 0;

What happens without this code change? The code path follows the
error leg because of EPROTO? So how does setting actual_length to
zero help?

thanks,
-- Shuah