scsi: sd: fix uninit access of sshdr
diff mbox series

Message ID 1571392910-137272-1-git-send-email-zhengbin13@huawei.com
State Deferred
Headers show
Series
  • scsi: sd: fix uninit access of sshdr
Related show

Commit Message

zhengbin Oct. 18, 2019, 10:01 a.m. UTC
Like sd_pr_command, before use sshdr, we need to check the result of
scsi_execute and whether sshdr is valid.

Signed-off-by: zhengbin <zhengbin13@huawei.com>
---
 drivers/scsi/sd.c | 57 ++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 22 deletions(-)

--
2.7.4

Comments

Bart Van Assche Oct. 21, 2019, 2:12 a.m. UTC | #1
On 2019-10-18 03:01, zhengbin wrote:
> @@ -1648,16 +1651,20 @@ 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 (driver_byte(res) == DRIVER_SENSE)
> +		/* we need to evaluate the error return  */
> +		if (driver_byte(res) == DRIVER_SENSE &&
> +		    scsi_sense_valid(sshdr)) {
>  			sd_print_sense_hdr(sdkp, sshdr);
> 

Does Hannes' comment about DRIVER_SENSE also apply to this patch?

Thanks,

Bart.

Patch
diff mbox series

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 32d9517..2041755 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -166,7 +166,7 @@  cache_type_store(struct device *dev, struct device_attribute *attr,
 	struct scsi_mode_data data;
 	struct scsi_sense_hdr sshdr;
 	static const char temp[] = "temporary ";
-	int len;
+	int len, the_result;

 	if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
 		/* no cache control on RBC devices; theoretically they
@@ -213,9 +213,12 @@  cache_type_store(struct device *dev, struct device_attribute *attr,
 	 */
 	data.device_specific = 0;

-	if (scsi_mode_select(sdp, 1, sp, 8, buffer_data, len, SD_TIMEOUT,
-			     SD_MAX_RETRIES, &data, &sshdr)) {
-		if (scsi_sense_valid(&sshdr))
+	the_result = scsi_mode_select(sdp, 1, sp, 8, buffer_data, len,
+				      SD_TIMEOUT, SD_MAX_RETRIES, &data,
+				      &sshdr);
+	if (the_result) {
+		if (driver_byte(the_result) == DRIVER_SENSE &&
+		    scsi_sense_valid(&sshdr))
 			sd_print_sense_hdr(sdkp, &sshdr);
 		return -EINVAL;
 	}
@@ -1648,16 +1651,20 @@  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 (driver_byte(res) == DRIVER_SENSE)
+		/* we need to evaluate the error return  */
+		if (driver_byte(res) == DRIVER_SENSE &&
+		    scsi_sense_valid(sshdr)) {
 			sd_print_sense_hdr(sdkp, sshdr);

-		/* we need to evaluate the error return  */
-		if (scsi_sense_valid(sshdr) &&
-			(sshdr->asc == 0x3a ||	/* medium not present */
-			 sshdr->asc == 0x20 ||	/* invalid command */
-			 (sshdr->asc == 0x74 && sshdr->ascq == 0x71)))	/* drive is password locked */
+			/* medium not present */
+			if (sshdr->asc == 0x3a ||
+			    /* invalid command */
+			    sshdr->asc == 0x20 ||
+			    /* drive is password locked */
+			    (sshdr->asc == 0x74 && sshdr->ascq == 0x71))
 				/* this is no error here */
 				return 0;
+		}

 		switch (host_byte(res)) {
 		/* ignore errors due to racing a disconnection */
@@ -2100,10 +2107,11 @@  sd_spinup_disk(struct scsi_disk *sdkp)
 			 * doesn't have any media in it, don't bother
 			 * with any more polling.
 			 */
-			if (media_not_present(sdkp, &sshdr))
+			if (driver_byte(the_result) == DRIVER_SENSE &&
+			    media_not_present(sdkp, &sshdr))
 				return;

-			if (the_result)
+			if (driver_byte(the_result) == DRIVER_SENSE)
 				sense_valid = scsi_sense_valid(&sshdr);
 			retries++;
 		} while (retries < 3 &&
@@ -2177,7 +2185,8 @@  sd_spinup_disk(struct scsi_disk *sdkp)
 			 * probably pointless to loop */
 			if(!spintime) {
 				sd_printk(KERN_NOTICE, sdkp, "Unit Not Ready\n");
-				sd_print_sense_hdr(sdkp, &sshdr);
+				if (sense_valid)
+					sd_print_sense_hdr(sdkp, &sshdr);
 			}
 			break;
 		}
@@ -2237,7 +2246,7 @@  static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device *sdp,
 			struct scsi_sense_hdr *sshdr, int sense_valid,
 			int the_result)
 {
-	if (driver_byte(the_result) == DRIVER_SENSE)
+	if (driver_byte(the_result) == DRIVER_SENSE && sense_valid)
 		sd_print_sense_hdr(sdkp, sshdr);
 	else
 		sd_printk(KERN_NOTICE, sdkp, "Sense not available.\n");
@@ -2291,10 +2300,11 @@  static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
 					buffer, RC16_LEN, &sshdr,
 					SD_TIMEOUT, SD_MAX_RETRIES, NULL);

-		if (media_not_present(sdkp, &sshdr))
+		if (driver_byte(the_result) == DRIVER_SENSE &&
+		    media_not_present(sdkp, &sshdr))
 			return -ENODEV;

-		if (the_result) {
+		if (driver_byte(the_result) == DRIVER_SENSE) {
 			sense_valid = scsi_sense_valid(&sshdr);
 			if (sense_valid &&
 			    sshdr.sense_key == ILLEGAL_REQUEST &&
@@ -2376,10 +2386,11 @@  static int read_capacity_10(struct scsi_disk *sdkp, struct scsi_device *sdp,
 					buffer, 8, &sshdr,
 					SD_TIMEOUT, SD_MAX_RETRIES, NULL);

-		if (media_not_present(sdkp, &sshdr))
+		if (driver_byte(the_result) == DRIVER_SENSE
+		    && media_not_present(sdkp, &sshdr))
 			return -ENODEV;

-		if (the_result) {
+		if (driver_byte(the_result) == DRIVER_SENSE) {
 			sense_valid = scsi_sense_valid(&sshdr);
 			if (sense_valid &&
 			    sshdr.sense_key == UNIT_ATTENTION &&
@@ -3504,12 +3515,14 @@  static int sd_start_stop_device(struct scsi_disk *sdkp, int start)
 			SD_TIMEOUT, SD_MAX_RETRIES, 0, RQF_PM, NULL);
 	if (res) {
 		sd_print_result(sdkp, "Start/Stop Unit failed", res);
-		if (driver_byte(res) == DRIVER_SENSE)
+		if (driver_byte(res) == DRIVER_SENSE &&
+		    scsi_sense_valid(&sshdr)) {
 			sd_print_sense_hdr(sdkp, &sshdr);
-		if (scsi_sense_valid(&sshdr) &&
+
 			/* 0x3a is medium not present */
-			sshdr.asc == 0x3a)
-			res = 0;
+			if (sshdr.asc == 0x3a)
+				res = 0;
+		}
 	}

 	/* SCSI error codes must not go to the generic layer */