Skip to content

Commit

Permalink
[SCSI] eliminate potential kmalloc failure in scsi_get_vpd_page()
Browse files Browse the repository at this point in the history
The best way to fix this is to eliminate the intenal kmalloc() and
make the caller allocate the required amount of storage.

Signed-off-by: James Bottomley <James.Bottomley@suse.de>
  • Loading branch information
James Bottomley authored and James Bottomley committed Jan 18, 2010
1 parent 534ef05 commit e3deec0
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 43 deletions.
40 changes: 12 additions & 28 deletions drivers/scsi/scsi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1026,55 +1026,39 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char *buffer,
* responsible for calling kfree() on this pointer when it is no longer
* needed. If we cannot retrieve the VPD page this routine returns %NULL.
*/
unsigned char *scsi_get_vpd_page(struct scsi_device *sdev, u8 page)
int scsi_get_vpd_page(struct scsi_device *sdev, u8 page, unsigned char *buf,
int buf_len)
{
int i, result;
unsigned int len;
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, init_vpd_len);
result = scsi_vpd_inquiry(sdev, buf, 0, buf_len);
if (result)
goto fail;

/* If the user actually wanted this page, we can skip the rest */
if (page == 0)
return buf;
return -EINVAL;

for (i = 0; i < buf[3]; i++)
for (i = 0; i < min((int)buf[3], buf_len - 4); i++)
if (buf[i + 4] == page)
goto found;

if (i < buf[3] && i > buf_len)
/* ran off the end of the buffer, give us benefit of doubt */
goto found;
/* The device claims it doesn't support the requested page */
goto fail;

found:
result = scsi_vpd_inquiry(sdev, buf, page, 255);
result = scsi_vpd_inquiry(sdev, buf, page, buf_len);
if (result)
goto fail;

/*
* Some pages are longer than 255 bytes. The actual length of
* the page is returned in the header.
*/
len = ((buf[2] << 8) | buf[3]) + 4;
if (len <= init_vpd_len)
return buf;

kfree(buf);
buf = kmalloc(len, GFP_KERNEL);
result = scsi_vpd_inquiry(sdev, buf, page, len);
if (result)
goto fail;

return buf;
return 0;

fail:
kfree(buf);
return NULL;
return -EINVAL;
}
EXPORT_SYMBOL_GPL(scsi_get_vpd_page);

Expand Down
26 changes: 15 additions & 11 deletions drivers/scsi/sd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1946,13 +1946,13 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
{
struct request_queue *q = sdkp->disk->queue;
unsigned int sector_sz = sdkp->device->sector_size;
char *buffer;
const int vpd_len = 32;
unsigned char *buffer = kmalloc(vpd_len, GFP_KERNEL);

/* Block Limits VPD */
buffer = scsi_get_vpd_page(sdkp->device, 0xb0);

if (buffer == NULL)
return;
if (!buffer ||
/* Block Limits VPD */
scsi_get_vpd_page(sdkp->device, 0xb0, buffer, vpd_len))
goto out;

blk_queue_io_min(sdkp->disk->queue,
get_unaligned_be16(&buffer[6]) * sector_sz);
Expand Down Expand Up @@ -1984,6 +1984,7 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
get_unaligned_be32(&buffer[32]) & ~(1 << 31);
}

out:
kfree(buffer);
}

Expand All @@ -1993,20 +1994,23 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
*/
static void sd_read_block_characteristics(struct scsi_disk *sdkp)
{
char *buffer;
unsigned char *buffer;
u16 rot;
const int vpd_len = 32;

/* Block Device Characteristics VPD */
buffer = scsi_get_vpd_page(sdkp->device, 0xb1);
buffer = kmalloc(vpd_len, GFP_KERNEL);

if (buffer == NULL)
return;
if (!buffer ||
/* Block Device Characteristics VPD */
scsi_get_vpd_page(sdkp->device, 0xb1, buffer, vpd_len))
goto out;

rot = get_unaligned_be16(&buffer[4]);

if (rot == 1)
queue_flag_set_unlocked(QUEUE_FLAG_NONROT, sdkp->disk->queue);

out:
kfree(buffer);
}

Expand Down
10 changes: 7 additions & 3 deletions drivers/scsi/ses.c
Original file line number Diff line number Diff line change
Expand Up @@ -448,13 +448,17 @@ static void ses_match_to_enclosure(struct enclosure_device *edev,
.addr = 0,
};

buf = scsi_get_vpd_page(sdev, 0x83);
if (!buf)
return;
buf = kmalloc(INIT_ALLOC_SIZE, GFP_KERNEL);
if (!buf || scsi_get_vpd_page(sdev, 0x83, buf, INIT_ALLOC_SIZE))
goto free;

ses_enclosure_data_process(edev, to_scsi_device(edev->edev.parent), 0);

vpd_len = ((buf[2] << 8) | buf[3]) + 4;
kfree(buf);
buf = kmalloc(vpd_len, GFP_KERNEL);
if (!buf ||scsi_get_vpd_page(sdev, 0x83, buf, vpd_len))
goto free;

desc = buf + 4;
while (desc < buf + vpd_len) {
Expand Down
3 changes: 2 additions & 1 deletion include/scsi/scsi_device.h
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@ extern int scsi_mode_select(struct scsi_device *sdev, int pf, int sp,
struct scsi_sense_hdr *);
extern int scsi_test_unit_ready(struct scsi_device *sdev, int timeout,
int retries, struct scsi_sense_hdr *sshdr);
extern unsigned char *scsi_get_vpd_page(struct scsi_device *, u8 page);
extern int scsi_get_vpd_page(struct scsi_device *, u8 page, unsigned char *buf,
int buf_len);
extern int scsi_device_set_state(struct scsi_device *sdev,
enum scsi_device_state state);
extern struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type,
Expand Down

0 comments on commit e3deec0

Please sign in to comment.