Re: Bugs in scsi_vpd_inquiry()

[Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>]

On Tue, 11 Aug 2009, James Bottomley wrote:

> Sort of, but it's not really doing it properly.  Lets do it like this.
> This should also fix the > 255 length problem older devices might have.

Do you mind including also the residue check?

Alan Stern



Index: usb-2.6/drivers/scsi/scsi.c
===================================================================
--- usb-2.6.orig/drivers/scsi/scsi.c
+++ usb-2.6/drivers/scsi/scsi.c
@@ -980,7 +980,8 @@ static int scsi_vpd_inquiry(struct scsi_
 							u8 page, unsigned len)
 {
 	int result;
-	unsigned char cmd[16];
+	int resid;
+	unsigned char cmd[6];
 
 	cmd[0] = INQUIRY;
 	cmd[1] = 1;		/* EVPD */
@@ -994,12 +995,12 @@ static int scsi_vpd_inquiry(struct scsi_
 	 * all the existing users tried this hard.
 	 */
 	result = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buffer,
-				  len + 4, NULL, 30 * HZ, 3, NULL);
+				  len, NULL, 30 * HZ, 3, &resid);
 	if (result)
 		return result;
 
-	/* Sanity check that we got the page back that we asked for */
-	if (buffer[1] != page)
+	/* Sanity check that we got the header and the page we asked for */
+	if (resid > len - 4 || buffer[1] != page)
 		return -EIO;
 
 	return 0;
@@ -1021,13 +1022,14 @@ unsigned char *scsi_get_vpd_page(struct 
 {
 	int i, result;
 	unsigned int len;
-	unsigned char *buf = kmalloc(259, GFP_KERNEL);
+	const unsigned int init_vpd_len = 255;
+	unsigned char *buf = kmalloc(init_vpd_len, GFP_KERNEL);
 
 	if (!buf)
 		return NULL;
 
 	/* Ask for all the pages supported by this device */
-	result = scsi_vpd_inquiry(sdev, buf, 0, 255);
+	result = scsi_vpd_inquiry(sdev, buf, 0, init_vpd_len);
 	if (result)
 		goto fail;
 
@@ -1042,7 +1044,7 @@ unsigned char *scsi_get_vpd_page(struct 
 	goto fail;
 
  found:
-	result = scsi_vpd_inquiry(sdev, buf, page, 255);
+	result = scsi_vpd_inquiry(sdev, buf, page, init_vpd_len);
 	if (result)
 		goto fail;
 
@@ -1050,12 +1052,12 @@ unsigned char *scsi_get_vpd_page(struct 
 	 * Some pages are longer than 255 bytes.  The actual length of
 	 * the page is returned in the header.
 	 */
-	len = (buf[2] << 8) | buf[3];
-	if (len <= 255)
+	len = ((buf[2] << 8) | buf[3]) + 4;
+	if (len <= init_vpd_len)
 		return buf;
 
 	kfree(buf);
-	buf = kmalloc(len + 4, GFP_KERNEL);
+	buf = kmalloc(len, GFP_KERNEL);
 	result = scsi_vpd_inquiry(sdev, buf, page, len);
 	if (result)
 		goto fail;

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html