Re: [PATCH] usb: gadget: f_tcm: reject a short write data-out transfer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] usb: f_tcm: deadlock in BOT mode when rejecting short OUT transfer
- [Medium] usb: gadget: f_tcm: log flooding via un-ratelimited pr_err() on short transfers
--

commit 9b6b77ebba18d1575ab661cb225bb326116c901e
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

usb: gadget: f_tcm: reject a short write data-out transfer

This commit routes short write data-out transfers in usbg_data_write_cmpl()
to the error path. This prevents stale buffer data from being committed to
the backend when a transfer falls short of its declared length.

> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index 9e6d4f39900a2..54e45d3c79563 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -1090,7 +1090,8 @@ static void usbg_data_write_cmpl(struct usb_ep *ep, struct usb_request *req)
>  		return;
>  	}
>  
> -	if (req->status) {
> +	/* a short data-out left the buffer unwritten; don't commit stale data */
> +	if (req->status || req->actual != req->length) {
>  		pr_err("%s() state %d transfer failed\n", __func__, cmd->state);
>  		goto cleanup;
>  	}

[Severity: High]
Could routing a successful short transfer into this error path cause a
bus deadlock in BOT mode?

A short packet correctly finishes the OUT data phase from the host's
perspective, but detecting req->actual != req->length here routes execution
to the cleanup path to abort the command without signaling that the data
phase is over or clearing cmd->data_len.

The cleanup path calls transport_send_check_condition_and_sense(), causing
the target core to invoke the queue_status fabric callback with a CHECK
CONDITION. In BOT mode, this triggers bot_send_status(), which falls to its
error branch and calls bot_send_bad_status(cmd).

Because cmd->data_len still holds the original full declared transfer length,
bot_send_bad_status() sees cmd->data_len > 0 and queues another OUT request
to sink the supposedly unreceived data:

drivers/usb/gadget/function/f_tcm.c:bot_send_bad_status() {
	if (cmd->data_len) {
		if (cmd->is_read) {
			ep = fu->ep_in;
...
		usb_ep_queue(ep, req, GFP_KERNEL);
	} else {
		bot_enqueue_sense_code(fu, cmd);
	}
}

Since the host already ended the data phase with the short packet, it expects
a CSW on the IN endpoint and will never send more OUT data, which would
result in a permanent bus deadlock.

[Severity: Medium]
Will this allow an untrusted host to cause log flooding?

A USB host can repeatedly send valid short OUT packets where req->actual <
req->length but req->status == 0. This change newly routes these standard
short transfers into the error path. 

Because the pr_err() call is unconditional and not rate-limited, an attached
host can trigger this behavior by initiating a WRITE and terminating the
data phase early, spamming the kernel log and overwhelming the system.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012154422.2436134-1-yhlee@isslab.korea.ac.kr?part=1