scsi: core: Stop using DRIVER_ERROR

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.

[mkp: resolve conflict and return bool]

Link: https://lore.kernel.org/r/20210427083046.31620-7-hare@suse.de
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index fd8b6febbf70c440e21850220c18cd7d2bf83ea4..513826eea682f1ed15e72e854335602db5401f96 100644 (file)
--- 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 9b89c26ccfdb6bfc24ae3b3cc56b7482ac7f0bb5..d5990f6fb313833e4a409b9c2cb5f754c0489377 100644 (file)
--- 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 ee11ec340654a7b0ac2b804989063c8bee1ff153..caa7c5fd233d25c0354671d75dfed33b98bae0f3 100644 (file)
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -369,7 +369,7 @@ retry:
                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 e9e2f0e15ac85ece57414fa488b89434fb258120..99dc6ec0b6e54ce6fe9505c39a2f537daac911c1 100644 (file)
--- 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 14872c9dc78cc6738a29043c8781de2952d23a75..d34e1b41dc719457ff0ec79e451f477112cbb37d 100644 (file)
--- 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 e763d368b892a1d5c3235cf15cf4d3f4bacfa20d..2cf36ccc6d4047437bb5b74492a676d4ba97d2b3 100644 (file)
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -211,20 +211,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 12f54571b83e4826cbcb511f0d5103dc3e3b1ad9..a0edcff7db1f49f9630935094a281905f23f4613 100644 (file)
--- 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 c37dd15d16d24f20b6065e6d7141da6c955fd58c..a9bb7ae2fafd697e89de5c9ca5e9062ee3395a90 100644 (file)
--- 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 2ef2954375f4c82baba8121a4799b8e8ad80df00..5733fbee2bae1a15c81d809b899f7e3d9ace51f1 100644 (file)
--- 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 e45d8d94574ca7c78b9d51da4600d589d3d6a082..d4a79fdcfffefae3da88b2310fbc49d865a14d10 100644 (file)
--- 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 15c305283b6cc1e1d09c0efbcdecdb4f8c9e3adf..0d6a716d281eefa2afb24ff59ba0b7096ef5d0cc 100644 (file)
--- a/drivers/scsi/sr_ioctl.c
+++ b/drivers/scsi/sr_ioctl.c
@@ -201,6 +201,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 af527e77fe66aa1b00afece5003f168843f6a714..9523e268c0e8d99096fb8dd428e31da6237281a9 100644 (file)
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8604,7 +8604,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 b9c86a7e3b97d6e5adea59063d89e2a2fe2fc014..1678b6f14af96c6b4fe226d69cb10c6898515951 100644 (file)
--- 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 7f392405991bb081eaf560e5837f16570dfcdf7c..6dc2d1b3735e177a3b5544faed60d8110e07eb29 100644 (file)
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -259,10 +259,13 @@ enum scsi_disposition {
  * This returns true for known good conditions that may be treated as
  * command completed normally
  */
-static inline int scsi_status_is_good(int status)
+static inline bool scsi_status_is_good(int status)
 {
+       if (status < 0)
+               return false;
+
        if (host_byte(status) == DID_NO_CONNECT)
-               return 0;
+               return false;
 
        /*
         * FIXME: bit0 is listed as reserved in SCSI-2, but is