diff mbox series

[v8,06/33] scsi: sd: Fix sshdr use in read_capacity_16

Message ID 20230614071719.6372-7-michael.christie@oracle.com
State Superseded
Headers show
Series scsi: Allow scsi_execute users to control retries | expand

Commit Message

Mike Christie June 14, 2023, 7:16 a.m. UTC 
If the_reset is < 0, scsi_execute_cmd will not have set the sshdr, so we
can't access it.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/scsi/sd.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

John Garry June 15, 2023, 3:41 p.m. UTC | #1 
On 14/06/2023 08:16, Mike Christie wrote:
> If the_reset is < 0, scsi_execute_cmd will not have set the sshdr,

The code change below now means that we only access for the_result > 0, 
so should this be "If the_reset is <= 0 ..."

> so we
> can't access it.

shouldn't access it, I'd say.

> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
>   drivers/scsi/sd.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 84edbc0a5747..a2daa96e5c87 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2428,11 +2428,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
>   		the_result = scsi_execute_cmd(sdp, cmd, REQ_OP_DRV_IN,
>   					      buffer, RC16_LEN, SD_TIMEOUT,
>   					      sdkp->max_retries, &exec_args);
> -
> -		if (media_not_present(sdkp, &sshdr))
> -			return -ENODEV;
> -
>   		if (the_result > 0) {
> +			if (media_not_present(sdkp, &sshdr))
> +				return -ENODEV;
> +
>   			sense_valid = scsi_sense_valid(&sshdr);
>   			if (sense_valid &&
>   			    sshdr.sense_key == ILLEGAL_REQUEST &&
Mike Christie June 15, 2023, 4:20 p.m. UTC | #2 
On 6/15/23 10:41 AM, John Garry wrote:
> On 14/06/2023 08:16, Mike Christie wrote:
>> If the_reset is < 0, scsi_execute_cmd will not have set the sshdr,
> 
> The code change below now means that we only access for the_result > 0, so should this be "If the_reset is <= 0 ..."
> 

Yeah, will fix here and the other patches.


>> so we
>> can't access it.
> 
> shouldn't access it, I'd say.

That's fine with me. Will fix here and the other patches.


> 
>>
>> Signed-off-by: Mike Christie <michael.christie@oracle.com>
>> ---
>>   drivers/scsi/sd.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>> index 84edbc0a5747..a2daa96e5c87 100644
>> --- a/drivers/scsi/sd.c
>> +++ b/drivers/scsi/sd.c
>> @@ -2428,11 +2428,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
>>           the_result = scsi_execute_cmd(sdp, cmd, REQ_OP_DRV_IN,
>>                             buffer, RC16_LEN, SD_TIMEOUT,
>>                             sdkp->max_retries, &exec_args);
>> -
>> -        if (media_not_present(sdkp, &sshdr))
>> -            return -ENODEV;
>> -
>>           if (the_result > 0) {
>> +            if (media_not_present(sdkp, &sshdr))
>> +                return -ENODEV;
>> +
>>               sense_valid = scsi_sense_valid(&sshdr);
>>               if (sense_valid &&
>>                   sshdr.sense_key == ILLEGAL_REQUEST &&
>
diff mbox series

Patch

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 84edbc0a5747..a2daa96e5c87 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2428,11 +2428,10 @@  static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
 		the_result = scsi_execute_cmd(sdp, cmd, REQ_OP_DRV_IN,
 					      buffer, RC16_LEN, SD_TIMEOUT,
 					      sdkp->max_retries, &exec_args);
-
-		if (media_not_present(sdkp, &sshdr))
-			return -ENODEV;
-
 		if (the_result > 0) {
+			if (media_not_present(sdkp, &sshdr))
+				return -ENODEV;
+
 			sense_valid = scsi_sense_valid(&sshdr);
 			if (sense_valid &&
 			    sshdr.sense_key == ILLEGAL_REQUEST &&