Re: Bugs in scsi_vpd_inquiry()

Alan Stern
2009-08-11T16:14:24Z

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;