Re: [PATCH v7 1/3] 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: unhandled usb_ep_queue() failure leaks TCM command
--

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

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

This commit fixes sleeping in atomic context by changing the queue flags
in bot_send_bad_status() and bot_send_status() from GFP_KERNEL to
GFP_ATOMIC.

> 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]
Does this code leak the TCM command and hang the USB BOT session if
usb_ep_queue() fails?

By changing the allocation flag to GFP_ATOMIC, usb_ep_queue() is more
likely to fail under memory pressure.

Because the return value isn't checked here, if it fails, the endpoint
completion callback (bot_err_compl) will never execute. The callback is
responsible for queuing the next CBW (Command Block Wrapper) to receive
subsequent commands, so the BOT instance will stall.

The command is also not freed by transport_generic_free_cmd(), leading
to eventual session tag exhaustion. A host-initiated USB Bulk Reset
(US_BULK_RESET_REQUEST) resumes listening for new CBWs but does not
explicitly abort the leaked TCM command.

>  	} 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]
Does this code leak the TCM command and hang the USB BOT session if
usb_ep_queue() fails?

Similarly, if the GFP_ATOMIC allocation fails under memory pressure, the
error is only logged and we unconditionally return 0.

The endpoint completion callback will never execute, which stalls the
BOT session because the next CBW won't be queued. The command is
permanently leaked.

>  	} else {
>  		cmd->csw_code = US_BULK_STAT_FAIL;
>  		bot_send_bad_status(cmd);

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