| From e78a68b1a0fc11275661ecae18d0d02ffe56594f Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 27 Apr 2021 10:30:09 +0200 |
| Subject: scsi: core: Fixup calling convention for scsi_mode_sense() |
| |
| From: Hannes Reinecke <hare@suse.de> |
| |
| [ Upstream commit 8793613de913e03e7c884f4cc56e350bc716431e ] |
| |
| The description for scsi_mode_sense() claims to return the number of valid |
| bytes on success, which is not what the code does. Additionally there is |
| no gain in returning the SCSI status, as everything the callers do is to |
| check against scsi_result_is_good(), which is what scsi_mode_sense() does |
| already. So change the calling convention to return a standard error code |
| on failure, and 0 on success, and adapt the description and all callers. |
| |
| Link: https://lore.kernel.org/r/20210427083046.31620-4-hare@suse.de |
| Reviewed-by: Bart Van Assche <bvanassche@acm.org> |
| Signed-off-by: Hannes Reinecke <hare@suse.de> |
| Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/scsi/scsi_lib.c | 10 ++++++---- |
| drivers/scsi/scsi_transport_sas.c | 9 ++++----- |
| drivers/scsi/sd.c | 12 ++++++------ |
| drivers/scsi/sr.c | 2 +- |
| 4 files changed, 17 insertions(+), 16 deletions(-) |
| |
| diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c |
| index e172c660dcd5..1b02556f9ec0 100644 |
| --- a/drivers/scsi/scsi_lib.c |
| +++ b/drivers/scsi/scsi_lib.c |
| @@ -2081,9 +2081,7 @@ EXPORT_SYMBOL_GPL(scsi_mode_select); |
| * @sshdr: place to put sense data (or NULL if no sense to be collected). |
| * must be SCSI_SENSE_BUFFERSIZE big. |
| * |
| - * Returns zero if unsuccessful, or the header offset (either 4 |
| - * or 8 depending on whether a six or ten byte command was |
| - * issued) if successful. |
| + * Returns zero if successful, or a negative error number on failure |
| */ |
| int |
| scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage, |
| @@ -2130,6 +2128,8 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage, |
| |
| result = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buffer, len, |
| sshdr, timeout, retries, NULL); |
| + if (result < 0) |
| + return result; |
| |
| /* This code looks awful: what it's doing is making sure an |
| * ILLEGAL REQUEST sense return identifies the actual command |
| @@ -2174,13 +2174,15 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage, |
| data->block_descriptor_length = buffer[3]; |
| } |
| data->header_length = header_length; |
| + result = 0; |
| } else if ((status_byte(result) == CHECK_CONDITION) && |
| scsi_sense_valid(sshdr) && |
| sshdr->sense_key == UNIT_ATTENTION && retry_count) { |
| retry_count--; |
| goto retry; |
| } |
| - |
| + if (result > 0) |
| + result = -EIO; |
| return result; |
| } |
| EXPORT_SYMBOL(scsi_mode_sense); |
| diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c |
| index c9abed8429c9..4a96fb05731d 100644 |
| --- a/drivers/scsi/scsi_transport_sas.c |
| +++ b/drivers/scsi/scsi_transport_sas.c |
| @@ -1229,16 +1229,15 @@ int sas_read_port_mode_page(struct scsi_device *sdev) |
| char *buffer = kzalloc(BUF_SIZE, GFP_KERNEL), *msdata; |
| struct sas_end_device *rdev = sas_sdev_to_rdev(sdev); |
| struct scsi_mode_data mode_data; |
| - int res, error; |
| + int error; |
| |
| if (!buffer) |
| return -ENOMEM; |
| |
| - res = scsi_mode_sense(sdev, 1, 0x19, buffer, BUF_SIZE, 30*HZ, 3, |
| - &mode_data, NULL); |
| + error = scsi_mode_sense(sdev, 1, 0x19, buffer, BUF_SIZE, 30*HZ, 3, |
| + &mode_data, NULL); |
| |
| - error = -EINVAL; |
| - if (!scsi_status_is_good(res)) |
| + if (error) |
| goto out; |
| |
| msdata = buffer + mode_data.header_length + |
| diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c |
| index a0356f3707b8..3431ac12b730 100644 |
| --- a/drivers/scsi/sd.c |
| +++ b/drivers/scsi/sd.c |
| @@ -2683,18 +2683,18 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer) |
| * 5: Illegal Request, Sense Code 24: Invalid field in |
| * CDB. |
| */ |
| - if (!scsi_status_is_good(res)) |
| + if (res < 0) |
| res = sd_do_mode_sense(sdkp, 0, 0, buffer, 4, &data, NULL); |
| |
| /* |
| * Third attempt: ask 255 bytes, as we did earlier. |
| */ |
| - if (!scsi_status_is_good(res)) |
| + if (res < 0) |
| res = sd_do_mode_sense(sdkp, 0, 0x3F, buffer, 255, |
| &data, NULL); |
| } |
| |
| - if (!scsi_status_is_good(res)) { |
| + if (res < 0) { |
| sd_first_printk(KERN_WARNING, sdkp, |
| "Test WP failed, assume Write Enabled\n"); |
| } else { |
| @@ -2755,7 +2755,7 @@ sd_read_cache_type(struct scsi_disk *sdkp, unsigned char *buffer) |
| res = sd_do_mode_sense(sdkp, dbd, modepage, buffer, first_len, |
| &data, &sshdr); |
| |
| - if (!scsi_status_is_good(res)) |
| + if (res < 0) |
| goto bad_sense; |
| |
| if (!data.header_length) { |
| @@ -2787,7 +2787,7 @@ sd_read_cache_type(struct scsi_disk *sdkp, unsigned char *buffer) |
| res = sd_do_mode_sense(sdkp, dbd, modepage, buffer, len, |
| &data, &sshdr); |
| |
| - if (scsi_status_is_good(res)) { |
| + if (!res) { |
| int offset = data.header_length + data.block_descriptor_length; |
| |
| while (offset < len) { |
| @@ -2905,7 +2905,7 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer) |
| res = scsi_mode_sense(sdp, 1, 0x0a, buffer, 36, SD_TIMEOUT, |
| sdkp->max_retries, &data, &sshdr); |
| |
| - if (!scsi_status_is_good(res) || !data.header_length || |
| + if (res < 0 || !data.header_length || |
| data.length < 6) { |
| sd_first_printk(KERN_WARNING, sdkp, |
| "getting Control mode page failed, assume no ATO\n"); |
| diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c |
| index 7815ed642d43..1a94c7b1de2d 100644 |
| --- a/drivers/scsi/sr.c |
| +++ b/drivers/scsi/sr.c |
| @@ -913,7 +913,7 @@ static void get_capabilities(struct scsi_cd *cd) |
| rc = scsi_mode_sense(cd->device, 0, 0x2a, buffer, ms_len, |
| SR_TIMEOUT, 3, &data, NULL); |
| |
| - if (!scsi_status_is_good(rc) || data.length > ms_len || |
| + if (rc < 0 || data.length > ms_len || |
| data.header_length + data.block_descriptor_length > data.length) { |
| /* failed, drive doesn't have capabilities mode page */ |
| cd->cdi.speed = 1; |
| -- |
| 2.30.2 |
| |