Skip to contentSkip to conversation

usb: storage: debug: uninitialized var in usb_stor_show_sense()

PATCH In review Dan Carpenter 2 participants 5 messages Nov 29, 2014 – Dec 2, 2014

Subsystems (4)
SCSI USB Kernel janitors USB mass storage driver
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 usb: storage: debug: uninitialized var in usb_stor_show_sense()

The "fmt" variable might be used uninitialized, it should be set to NULL at the start.

Fixes d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
Signed-off-by Dan Carpenter <[email protected]>
drivers/usb/storage/debug.c +2 -1
index 57bf3ad..fd00509 100644
@@ -164,7 +164,8 @@ void usb_stor_show_sense(const struct us_data *us,
 			 unsigned char asc,
 			 unsigned char ascq)
 {
-	const char *what, *keystr, *fmt;
+	const char *fmt = NULL;
+	const char *what, *keystr;
 
 	keystr = scsi_sense_key_string(key);
 	what = scsi_extd_sense_format(asc, ascq, &fmt);
maintainer
#1 lore raw
Quoted 5 lines · Dan Carpenter

The "fmt" variable might be used uninitialized, it should be set to NULL at the start.

Fixes d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
Signed-off-by Dan Carpenter <[email protected]>

Hm, that' falls into the category of an interface that's hard to get right if every caller has to remember to set fmt to NULL.

Could you instead set *fmt to NULL in scsi_extd_sense_format()? That way the interface is much easier.

Thanks,

James

author
#2 lore raw
Quoted 4 lines · James Bottomley
Quoted 3 lines · Dan Carpenter

The "fmt" variable might be used uninitialized, it should be set to NULL at the start.

Quoted 9 lines
Quoted 2 lines
Fixes d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
Signed-off-by Dan Carpenter <[email protected]>

Hm, that' falls into the category of an interface that's hard to get right if every caller has to remember to set fmt to NULL.

Could you instead set *fmt to NULL in scsi_extd_sense_format()? That way the interface is much easier.

Sure. I will send that tomorrow.

regards,
dan carpenter

author
#3 lore raw

PATCH v2 1/1 scsi: set fmt to NULL scsi_extd_sense_format() by default

In reply to James Bottomley

One of the two callers passes an unintialized pointer as "fmt" and expects it to be set to NULL if there is no format string. Let's make this function work as expected.

Fixes d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
Signed-off-by Dan Carpenter <[email protected]>

---
v2: Change scsi_extd_sense_format() instead of the caller

drivers/scsi/constants.c +3 -0
index aee62f6..e2068a2 100644
@@ -1271,6 +1271,7 @@ scsi_extd_sense_format(unsigned char asc, unsigned char ascq, const char **fmt)
 	int i;
 	unsigned short code = ((asc << 8) | ascq);
 
+	*fmt = NULL;
 	for (i = 0; additional[i].text; i++)
 		if (additional[i].code12 = code)
 			return additional[i].text;

@@ -1282,6 +1283,8 @@ scsi_extd_sense_format(unsigned char asc, unsigned char ascq, const char **fmt)
 			return additional2[i].str;
 		}
 	}
+#else
+	*fmt = NULL;
 #endif
 	return NULL;
 }
maintainer Reviewed-by on v2 1/1
#4 lore raw

Re: [patch v2] scsi: set fmt to NULL scsi_extd_sense_format() by default

Quoted 8 lines · Dan Carpenter

One of the two callers passes an unintialized pointer as "fmt" and expects it to be set to NULL if there is no format string. Let's make this function work as expected.

Fixes d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
Signed-off-by Dan Carpenter <[email protected]>

---
v2: Change scsi_extd_sense_format() instead of the caller

Reviewed-by James Bottomley <[email protected]>

James