| Message ID | 9cb2ee13f6edbcfd9b719e4b4b53f7cec0080d3b.1657149962.git.Thinh.Nguyen@synopsys.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: f_tcm: Enhance UASP driver | expand |
Hi Thinh, On Wed, Jul 06, 2022 at 04:35:38PM -0700, Thinh Nguyen wrote: > The transport_get_sense_buffer() sets sense data length to be the > allocated sense buffer length TRANSPORT_SENSE_BUFFER. However, the sense > data length is depending on the sense data description. Check the sense > data for cmd->scsi_sense_length. > > See SPC4-R37 section 4.5.2.1. > > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > --- > drivers/target/target_core_transport.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c > index 9734952a6228..2fc4ba1814c6 100644 > --- a/drivers/target/target_core_transport.c > +++ b/drivers/target/target_core_transport.c > @@ -771,8 +771,6 @@ static unsigned char *transport_get_sense_buffer(struct se_cmd *cmd) > if (cmd->se_cmd_flags & SCF_SENT_CHECK_CONDITION) > return NULL; > > - cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; > - > pr_debug("HBA_[%u]_PLUG[%s]: Requesting sense for SAM STATUS: 0x%02x\n", > dev->se_hba->hba_id, dev->transport->name, cmd->scsi_status); > return cmd->sense_buffer; > @@ -791,7 +789,12 @@ void transport_copy_sense_to_cmd(struct se_cmd *cmd, unsigned char *sense) > } > > cmd->se_cmd_flags |= SCF_TRANSPORT_TASK_SENSE; > - memcpy(cmd_sense_buf, sense, cmd->scsi_sense_length); > + memcpy(cmd_sense_buf, sense, TRANSPORT_SENSE_BUFFER); > + > + /* Sense data length = min sense data + additional sense data length */ > + cmd->scsi_sense_length = min_t(int, 8 + cmd_sense_buf[7], > + TRANSPORT_SENSE_BUFFER); Looks a bit strange - to copy big amount of data and then to reduce the length. Better vice versa - set correct length and copy only that data. > + > spin_unlock_irqrestore(&cmd->t_state_lock, flags); > } > EXPORT_SYMBOL(transport_copy_sense_to_cmd);
On 7/7/2022, Dmitry Bogdanov wrote: > Hi Thinh, > On Wed, Jul 06, 2022 at 04:35:38PM -0700, Thinh Nguyen wrote: >> The transport_get_sense_buffer() sets sense data length to be the >> allocated sense buffer length TRANSPORT_SENSE_BUFFER. However, the sense >> data length is depending on the sense data description. Check the sense >> data for cmd->scsi_sense_length. >> >> See SPC4-R37 section 4.5.2.1. >> >> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> >> --- >> drivers/target/target_core_transport.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c >> index 9734952a6228..2fc4ba1814c6 100644 >> --- a/drivers/target/target_core_transport.c >> +++ b/drivers/target/target_core_transport.c >> @@ -771,8 +771,6 @@ static unsigned char *transport_get_sense_buffer(struct se_cmd *cmd) >> if (cmd->se_cmd_flags & SCF_SENT_CHECK_CONDITION) >> return NULL; >> >> - cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; >> - >> pr_debug("HBA_[%u]_PLUG[%s]: Requesting sense for SAM STATUS: 0x%02x\n", >> dev->se_hba->hba_id, dev->transport->name, cmd->scsi_status); >> return cmd->sense_buffer; >> @@ -791,7 +789,12 @@ void transport_copy_sense_to_cmd(struct se_cmd *cmd, unsigned char *sense) >> } >> >> cmd->se_cmd_flags |= SCF_TRANSPORT_TASK_SENSE; >> - memcpy(cmd_sense_buf, sense, cmd->scsi_sense_length); >> + memcpy(cmd_sense_buf, sense, TRANSPORT_SENSE_BUFFER); >> + >> + /* Sense data length = min sense data + additional sense data length */ >> + cmd->scsi_sense_length = min_t(int, 8 + cmd_sense_buf[7], >> + TRANSPORT_SENSE_BUFFER); > Looks a bit strange - to copy big amount of data and then to reduce the > length. Better vice versa - set correct length and copy only that data. Sure, I'll do that. Thanks, Thinh >> + >> spin_unlock_irqrestore(&cmd->t_state_lock, flags); >> } >> EXPORT_SYMBOL(transport_copy_sense_to_cmd);
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 9734952a6228..2fc4ba1814c6 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -771,8 +771,6 @@ static unsigned char *transport_get_sense_buffer(struct se_cmd *cmd) if (cmd->se_cmd_flags & SCF_SENT_CHECK_CONDITION) return NULL; - cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; - pr_debug("HBA_[%u]_PLUG[%s]: Requesting sense for SAM STATUS: 0x%02x\n", dev->se_hba->hba_id, dev->transport->name, cmd->scsi_status); return cmd->sense_buffer; @@ -791,7 +789,12 @@ void transport_copy_sense_to_cmd(struct se_cmd *cmd, unsigned char *sense) } cmd->se_cmd_flags |= SCF_TRANSPORT_TASK_SENSE; - memcpy(cmd_sense_buf, sense, cmd->scsi_sense_length); + memcpy(cmd_sense_buf, sense, TRANSPORT_SENSE_BUFFER); + + /* Sense data length = min sense data + additional sense data length */ + cmd->scsi_sense_length = min_t(int, 8 + cmd_sense_buf[7], + TRANSPORT_SENSE_BUFFER); + spin_unlock_irqrestore(&cmd->t_state_lock, flags); } EXPORT_SYMBOL(transport_copy_sense_to_cmd);
The transport_get_sense_buffer() sets sense data length to be the allocated sense buffer length TRANSPORT_SENSE_BUFFER. However, the sense data length is depending on the sense data description. Check the sense data for cmd->scsi_sense_length. See SPC4-R37 section 4.5.2.1. Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> --- drivers/target/target_core_transport.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)