Re: [PATCH v6 1/2] usb: gadget: f_tcm: queue the BOT status requests with GFP_ATOMIC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: f_tcm: command leak and session hang on usb_ep_queue failure
--

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

usb: gadget: f_tcm: queue the BOT status requests with GFP_ATOMIC

This commit changes the allocation flags used when queuing BOT status requests
in the f_tcm USB gadget function. Since the completion callbacks are executed
with interrupts disabled, GFP_KERNEL is replaced with GFP_ATOMIC to prevent
sleeping in an atomic context.

> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index 9e6d4f39900a2..12e2b20e33b92 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -139,7 +139,7 @@ static void bot_send_bad_status(struct usbg_cmd *cmd)
>  		req->complete = bot_err_compl;
>  		req->context = cmd;
>  		req->buf = fu->cmd[0].buf;
> -		usb_ep_queue(ep, req, GFP_KERNEL);
> +		usb_ep_queue(ep, req, GFP_ATOMIC);

[Severity: High]
In bot_send_bad_status(), if usb_ep_queue() fails, do the target core command
(usbg_cmd) and dynamically allocated buffers (such as cmd->data_buf) leak here?
The return value is completely ignored, which means the completion callback
is never invoked to free them.

Since the target core expects all commands to complete before session teardown,
could this cause target_wait_for_sess_cmds() to loop forever waiting for
completion, resulting in a permanent kernel hang? A faulty or untrusted USB host
can trigger this by resetting the bus or disconnecting at the exact moment
usb_ep_queue() attempts to queue the status request.

>  	} else {
>  		bot_enqueue_sense_code(fu, cmd);
>  	}
> @@ -167,7 +167,7 @@ static int bot_send_status(struct usbg_cmd *cmd, bool moved_data)
>  		csw->Status = US_BULK_STAT_OK;
>  		fu->bot_status.req->context = cmd;
>  
> -		ret = usb_ep_queue(fu->ep_in, fu->bot_status.req, GFP_KERNEL);
> +		ret = usb_ep_queue(fu->ep_in, fu->bot_status.req, GFP_ATOMIC);
>  		if (ret)
>  			pr_err("%s(%d) ERR: %d\n", __func__, __LINE__, ret);

[Severity: High]
Similarly in bot_send_status(), the return value of usb_ep_queue() is checked
but only logged. If the queue fails, does this lead to the same command leak
and session hang because the status completion handler is never called to free
the command?

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