| Message ID | 61593a434f543184f41df91a4ba858e1158fd33c.1657149962.git.Thinh.Nguyen@synopsys.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: f_tcm: Enhance UASP driver | expand |
On 07.07.22 01:35, Thinh Nguyen wrote: > CHECK CONDITION returns sense data, and sense data is minimum 8 bytes > long plus additional sense data length in the data buffer. Don't just > set the allocated buffer length to the cmd->scsi_sense_length and check > the sense data for that. > > See SPC4-r37 section 4.5.2.1. > > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > --- > drivers/target/target_core_transport.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c > index bc1e4a7c4538..9734952a6228 100644 > --- a/drivers/target/target_core_transport.c > +++ b/drivers/target/target_core_transport.c > @@ -3459,12 +3459,20 @@ static void translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason) > > cmd->se_cmd_flags |= SCF_EMULATED_TASK_SENSE; > cmd->scsi_status = SAM_STAT_CHECK_CONDITION; > - cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; > + > + /* > + * CHECK CONDITION returns sense data, and sense data is minimum 8 > + * bytes long. See SPC4-r37 section 4.5.2.1. > + */ > + cmd->scsi_sense_length = 8; > + > scsi_build_sense_buffer(desc_format, buffer, key, asc, ascq); > if (sd->add_sense_info) > WARN_ON_ONCE(scsi_set_sense_information(buffer, > cmd->scsi_sense_length, > cmd->sense_info) < 0); > + /* Additional sense data length */ > + cmd->scsi_sense_length += buffer[7]; Doesn't this need to check for overflows? Regards Oliver
On 7/7/2022, Oliver Neukum wrote: > > On 07.07.22 01:35, Thinh Nguyen wrote: >> CHECK CONDITION returns sense data, and sense data is minimum 8 bytes >> long plus additional sense data length in the data buffer. Don't just >> set the allocated buffer length to the cmd->scsi_sense_length and check >> the sense data for that. >> >> See SPC4-r37 section 4.5.2.1. >> >> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> >> --- >> drivers/target/target_core_transport.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c >> index bc1e4a7c4538..9734952a6228 100644 >> --- a/drivers/target/target_core_transport.c >> +++ b/drivers/target/target_core_transport.c >> @@ -3459,12 +3459,20 @@ static void translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason) >> >> cmd->se_cmd_flags |= SCF_EMULATED_TASK_SENSE; >> cmd->scsi_status = SAM_STAT_CHECK_CONDITION; >> - cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; >> + >> + /* >> + * CHECK CONDITION returns sense data, and sense data is minimum 8 >> + * bytes long. See SPC4-r37 section 4.5.2.1. >> + */ >> + cmd->scsi_sense_length = 8; >> + >> scsi_build_sense_buffer(desc_format, buffer, key, asc, ascq); >> if (sd->add_sense_info) >> WARN_ON_ONCE(scsi_set_sense_information(buffer, >> cmd->scsi_sense_length, >> cmd->sense_info) < 0); >> + /* Additional sense data length */ >> + cmd->scsi_sense_length += buffer[7]; > Doesn't this need to check for overflows? I missed that. Will fix. Thanks, Thinh
Hi Thinh, On Wed, Jul 06, 2022 at 04:35:32PM -0700, Thinh Nguyen wrote: > CHECK CONDITION returns sense data, and sense data is minimum 8 bytes > long plus additional sense data length in the data buffer. Don't just > set the allocated buffer length to the cmd->scsi_sense_length and check > the sense data for that. > > See SPC4-r37 section 4.5.2.1. > > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > --- > drivers/target/target_core_transport.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c > index bc1e4a7c4538..9734952a6228 100644 > --- a/drivers/target/target_core_transport.c > +++ b/drivers/target/target_core_transport.c > @@ -3459,12 +3459,20 @@ static void translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason) > > cmd->se_cmd_flags |= SCF_EMULATED_TASK_SENSE; > cmd->scsi_status = SAM_STAT_CHECK_CONDITION; > - cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; > + > + /* > + * CHECK CONDITION returns sense data, and sense data is minimum 8 > + * bytes long. See SPC4-r37 section 4.5.2.1. > + */ > + cmd->scsi_sense_length = 8; > + > scsi_build_sense_buffer(desc_format, buffer, key, asc, ascq); > if (sd->add_sense_info) > WARN_ON_ONCE(scsi_set_sense_information(buffer, > cmd->scsi_sense_length, scsi_set_sense_information()'s second argument is buffer length; send there TRANSPORT_SENSE_BUFFER and the patch will be correct. > cmd->sense_info) < 0); > + /* Additional sense data length */ > + cmd->scsi_sense_length += buffer[7]; > } > > int
On 7/7/2022, Dmitry Bogdanov wrote: > Hi Thinh, > > On Wed, Jul 06, 2022 at 04:35:32PM -0700, Thinh Nguyen wrote: >> CHECK CONDITION returns sense data, and sense data is minimum 8 bytes >> long plus additional sense data length in the data buffer. Don't just >> set the allocated buffer length to the cmd->scsi_sense_length and check >> the sense data for that. >> >> See SPC4-r37 section 4.5.2.1. >> >> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> >> --- >> drivers/target/target_core_transport.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c >> index bc1e4a7c4538..9734952a6228 100644 >> --- a/drivers/target/target_core_transport.c >> +++ b/drivers/target/target_core_transport.c >> @@ -3459,12 +3459,20 @@ static void translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason) >> >> cmd->se_cmd_flags |= SCF_EMULATED_TASK_SENSE; >> cmd->scsi_status = SAM_STAT_CHECK_CONDITION; >> - cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; >> + >> + /* >> + * CHECK CONDITION returns sense data, and sense data is minimum 8 >> + * bytes long. See SPC4-r37 section 4.5.2.1. >> + */ >> + cmd->scsi_sense_length = 8; >> + >> scsi_build_sense_buffer(desc_format, buffer, key, asc, ascq); >> if (sd->add_sense_info) >> WARN_ON_ONCE(scsi_set_sense_information(buffer, >> cmd->scsi_sense_length, > scsi_set_sense_information()'s second argument is buffer length; send > there TRANSPORT_SENSE_BUFFER and the patch will be correct. Sure, I'll do that. Thanks, Thinh >> cmd->sense_info) < 0); >> + /* Additional sense data length */ >> + cmd->scsi_sense_length += buffer[7]; >> } >> >> int
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index bc1e4a7c4538..9734952a6228 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -3459,12 +3459,20 @@ static void translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason) cmd->se_cmd_flags |= SCF_EMULATED_TASK_SENSE; cmd->scsi_status = SAM_STAT_CHECK_CONDITION; - cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; + + /* + * CHECK CONDITION returns sense data, and sense data is minimum 8 + * bytes long. See SPC4-r37 section 4.5.2.1. + */ + cmd->scsi_sense_length = 8; + scsi_build_sense_buffer(desc_format, buffer, key, asc, ascq); if (sd->add_sense_info) WARN_ON_ONCE(scsi_set_sense_information(buffer, cmd->scsi_sense_length, cmd->sense_info) < 0); + /* Additional sense data length */ + cmd->scsi_sense_length += buffer[7]; } int
CHECK CONDITION returns sense data, and sense data is minimum 8 bytes long plus additional sense data length in the data buffer. Don't just set the allocated buffer length to the cmd->scsi_sense_length and check the sense data for that. See SPC4-r37 section 4.5.2.1. Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> --- drivers/target/target_core_transport.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)