From: Hannes Reinecke <hare@suse.de>
To: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>,
James Bottomley <james.bottomley@hansenpartnership.com>,
Bart van Assche <bvanassche@acm.org>,
linux-scsi@vger.kernel.org, Hannes Reinecke <hare@suse.de>
Subject: [PATCH 06/40] scsi: stop using DRIVER_ERROR
Date: Tue, 27 Apr 2021 10:30:12 +0200 [thread overview]
Message-ID: <20210427083046.31620-7-hare@suse.de> (raw)
In-Reply-To: <20210427083046.31620-1-hare@suse.de>
Return the actual error code in __scsi_execute() (which, according
to the documentation, should have happened anyway).
And audit all callers to cope with negative return values from
__scsi_execute() and friends.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
drivers/ata/libata-scsi.c | 8 ++++++++
drivers/scsi/ch.c | 3 ++-
drivers/scsi/cxlflash/superpipe.c | 2 +-
drivers/scsi/scsi.c | 2 ++
drivers/scsi/scsi_ioctl.c | 4 +++-
drivers/scsi/scsi_lib.c | 15 +++++++++------
drivers/scsi/scsi_scan.c | 4 ++--
drivers/scsi/scsi_transport_spi.c | 2 +-
drivers/scsi/sd.c | 15 +++++++++------
drivers/scsi/sd_zbc.c | 2 +-
drivers/scsi/sr_ioctl.c | 4 ++++
drivers/scsi/ufs/ufshcd.c | 2 +-
drivers/scsi/virtio_scsi.c | 2 +-
include/scsi/scsi.h | 3 +++
14 files changed, 47 insertions(+), 21 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 48b8934970f3..c5129b9e3afd 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -409,6 +409,10 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg)
cmd_result = scsi_execute(scsidev, scsi_cmd, data_dir, argbuf, argsize,
sensebuf, &sshdr, (10*HZ), 5, 0, 0, NULL);
+ if (cmd_result < 0) {
+ rc = cmd_result;
+ goto error;
+ }
if (driver_byte(cmd_result) == DRIVER_SENSE) {/* sense data available */
u8 *desc = sensebuf + 8;
cmd_result &= ~(0xFF<<24); /* DRIVER_SENSE is not an error */
@@ -490,6 +494,10 @@ int ata_task_ioctl(struct scsi_device *scsidev, void __user *arg)
cmd_result = scsi_execute(scsidev, scsi_cmd, DMA_NONE, NULL, 0,
sensebuf, &sshdr, (10*HZ), 5, 0, 0, NULL);
+ if (cmd_result < 0) {
+ rc = cmd_result;
+ goto error;
+ }
if (driver_byte(cmd_result) == DRIVER_SENSE) {/* sense data available */
u8 *desc = sensebuf + 8;
cmd_result &= ~(0xFF<<24); /* DRIVER_SENSE is not an error */
diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index cb74ab1ae5a4..0e7d1214c3d8 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -198,7 +198,8 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd, int cmd_len,
result = scsi_execute_req(ch->device, cmd, direction, buffer,
buflength, &sshdr, timeout * HZ,
MAX_RETRIES, NULL);
-
+ if (result < 0)
+ return result;
if (driver_byte(result) == DRIVER_SENSE) {
if (debug)
scsi_print_sense_hdr(ch->device, ch->name, &sshdr);
diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c
index ee11ec340654..caa7c5fd233d 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -369,7 +369,7 @@ static int read_cap16(struct scsi_device *sdev, struct llun_info *lli)
goto out;
}
- if (driver_byte(result) == DRIVER_SENSE) {
+ if (result > 0 && driver_byte(result) == DRIVER_SENSE) {
result &= ~(0xFF<<24); /* DRIVER_SENSE is not an error */
if (result & SAM_STAT_CHECK_CONDITION) {
switch (sshdr.sense_key) {
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index e9e2f0e15ac8..99dc6ec0b6e5 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -508,6 +508,8 @@ int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer,
result = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buffer, len,
&sshdr, 30 * HZ, 3, NULL);
+ if (result < 0)
+ return result;
if (result && scsi_sense_valid(&sshdr) &&
sshdr.sense_key == ILLEGAL_REQUEST &&
(sshdr.asc == 0x20 || sshdr.asc == 0x24) && sshdr.ascq == 0x00)
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index 14872c9dc78c..d34e1b41dc71 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -101,6 +101,8 @@ static int ioctl_internal_command(struct scsi_device *sdev, char *cmd,
SCSI_LOG_IOCTL(2, sdev_printk(KERN_INFO, sdev,
"Ioctl returned 0x%x\n", result));
+ if (result < 0)
+ goto out;
if (driver_byte(result) == DRIVER_SENSE &&
scsi_sense_valid(&sshdr)) {
switch (sshdr.sense_key) {
@@ -133,7 +135,7 @@ static int ioctl_internal_command(struct scsi_device *sdev, char *cmd,
break;
}
}
-
+out:
SCSI_LOG_IOCTL(2, sdev_printk(KERN_INFO, sdev,
"IOCTL Releasing command\n"));
return result;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ba54b1ba5edb..cd316e935281 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -245,20 +245,23 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
{
struct request *req;
struct scsi_request *rq;
- int ret = DRIVER_ERROR << 24;
+ int ret;
req = blk_get_request(sdev->request_queue,
data_direction == DMA_TO_DEVICE ?
REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN,
rq_flags & RQF_PM ? BLK_MQ_REQ_PM : 0);
if (IS_ERR(req))
- return ret;
- rq = scsi_req(req);
+ return PTR_ERR(req);
- if (bufflen && blk_rq_map_kern(sdev->request_queue, req,
- buffer, bufflen, GFP_NOIO))
- goto out;
+ rq = scsi_req(req);
+ if (bufflen) {
+ ret = blk_rq_map_kern(sdev->request_queue, req,
+ buffer, bufflen, GFP_NOIO);
+ if (ret)
+ goto out;
+ }
rq->cmd_len = COMMAND_SIZE(cmd[0]);
memcpy(rq->cmd, cmd, rq->cmd_len);
rq->retries = retries;
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 9f1b7f3c650a..493f17bf26f2 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -616,7 +616,7 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
"scsi scan: INQUIRY %s with code 0x%x\n",
result ? "failed" : "successful", result));
- if (result) {
+ if (result > 0) {
/*
* not-ready to ready transition [asc/ascq=0x28/0x0]
* or power-on, reset [asc/ascq=0x29/0x0], continue.
@@ -631,7 +631,7 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
(sshdr.ascq == 0))
continue;
}
- } else {
+ } else if (result == 0) {
/*
* if nothing was transferred, we try
* again. It's a workaround for some USB
diff --git a/drivers/scsi/scsi_transport_spi.c b/drivers/scsi/scsi_transport_spi.c
index c37dd15d16d2..a9bb7ae2fafd 100644
--- a/drivers/scsi/scsi_transport_spi.c
+++ b/drivers/scsi/scsi_transport_spi.c
@@ -127,7 +127,7 @@ static int spi_execute(struct scsi_device *sdev, const void *cmd,
REQ_FAILFAST_TRANSPORT |
REQ_FAILFAST_DRIVER,
RQF_PM, NULL);
- if (driver_byte(result) != DRIVER_SENSE ||
+ if (result < 0 || driver_byte(result) != DRIVER_SENSE ||
sshdr->sense_key != UNIT_ATTENTION)
break;
}
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 2ef2954375f4..5733fbee2bae 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1658,7 +1658,7 @@ static unsigned int sd_check_events(struct gendisk *disk, unsigned int clearing)
&sshdr);
/* failed to execute TUR, assume media not present */
- if (host_byte(retval)) {
+ if (retval < 0 || host_byte(retval)) {
set_media_not_present(sdkp);
goto out;
}
@@ -1719,6 +1719,9 @@ static int sd_sync_cache(struct scsi_disk *sdkp, struct scsi_sense_hdr *sshdr)
if (res) {
sd_print_result(sdkp, "Synchronize Cache(10) failed", res);
+ if (res < 0)
+ return res;
+
if (driver_byte(res) == DRIVER_SENSE)
sd_print_sense_hdr(sdkp, sshdr);
@@ -1825,7 +1828,7 @@ static int sd_pr_command(struct block_device *bdev, u8 sa,
result = scsi_execute_req(sdev, cmd, DMA_TO_DEVICE, &data, sizeof(data),
&sshdr, SD_TIMEOUT, sdkp->max_retries, NULL);
- if (driver_byte(result) == DRIVER_SENSE &&
+ if (result > 0 && driver_byte(result) == DRIVER_SENSE &&
scsi_sense_valid(&sshdr)) {
sdev_printk(KERN_INFO, sdev, "PR command failed: %d\n", result);
scsi_print_sense_hdr(sdev, NULL, &sshdr);
@@ -2177,7 +2180,7 @@ sd_spinup_disk(struct scsi_disk *sdkp)
((driver_byte(the_result) == DRIVER_SENSE) &&
sense_valid && sshdr.sense_key == UNIT_ATTENTION)));
- if (driver_byte(the_result) != DRIVER_SENSE) {
+ if (the_result < 0 || driver_byte(the_result) != DRIVER_SENSE) {
/* no sense, TUR either succeeded or failed
* with a status error */
if(!spintime && !scsi_status_is_good(the_result)) {
@@ -2362,7 +2365,7 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
if (media_not_present(sdkp, &sshdr))
return -ENODEV;
- if (the_result) {
+ if (the_result > 0) {
sense_valid = scsi_sense_valid(&sshdr);
if (sense_valid &&
sshdr.sense_key == ILLEGAL_REQUEST &&
@@ -2447,7 +2450,7 @@ static int read_capacity_10(struct scsi_disk *sdkp, struct scsi_device *sdp,
if (media_not_present(sdkp, &sshdr))
return -ENODEV;
- if (the_result) {
+ if (the_result > 0) {
sense_valid = scsi_sense_valid(&sshdr);
if (sense_valid &&
sshdr.sense_key == UNIT_ATTENTION &&
@@ -3591,7 +3594,7 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start)
SD_TIMEOUT, sdkp->max_retries, 0, RQF_PM, NULL);
if (res) {
sd_print_result(sdkp, "Start/Stop Unit failed", res);
- if (driver_byte(res) == DRIVER_SENSE)
+ if (res > 0 && driver_byte(res) == DRIVER_SENSE)
sd_print_sense_hdr(sdkp, &sshdr);
if (scsi_sense_valid(&sshdr) &&
/* 0x3a is medium not present */
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index e45d8d94574c..d4a79fdcfffe 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -116,7 +116,7 @@ static int sd_zbc_do_report_zones(struct scsi_disk *sdkp, unsigned char *buf,
sd_printk(KERN_ERR, sdkp,
"REPORT ZONES start lba %llu failed\n", lba);
sd_print_result(sdkp, "REPORT ZONES", result);
- if (driver_byte(result) == DRIVER_SENSE &&
+ if (result > 0 && driver_byte(result) == DRIVER_SENSE &&
scsi_sense_valid(&sshdr))
sd_print_sense_hdr(sdkp, &sshdr);
return -EIO;
diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
index 5703f8400b73..a78f2138d784 100644
--- a/drivers/scsi/sr_ioctl.c
+++ b/drivers/scsi/sr_ioctl.c
@@ -205,6 +205,10 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)
cgc->timeout, IOCTL_RETRIES, 0, 0, NULL);
/* Minimal error checking. Ignore cases we know about, and report the rest. */
+ if (result < 0) {
+ err = result;
+ goto out;
+ }
if (driver_byte(result) != 0) {
switch (sshdr->sense_key) {
case UNIT_ATTENTION:
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 0625da7a42ee..f743434073ac 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8487,7 +8487,7 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
sdev_printk(KERN_WARNING, sdp,
"START_STOP failed for power mode: %d, result %x\n",
pwr_mode, ret);
- if (driver_byte(ret) == DRIVER_SENSE)
+ if (ret > 0 && driver_byte(ret) == DRIVER_SENSE)
scsi_print_sense_hdr(sdp, NULL, &sshdr);
}
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index b9c86a7e3b97..1678b6f14af9 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -355,7 +355,7 @@ static void virtscsi_rescan_hotunplug(struct virtio_scsi *vscsi)
if (result == 0 && inq_result[0] >> 5) {
/* PQ indicates the LUN is not attached */
scsi_remove_device(sdev);
- } else if (host_byte(result) == DID_BAD_TARGET) {
+ } else if (result > 0 && host_byte(result) == DID_BAD_TARGET) {
/*
* If all LUNs of a virtio-scsi device are unplugged
* it will respond with BAD TARGET on any INQUIRY
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 246ced401683..d0a24e55ad63 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -40,6 +40,9 @@ enum scsi_timeouts {
*/
static inline int scsi_status_is_good(int status)
{
+ if (status < 0)
+ return false;
+
/*
* FIXME: bit0 is listed as reserved in SCSI-2, but is
* significant in SCSI-3. For now, we follow the SCSI-2
--
2.29.2
next prev parent reply other threads:[~2021-04-27 8:31 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-27 8:30 [PATCHv3 00/40] SCSI result cleanup, part 2 Hannes Reinecke
2021-04-27 8:30 ` [PATCH 01/40] st: return error code in st_scsi_execute() Hannes Reinecke
2021-04-27 12:11 ` "Kai Mäkisara (Kolumbus)"
2021-04-27 8:30 ` [PATCH 02/40] scsi_ioctl: return error code when blk_rq_map_kern() fails Hannes Reinecke
2021-04-27 8:30 ` [PATCH 03/40] scsi: Fixup calling convention for scsi_mode_sense() Hannes Reinecke
2021-04-27 8:30 ` [PATCH 04/40] scsi: reshuffle response handling in scsi_mode_sense() Hannes Reinecke
2021-04-27 8:30 ` [PATCH 05/40] scsi_dh_alua: check for negative result value Hannes Reinecke
2021-04-27 8:30 ` Hannes Reinecke [this message]
2021-04-27 8:30 ` [PATCH 07/40] scsi: introduce scsi_build_sense() Hannes Reinecke
2021-04-27 8:30 ` [PATCH 08/40] scsi: introduce scsi_status_is_check_condition() Hannes Reinecke
2021-04-27 8:30 ` [PATCH 09/40] scsi: Kill DRIVER_SENSE Hannes Reinecke
2021-04-27 8:30 ` [PATCH 10/40] scsi: do not use DRIVER_INVALID Hannes Reinecke
2021-04-27 8:30 ` [PATCH 11/40] scsi_error: use DID_TIME_OUT instead of DRIVER_TIMEOUT Hannes Reinecke
2021-04-27 8:30 ` [PATCH 12/40] xen-scsiback: use DID_ERROR instead of DRIVER_ERROR Hannes Reinecke
2021-04-27 8:30 ` [PATCH 13/40] xen-scsifront: compability status handling Hannes Reinecke
2021-04-27 8:30 ` [PATCH 14/40] scsi: Drop the now obsolete driver_byte definitions Hannes Reinecke
2021-04-27 8:30 ` [PATCH 15/40] NCR5380: Fold SCSI message ABORT onto DID_ABORT Hannes Reinecke
2021-04-27 8:30 ` [PATCH 16/40] scsi: add get_{status,host}_byte() accessor function Hannes Reinecke
2021-04-27 8:30 ` [PATCH 17/40] scsi: add scsi_msg_to_host_byte() Hannes Reinecke
2021-04-27 8:30 ` [PATCH 18/40] dc395: use standard macros to set SCSI result Hannes Reinecke
2021-04-27 8:30 ` [PATCH 19/40] dc395: translate message bytes Hannes Reinecke
2021-04-27 8:30 ` [PATCH 20/40] qlogicfas408: make ql_pcmd() a void function Hannes Reinecke
2021-04-27 8:30 ` [PATCH 21/40] qlogicfas408: whitespace cleanup Hannes Reinecke
2021-04-27 8:30 ` [PATCH 22/40] qlogicfas408: translate message to host byte status Hannes Reinecke
2021-04-27 8:30 ` [PATCH 23/40] nsp32: whitespace cleanup Hannes Reinecke
2021-04-27 8:30 ` [PATCH 24/40] nsp32: do not set message byte Hannes Reinecke
2021-04-27 8:30 ` [PATCH 25/40] wd33c93: translate message byte to host byte Hannes Reinecke
2021-04-27 8:30 ` [PATCH 26/40] mesh: translate message to host byte status Hannes Reinecke
2021-04-27 8:30 ` [PATCH 27/40] acornscsi: remove acornscsi_reportstatus() Hannes Reinecke
2021-04-27 8:30 ` [PATCH 28/40] acornscsi: translate message byte to host byte Hannes Reinecke
2021-04-27 8:30 ` [PATCH 29/40] aha152x: modify done() to use separate status bytes Hannes Reinecke
2021-04-27 8:30 ` [PATCH 30/40] aha152x: do not set message byte when calling scsi_done() Hannes Reinecke
2021-04-27 8:30 ` [PATCH 31/40] advansys: do not set message byte in SCSI status Hannes Reinecke
2021-04-27 8:30 ` [PATCH 32/40] fas216: translate message to host byte status Hannes Reinecke
2021-04-27 8:30 ` [PATCH 33/40] fas216: Use get_status_byte() to avoid using linux-specific status codes Hannes Reinecke
2021-04-27 8:30 ` [PATCH 34/40] FlashPoint: Use standard SCSI definitions Hannes Reinecke
2021-04-27 8:30 ` [PATCH 35/40] fdomain: drop last argument to fdomain_finish_cmd() Hannes Reinecke
2021-04-27 8:30 ` [PATCH 36/40] fdomain: translate message to host byte status Hannes Reinecke
2021-04-27 8:30 ` [PATCH 37/40] scsi: drop message byte helper Hannes Reinecke
2021-04-27 8:30 ` [PATCH 38/40] scsi: kill message byte Hannes Reinecke
2021-04-27 8:30 ` [PATCH 39/40] target: use standard SAM status types Hannes Reinecke
2021-04-27 8:30 ` [PATCH 40/40] scsi: drop obsolete linux-specific SCSI status codes Hannes Reinecke
2021-04-29 6:48 ` Christoph Hellwig
2021-04-29 7:29 ` Hannes Reinecke
2021-04-29 15:52 ` Douglas Gilbert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210427083046.31620-7-hare@suse.de \
--to=hare@suse.de \
--cc=bvanassche@acm.org \
--cc=hch@lst.de \
--cc=james.bottomley@hansenpartnership.com \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox