Re: [PATCH 18/36] usb: gadget: f_tcm: Don't set static stream_id

Dmitry Bogdanov
2022-07-08T07:00:41Z

On Wed, Jul 06, 2022 at 04:36:15PM -0700, Thinh Nguyen wrote:
> Host can assign stream ID value greater than number of streams
> allocated. The tcm function needs to keep track of which stream is
> available to assign the stream ID. This patch doesn't track that, but at
> least it makes sure that there's no Oops if the host send tag with a
> value greater than the number of supported streams.
>
> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
> drivers/usb/gadget/function/f_tcm.c | 32 +++++------------------------
> 1 file changed, 5 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index 270ec631481d..7721216dc9bc 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -532,6 +532,7 @@ static int uasp_prepare_r_request(struct usbg_cmd *cmd)
> }
>
> stream->req_in->is_last = 1;
> + stream->req_in->stream_id = cmd->tag;
> stream->req_in->complete = uasp_status_data_cmpl;
> stream->req_in->length = se_cmd->data_length;
> stream->req_in->context = cmd;
> @@ -556,6 +557,7 @@ static void uasp_prepare_status(struct usbg_cmd *cmd)
> iu->len = cpu_to_be16(se_cmd->scsi_sense_length);
> iu->status = se_cmd->scsi_status;
> stream->req_status->is_last = 1;
> + stream->req_status->stream_id = cmd->tag;
> stream->req_status->context = cmd;
> stream->req_status->length = se_cmd->scsi_sense_length + 16;
> stream->req_status->buf = iu;
> @@ -786,19 +788,6 @@ static int uasp_alloc_cmd(struct f_uas *fu)
> return -ENOMEM;
> }
>
> -static void uasp_setup_stream_res(struct f_uas *fu, int max_streams)
> -{
> - int i;
> -
> - for (i = 0; i < max_streams; i++) {
> - struct uas_stream *s = &fu->stream[i];
> -
> - s->req_in->stream_id = i + 1;
> - s->req_out->stream_id = i + 1;
> - s->req_status->stream_id = i + 1;
> - }
> -}
> -
> static int uasp_prepare_reqs(struct f_uas *fu)
> {
> int ret;
> @@ -819,7 +808,6 @@ static int uasp_prepare_reqs(struct f_uas *fu)
> ret = uasp_alloc_cmd(fu);
> if (ret)
> goto err_free_stream;
> - uasp_setup_stream_res(fu, max_streams);
>
> ret = usb_ep_queue(fu->ep_cmd, fu->cmd.req, GFP_ATOMIC);
> if (ret)
> @@ -995,6 +983,7 @@ static int usbg_prepare_w_request(struct usbg_cmd *cmd, struct usb_request *req)
> }
>
> req->is_last = 1;
> + req->stream_id = cmd->tag;
> req->complete = usbg_data_write_cmpl;
> req->length = se_cmd->data_length;
> req->context = cmd;
> @@ -1125,16 +1114,8 @@ static int usbg_submit_command(struct f_uas *fu,
> }
> memcpy(cmd->cmd_buf, cmd_iu->cdb, cmd_len);
>
> - if (fu->flags & USBG_USE_STREAMS) {
> - if (cmd->tag > UASP_SS_EP_COMP_NUM_STREAMS)
> - goto err;
> - if (!cmd->tag)
> - cmd->stream = &fu->stream[0];
> - else
> - cmd->stream = &fu->stream[cmd->tag - 1];
> - } else {
> - cmd->stream = &fu->stream[0];
> - }
> + cmd->stream = &fu->stream[cmd->tag %
> + UASP_SS_EP_COMP_NUM_STREAMS];
Use USBG_NUM_CMDS instead of UASP_SS_EP_COMP_NUM_STREAMS like in other
places.
>
> switch (cmd_iu->prio_attr & 0x7) {
> case UAS_HEAD_TAG:
> @@ -1161,9 +1142,6 @@ static int usbg_submit_command(struct f_uas *fu,
> queue_work(tpg->workqueue, &cmd->work);
>
> return 0;
> -err:
> - usbg_release_cmd(&cmd->se_cmd);
> - return -EINVAL;
> }
>
> static void bot_cmd_work(struct work_struct *work)