On 7/8/2022, Dmitry Bogdanov wrote:
> 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@xxxxxxxxxxxx>
>> ---
>> 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.
Sure, I'll do that.
Thanks,
Thinh
>>
>> 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)
