On Wed, Feb 14, 2024 at 2:14 PM Martin K. Petersen
<martin.petersen@xxxxxxxxxx> wrote:
>
> Commit c92a6b5d6335 ("scsi: core: Query VPD size before getting full
> page") removed the logic which checks whether a VPD page is present on
> the supported pages list before asking for the page itself. That was
> done because SPC helpfully states "The Supported VPD Pages VPD page
> list may or may not include all the VPD pages that are able to be
> returned by the device server". Testing had revealed a few devices
> that supported some of the 0xBn pages but didn't actually list them in
> page 0.
>
> Julian Sikorski bisected a problem with his drive resetting during
> discovery to the commit above. As it turns out, this particular drive
> firmware will crash if we attempt to fetch page 0xB9.
>
> Various approaches were attempted to work around this. In the end,
> reinstating the logic that consults VPD page 0 before fetching any
> other page was the path of least resistance. A firmware update for the
> devices which originally compelled us to remove the check has since
> been released.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Bart Van Assche <bvanassche@xxxxxxx>
> Fixes: c92a6b5d6335 ("scsi: core: Query VPD size before getting full page")
> Reported-by: Julian Sikorski <belegdol@xxxxxxxxx>
> Tested-by: Julian Sikorski <belegdol@xxxxxxxxx>
> Signed-off-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
>
> ---
>
> v2: Address Bart's comments.
> ---
> drivers/scsi/scsi.c | 22 ++++++++++++++++++++--
> include/scsi/scsi_device.h | 4 ----
> 2 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 76d369343c7a..8cad9792a562 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -328,21 +328,39 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char *buffer,
> return result + 4;
> }
>
> +enum scsi_vpd_parameters {
> + SCSI_VPD_HEADER_SIZE = 4,
> + SCSI_VPD_LIST_SIZE = 36,
> +};
> +
> static int scsi_get_vpd_size(struct scsi_device *sdev, u8 page)
> {
> - unsigned char vpd_header[SCSI_VPD_HEADER_SIZE] __aligned(4);
> + unsigned char vpd[SCSI_VPD_LIST_SIZE] __aligned(4);
> int result;
>
> if (sdev->no_vpd_size)
> return SCSI_DEFAULT_VPD_LEN;
>
> + /*
> + * Fetch the supported pages VPD and validate that the requested page
> + * number is present.
> + */
> + if (page != 0) {
> + result = scsi_vpd_inquiry(sdev, vpd, 0, sizeof(vpd));
> + if (result < SCSI_VPD_HEADER_SIZE)
> + return 0;
> +
> + result -= SCSI_VPD_HEADER_SIZE;
> + if (!memchr(&vpd[SCSI_VPD_HEADER_SIZE], page, result))
> + return 0;
> + }
> /*
> * 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));
> + result = scsi_vpd_inquiry(sdev, vpd, page, SCSI_VPD_HEADER_SIZE);
> if (result < 0)
> return 0;
>
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index cb019c80763b..72a6b3923fc7 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -100,10 +100,6 @@ 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;
> --
> 2.42.1
>
>
Reviewed-by: Lee Duncan <lee.duncan@xxxxxxxx>
