mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-09-06 10:57:46 +00:00
scsi: core: Query VPD size before getting full page
We currently default to 255 bytes when fetching VPD pages during discovery. However, we have had a few devices that are known to wedge if the requested buffer exceeds a certain size. See commitaf73623f5f
("[SCSI] sd: Reduce buffer size for vpd request") which works around one example of this problem in the SCSI disk driver. With commitd188b0675b
("scsi: core: Add sysfs attributes for VPD pages 0h and 89h") we now risk triggering the same issue in the generic midlayer code. The problem with the ATA VPD page in particular is that the SCSI portion of the page is trailed by 512 bytes of verbatim ATA Identify Device information. However, not all controllers actually provide the additional 512 bytes and will lock up if one asks for more than the 64 bytes containing the SCSI protocol fields. Instead of picking a new, somewhat arbitrary, number of bytes for the VPD buffer size, start fetching the 4-byte header for each page. The header contains the size of the page as far as the device is concerned. We can use the reported size to specify the correct allocation length when subsequently fetching the full page. The header validation is done by a new helper function scsi_get_vpd_size() and both scsi_get_vpd_page() and scsi_get_vpd_buf() now rely on this to query the page size. In addition, scsi_get_vpd_page() is simplified to mirror the logic in scsi_get_vpd_page(). This involves removing the Supported VPD Pages lookup prior to attempting to query a page. There does not appear any evidence, even in the oldest SCSI specs, that this step is required. We already rely on scsi_get_vpd_page() throughout the stack and this function never consulted the Supported VPD Pages. Since this has not caused any problems it should be safe to remove the precondition from scsi_get_vpd_page(). Instrumented runs also revealed that the Supported VPD Pages lookup had little effect since the device page index often was larger than the supplied buffer size. As a result, inquiries frequently bypassed the index check and went through the "If we ran off the end of the buffer, give us the benefit of the doubt" code path which assumed the page was present despite not being listed. The revised code takes both the page size reported by the device as well as the size of the buffer provided by the scsi_get_vpd_page() caller into account. Link: https://lore.kernel.org/r/20220302053559.32147-3-martin.petersen@oracle.com Fixes:d188b0675b
("scsi: core: Add sysfs attributes for VPD pages 0h and 89h") Reported-by: Maciej W. Rozycki <macro@orcam.me.uk> Tested-by: Maciej W. Rozycki <macro@orcam.me.uk> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
This commit is contained in:
parent
dc1178767c
commit
c92a6b5d63
2 changed files with 62 additions and 32 deletions
|
@ -321,6 +321,31 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char *buffer,
|
|||
return get_unaligned_be16(&buffer[2]) + 4;
|
||||
}
|
||||
|
||||
static int scsi_get_vpd_size(struct scsi_device *sdev, u8 page)
|
||||
{
|
||||
unsigned char vpd_header[SCSI_VPD_HEADER_SIZE] __aligned(4);
|
||||
int result;
|
||||
|
||||
/*
|
||||
* Fetch the VPD page header to find out how big the page
|
||||
* is. This is done to prevent problems on legacy devices
|
||||
* which can not handle allocation lengths as large as
|
||||
* potentially requested by the caller.
|
||||
*/
|
||||
result = scsi_vpd_inquiry(sdev, vpd_header, page, sizeof(vpd_header));
|
||||
if (result < 0)
|
||||
return 0;
|
||||
|
||||
if (result < SCSI_VPD_HEADER_SIZE) {
|
||||
dev_warn_once(&sdev->sdev_gendev,
|
||||
"%s: short VPD page 0x%02x length: %d bytes\n",
|
||||
__func__, page, result);
|
||||
return 0;
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* scsi_get_vpd_page - Get Vital Product Data from a SCSI device
|
||||
* @sdev: The device to ask
|
||||
|
@ -330,47 +355,38 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char *buffer,
|
|||
*
|
||||
* SCSI devices may optionally supply Vital Product Data. Each 'page'
|
||||
* of VPD is defined in the appropriate SCSI document (eg SPC, SBC).
|
||||
* If the device supports this VPD page, this routine returns a pointer
|
||||
* to a buffer containing the data from that page. The caller is
|
||||
* responsible for calling kfree() on this pointer when it is no longer
|
||||
* needed. If we cannot retrieve the VPD page this routine returns %NULL.
|
||||
* If the device supports this VPD page, this routine fills @buf
|
||||
* with the data from that page and return 0. If the VPD page is not
|
||||
* supported or its content cannot be retrieved, -EINVAL is returned.
|
||||
*/
|
||||
int scsi_get_vpd_page(struct scsi_device *sdev, u8 page, unsigned char *buf,
|
||||
int buf_len)
|
||||
{
|
||||
int i, result;
|
||||
int result, vpd_len;
|
||||
|
||||
if (sdev->skip_vpd_pages)
|
||||
goto fail;
|
||||
if (!scsi_device_supports_vpd(sdev))
|
||||
return -EINVAL;
|
||||
|
||||
/* Ask for all the pages supported by this device */
|
||||
result = scsi_vpd_inquiry(sdev, buf, 0, buf_len);
|
||||
if (result < 4)
|
||||
goto fail;
|
||||
vpd_len = scsi_get_vpd_size(sdev, page);
|
||||
if (vpd_len <= 0)
|
||||
return -EINVAL;
|
||||
|
||||
/* If the user actually wanted this page, we can skip the rest */
|
||||
if (page == 0)
|
||||
return 0;
|
||||
vpd_len = min(vpd_len, buf_len);
|
||||
|
||||
for (i = 4; i < min(result, buf_len); i++)
|
||||
if (buf[i] == page)
|
||||
goto found;
|
||||
|
||||
if (i < result && 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, buf_len);
|
||||
/*
|
||||
* Fetch the actual page. Since the appropriate size was reported
|
||||
* by the device it is now safe to ask for something bigger.
|
||||
*/
|
||||
memset(buf, 0, buf_len);
|
||||
result = scsi_vpd_inquiry(sdev, buf, page, vpd_len);
|
||||
if (result < 0)
|
||||
goto fail;
|
||||
return -EINVAL;
|
||||
else if (result > vpd_len)
|
||||
dev_warn_once(&sdev->sdev_gendev,
|
||||
"%s: VPD page 0x%02x result %d > %d bytes\n",
|
||||
__func__, page, result, vpd_len);
|
||||
|
||||
return 0;
|
||||
|
||||
fail:
|
||||
return -EINVAL;
|
||||
}
|
||||
EXPORT_SYMBOL_GPL(scsi_get_vpd_page);
|
||||
|
||||
|
@ -384,9 +400,17 @@ EXPORT_SYMBOL_GPL(scsi_get_vpd_page);
|
|||
static struct scsi_vpd *scsi_get_vpd_buf(struct scsi_device *sdev, u8 page)
|
||||
{
|
||||
struct scsi_vpd *vpd_buf;
|
||||
int vpd_len = SCSI_VPD_PG_LEN, result;
|
||||
int vpd_len, result;
|
||||
|
||||
vpd_len = scsi_get_vpd_size(sdev, page);
|
||||
if (vpd_len <= 0)
|
||||
return NULL;
|
||||
|
||||
retry_pg:
|
||||
/*
|
||||
* Fetch the actual page. Since the appropriate size was reported
|
||||
* by the device it is now safe to ask for something bigger.
|
||||
*/
|
||||
vpd_buf = kmalloc(sizeof(*vpd_buf) + vpd_len, GFP_KERNEL);
|
||||
if (!vpd_buf)
|
||||
return NULL;
|
||||
|
@ -397,6 +421,9 @@ static struct scsi_vpd *scsi_get_vpd_buf(struct scsi_device *sdev, u8 page)
|
|||
return NULL;
|
||||
}
|
||||
if (result > vpd_len) {
|
||||
dev_warn_once(&sdev->sdev_gendev,
|
||||
"%s: VPD page 0x%02x result %d > %d bytes\n",
|
||||
__func__, page, result, vpd_len);
|
||||
vpd_len = result;
|
||||
kfree(vpd_buf);
|
||||
goto retry_pg;
|
||||
|
|
|
@ -100,6 +100,10 @@ struct scsi_vpd {
|
|||
unsigned char data[];
|
||||
};
|
||||
|
||||
enum scsi_vpd_parameters {
|
||||
SCSI_VPD_HEADER_SIZE = 4,
|
||||
};
|
||||
|
||||
struct scsi_device {
|
||||
struct Scsi_Host *host;
|
||||
struct request_queue *request_queue;
|
||||
|
@ -141,7 +145,6 @@ struct scsi_device {
|
|||
const char * model; /* ... after scan; point to static string */
|
||||
const char * rev; /* ... "nullnullnullnull" before scan */
|
||||
|
||||
#define SCSI_VPD_PG_LEN 255
|
||||
struct scsi_vpd __rcu *vpd_pg0;
|
||||
struct scsi_vpd __rcu *vpd_pg83;
|
||||
struct scsi_vpd __rcu *vpd_pg80;
|
||||
|
|
Loading…
Reference in a new issue