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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: gadget: f_tcm: info leak of uninit heap via dummy IN data
--

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

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

usbg_data_write_cmpl() copies se_cmd->data_length bytes out of the bounce
buffer into the command's scatter-gather list without checking how much the
host actually sent. A bot host can declare a larger out transfer than it
delivers and end the data stage with a short packet.

Fail the command when req->actual does not match req->length instead of
committing the stale tail.

> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index 12e2b20e33b92..a37a3ec8e7a4d 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -120,7 +120,8 @@ static void bot_send_bad_status(struct usbg_cmd *cmd)
>  
>  	csw->Residue = cpu_to_le32(cmd->data_len);
>  
> -	if (cmd->data_len) {
> +	/* nothing to drain once the OUT data phase has completed */
> +	if (cmd->data_len && !(cmd->flags & USBG_CMD_OUT_DATA_DONE)) {
>  		if (cmd->is_read) {
>  			ep = fu->ep_in;
>  			req = fu->bot_req_in;

[Severity: High]
Does this code transmit uninitialized kernel heap memory to an untrusted
usb host?

Looking at bot_prepare_reqs(), fu->cmd[0].buf is allocated with kmalloc()
without being zeroed:

drivers/usb/gadget/function/f_tcm.c:bot_prepare_reqs() {
    ...
	fu->cmd[0].buf = kmalloc(fu->ep_out->maxpacket, GFP_KERNEL);
	if (!fu->cmd[0].buf)
		goto err_buf;
    ...
}

The usb host writes a 31-byte bot cbw into it, leaving the remaining space
(up to 993 bytes for superspeed endpoints) uninitialized.

When a data-in command (host read) is rejected prior to its data phase,
this uninitialized fu->cmd[0].buf is explicitly queued further down inside
this same bot_send_bad_status() block to satisfy the host's requested length:

		req->buf = fu->cmd[0].buf;
		usb_ep_queue(ep, req, GFP_ATOMIC);

The completion handler then continuously re-queues this same buffer until
the host's requested data length (which can be up to 4GB) is fully satisfied:

drivers/usb/gadget/function/f_tcm.c:bot_err_compl() {
    ...
	if (cmd->data_len) {
        ...
		usb_ep_queue(ep, req, GFP_ATOMIC);
		return;
	}
    ...
}

Can an attached usb host trigger this by sending an invalid data-in command,
creating a large information leak?

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