On Wed, Mar 12, 2014 at 2:43 PM, Douglas Gilbert <dgilbert@xxxxxxxxxxxx> wrote: > On 14-03-12 10:14 PM, Muthukumar R wrote: >> >> On Mon, Mar 10, 2014 at 7:28 AM, Hannes Reinecke <hare@xxxxxxx> wrote: >>> >>> We should be returning the number of bytes of the >>> requested VPD page in scsi_vpd_inquiry. >>> This makes it easier for the caller to verify the >>> required space. >>> >>> Signed-off-by: Hannes Reinecke <hare@xxxxxxx> >>> --- >>> drivers/scsi/scsi.c | 17 ++++++++++------- >>> 1 file changed, 10 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c >>> index d8afec8..ecaeff1 100644 >>> --- a/drivers/scsi/scsi.c >>> +++ b/drivers/scsi/scsi.c >>> @@ -954,7 +954,7 @@ EXPORT_SYMBOL(scsi_track_queue_full); >>> * This is an internal helper function. You probably want to use >>> * scsi_get_vpd_page instead. >>> * >>> - * Returns 0 on success or a negative error number. >>> + * Returns size of the vpg page on success or a negative error number. >>> */ >> >> Typo: ^^^ >> >> Should be: vpd page >> >> >>> static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char >>> *buffer, >>> u8 page, >>> unsigned len) >>> @@ -962,6 +962,9 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, >>> unsigned char *buffer, >>> int result; >>> unsigned char cmd[16]; >>> >>> + if (len < 4) >>> + return -EINVAL; >>> + >>> cmd[0] = INQUIRY; >>> cmd[1] = 1; /* EVPD */ >>> cmd[2] = page; >>> @@ -982,7 +985,7 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, >>> unsigned char *buffer, >>> if (buffer[1] != page) >>> return -EIO; >>> >>> - return 0; >>> + return get_unaligned_be16(&buffer[2]) + 4; >>> } >>> >>> /** >>> @@ -1009,18 +1012,18 @@ int scsi_get_vpd_page(struct scsi_device *sdev, >>> u8 page, unsigned char *buf, >>> >>> /* Ask for all the pages supported by this device */ >>> result = scsi_vpd_inquiry(sdev, buf, 0, buf_len); >>> - if (result) >>> + if (result < 4) >>> goto fail; >> >> >> >> You mean: >> >> if (result < 0) >> goto fail; > > > I think that he means: > > if (result < 4) > goto fail; > > The first 4 bytes of a VPD page are a header with buf[2] and > buf[3] being the length of the following payload. Without > both of them "fail" is a quite accurate description. > > That said, it is hard to get a number between 0 and 3 with: > > return get_unaligned_be16(&buffer[2]) + 4; > but if you wanted to take resid into account it is possible. > > IMO this snippet is fine. > I see.. thanks. Comment is slightly misleading but I guess its OK in this case. > > Doug Gilbert > > > > -- 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
