Skip to contentSkip to conversation

restrict buffer size to 254 in scsi_id

PATCH In review Hannes Reinecke 2 participants 2 messages Jul 19, 2004 – Jul 30, 2004

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 restrict buffer size to 254 in scsi_id

Hi all,

this patch restricts the max buffer size to 254 in scsi_id. According to our SCSI-god-in-residence this is needed as some Wide-SCSI device might behave nasty if asked for odd lengths. We had a rather lengthy bugzilla for it and even Patrick agreed that this is a good thing.
Patrick, is this correct? If so, could you also apply it for the scsi_id program?

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke			[email protected]
SuSE Linux AG				S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

We have to restrict the maximum length to less than 0xff. However, according to Kurt Garloff:

Also, you probably send a 6 byte command, max alloc length would be 255, and we'd better round that down to 254 then. (Uneven counts on wide SCSI busses are a bit nasty and tend to trigger bugs in the LLDs w.r.t. handling of the IGNORE_RESIDUAL msg.)

So we'll be using 254 throughout.

extras/scsi_id/scsi_serial.c +9 -5
Index: udev-021/extras/scsi_id/scsi_serial.c
===================================================================
@@ -303,7 +303,7 @@ static int scsi_inquiry(struct sysfs_dev
 	unsigned char *buffer;
 	int retry = 3; /* rather random */
 
-	if (buflen > 255) {
+	if (buflen > 254) {
 		log_message(LOG_WARNING, "buflen %d too long\n", buflen);
 		return -1;
 	}
@@ -356,6 +356,7 @@ resend:
 	return retval;
 }
 
+/* Get list of supported EVPD pages */
 static int do_scsi_page0_inquiry(struct sysfs_device *scsi_dev, int fd,
 				 char *buffer, int len)
 {
@@ -544,6 +561,7 @@ static int check_fill_0x83_id(struct sys
 	return 0;
 }
 
+/* Get device identification VPD page */
 static int do_scsi_page83_inquiry(struct sysfs_device *scsi_dev, int fd,
 				  char *serial, int len)
 {
@@ -552,7 +570,7 @@ static int do_scsi_page83_inquiry(struct
 	unsigned char page_83[256];
 
 	memset(page_83, 0, 256);
-	retval = scsi_inquiry(scsi_dev, fd, 1, 0x83, page_83, 255);
+	retval = scsi_inquiry(scsi_dev, fd, 1, 0x83, page_83, 254);
 	if (retval < 0)
 		return 1;
 
@@ -601,6 +619,7 @@ static int do_scsi_page83_inquiry(struct
 	return 1;
 }
 
+/* Get unit serial number VPD page */
 static int do_scsi_page80_inquiry(struct sysfs_device *scsi_dev, int fd,
 				  char *serial, int max_len)
 {
@@ -611,7 +630,7 @@ static int do_scsi_page80_inquiry(struct
 	unsigned char buf[256];
 
 	memset(buf, 0, 256);
-	retval = scsi_inquiry(scsi_dev, fd, 1, 0x80, buf, 255);
+	retval = scsi_inquiry(scsi_dev, fd, 1, 0x80, buf, 254);
 	if (retval < 0)
 		return retval;
 
@@ -649,7 +668,8 @@ int scsi_get_serial (struct sysfs_device
 	int ind;
 	int retval;
 
-	if (len > 255) {
+	if (len > 254) {
+		len = 254;
 	}
 	memset(serial, 0, len);
 	dprintf("opening %s\n", devname);
@@ -686,7 +706,7 @@ int scsi_get_serial (struct sysfs_device
 	 * Get page 0, the page of the pages. By default, try from best to
 	 * worst of supported pages: 0x83 then 0x80.
 	 */
-	if (do_scsi_page0_inquiry(scsi_dev, fd, page0, 255)) {
+	if (do_scsi_page0_inquiry(scsi_dev, fd, page0, 254)) {
 		/*
 		 * Don't try anything else. Black list if a specific page
 		 * should be used for this vendor+model, or maybe have an
#1 lore raw
Quoted 9 lines · Hannes Reinecke

Hi all,

this patch restricts the max buffer size to 254 in scsi_id. According to our SCSI-god-in-residence this is needed as some Wide-SCSI device might behave nasty if asked for odd lengths.
We had a rather lengthy bugzilla for it and even Patrick agreed that this is a good thing.
Patrick, is this correct? If so, could you also apply it for the scsi_id program?

Hannes -

Looks good, thanks.

I applied, only change was to remove what had been a dumb no-op if check in scsi_get_serial, since the passed in buffer size can never be too large, just too small.

There are some buffer sizes that should change to match the 254 (should have just been a define), I'll change those.

-- Patrick Mansfield

-------------------------------------------------------
This SF.Net email is sponsored by OSTG. Have you noticed the changes on Linux.com, ITManagersJournal and NewsForge in the past few weeks? Now, one more big change to announce. We are now OSTG- Open Source Technology Group. Come see the changes on the new OSTG site. www.ostg.com
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel