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

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

[Yehyeong Lee]

usbg_data_write_cmpl() commits the WRITE buffer to the backend as soon as
the OUT request completes successfully, looking only at req->status and
never at req->actual.  A host that declares an 8192-byte WRITE but ends the
data-out after 4096 bytes still reaches target_execute_cmd() with the full
length.

The buffer is not zeroed (transport_generic_new_cmd() leaves zero_flag
clear for a data CDB), so the bytes that never arrived keep the previous
page contents, which the backend writes to the device for the host to read
back. The target core refuses a WRITE whose declared length is wrong, but
it does not see a data phase that simply falls short.

Fail the command when the data-out is short, using the transfer-failed path
that is already there, the way f_mass_storage treats a short bulk-out.  A
conforming host always sends the full length, so only a truncated transfer
is rejected.  There is no oops; poisoning the buffer and ending an
8192-byte write at 4096 bytes leaves 4096 stale bytes committed before the
change, and after it the command is aborted with nothing committed.

Fixes: c52661d60f63 ("usb-gadget: Initial merge of target module for UASP + BOT")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
 drivers/usb/gadget/function/f_tcm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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;
 	}
-- 
2.43.0

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

[sashiko-bot]

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

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

[Yehyeong Lee]

Thanks - both are addressed in v2.

Deadlock: you are right, the failure path reached bot_send_bad_status() with
cmd->data_len still at the full declared length, so it queued an OUT drain for
data the host had already stopped sending.  v2 clears cmd->data_len on this
path - the OUT data phase is over, so the status phase sends the CSW instead
of draining.  I confirmed the drain is no longer queued: ep_out stays idle and
only the CSW goes out on ep_in.

Log: v2 uses dev_err_ratelimited() for that message.

For what it is worth, the same bot_send_bad_status() drain is reachable
without this patch by any BOT WRITE that completes its data phase and then
fails in the backend; the failure branch lacks the moved_data guard the
good-status branch has.  That is a separate pre-existing issue, left alone
here.

v2: https://lore.kernel.org/linux-usb/20261013005854.2470384-1-yhlee@isslab.korea.ac.kr/

Thanks,
Yehyeong