Re: [PATCH] usbip: vudc: snapshot endpoint state under lock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usbip: vudc: memory exhaustion WARN via unvalidated transfer_buffer_length
--

commit 8f3791b0614070178b7b8445680ee73ac6933327
Author: Sung Byeongchan <tjdqudcks0424@naver.com>

usbip: vudc: snapshot endpoint state under lock

v_recv_cmd_submit() finds an endpoint and records its type while holding
udc->lock, but then drops the lock and rereads both ep->type and ep->desc.
The VUDC timer drops this lock around gadget setup callbacks, and
endpoint disable clears ep->desc under the lock.  A USB/IP host can
therefore race an ISO CMD_SUBMIT with endpoint disable and make
usb_endpoint_maxp() dereference NULL.

Copy the endpoint type and derived isochronous maximum packet size while
the lock still protects the descriptor.  Use the snapshots for
validation, URB allocation, and pipe setup after unlocking.

Cc: stable@vger.kernel.org

> diff --git a/drivers/usb/usbip/vudc_rx.c b/drivers/usb/usbip/vudc_rx.c
> index 51bb70837b902..276f645734484 100644
> --- a/drivers/usb/usbip/vudc_rx.c
> +++ b/drivers/usb/usbip/vudc_rx.c
[ ... ]
> @@ -139,7 +142,7 @@ static int v_recv_cmd_submit(struct vudc *udc,
>  		}
>  	}
>  
> -	ret = alloc_urb_from_cmd(&urb_p->urb, pdu, urb_p->ep->type);
> +	ret = alloc_urb_from_cmd(&urb_p->urb, pdu, urb_p->type);

[Severity: High]
Does this code allow an attacker to trigger a memory exhaustion WARN?

In v_recv_cmd_submit(), there appears to be no bounds check on
transfer_buffer_length for non-ISOC endpoints before it is passed to
alloc_urb_from_cmd().

Inside alloc_urb_from_cmd(), usbip_pack_pdu() copies the signed __s32
transfer_buffer_length field into the unsigned u32
urb->transfer_buffer_length.

If a malicious USB/IP host provides a negative length, such as -1, it would
become a massive u32 value like 0xFFFFFFFF. This bypasses the > 0 check and
is passed directly to kzalloc() with GFP_KERNEL:

alloc_urb_from_cmd() {
    ...
    if (urb->transfer_buffer_length > 0) {
        urb->transfer_buffer = kzalloc(urb->transfer_buffer_length,
            GFP_KERNEL);
        ...
    }
}

Without __GFP_NOWARN, requesting a size larger than KMALLOC_MAX_SIZE causes
the SLUB allocator to trigger a WARN_ON_ONCE_GFP. On systems configured with
panic_on_warn, this could result in a kernel panic and remote
Denial-of-Service.

Can a bounds check be added for non-ISOC endpoints before calling
alloc_urb_from_cmd()?

>  	if (ret) {
>  		usbip_event_add(&udc->ud, VUDC_EVENT_ERROR_MALLOC);
>  		ret = -ENOMEM;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007063054.26374-1-tjdqudcks0424@naver.com?part=1