diff mbox series

scsi: core: Handle devices which return an unusually large VPD page count

Message ID 20240521023040.2703884-1-martin.petersen@oracle.com (mailing list archive)
State Accepted
Headers show
Series scsi: core: Handle devices which return an unusually large VPD page count | expand

Commit Message

Martin K. Petersen May 21, 2024, 2:30 a.m. UTC
Peter Schneider reported that a system would no longer boot after
updating to 6.8.4.  Peter bisected the issue and identified commit
b5fc07a5fb56 ("scsi: core: Consult supported VPD page list prior to
fetching page") as being the culprit.

Turns out the enclosure device in Peter's system reports a byteswapped
page length for VPD page 0. It reports "02 00" as page length instead
of "00 02". This causes us to attempt to access 516 bytes (page length
+ header) of information despite only 2 pages being present.

Limit the page search scope to the size of our VPD buffer to guard
against devices returning a larger page count than requested.

Cc: stable@vger.kernel.org
Reported-by: Peter Schneider <pschneider1968@googlemail.com>
Tested-by: Peter Schneider <pschneider1968@googlemail.com>
Fixes: b5fc07a5fb56 ("scsi: core: Consult supported VPD page list prior to fetching page")
Link: https://lore.kernel.org/all/eec6ebbf-061b-4a7b-96dc-ea748aa4d035@googlemail.com/
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/scsi.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Peter Schneider May 21, 2024, 1:32 p.m. UTC | #1
Am 21.05.2024 um 04:30 schrieb Martin K. Petersen:
 > Peter Schneider reported that a system would no longer boot after
 > updating to 6.8.4.  Peter bisected the issue and identified commit
 > b5fc07a5fb56 ("scsi: core: Consult supported VPD page list prior to
 > fetching page") as being the culprit.
 >
 > Turns out the enclosure device in Peter's system reports a byteswapped
 > page length for VPD page 0. It reports "02 00" as page length instead
 > of "00 02". This causes us to attempt to access 516 bytes (page length
 > + header) of information despite only 2 pages being present.
 >
 > Limit the page search scope to the size of our VPD buffer to guard
 > against devices returning a larger page count than requested.
 >
 > Cc: stable@vger.kernel.org
 > Reported-by: Peter Schneider <pschneider1968@googlemail.com>
 > Tested-by: Peter Schneider <pschneider1968@googlemail.com>
 > Fixes: b5fc07a5fb56 ("scsi: core: Consult supported VPD page list prior to fetching page")
 > Link: https://lore.kernel.org/all/eec6ebbf-061b-4a7b-96dc-ea748aa4d035@googlemail.com/
 > Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
 > ---
 >   drivers/scsi/scsi.c | 7 +++++++
 >   1 file changed, 7 insertions(+)
 >
 > diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
 > index 3e0c0381277a..f0464db3f9de 100644
 > --- a/drivers/scsi/scsi.c
 > +++ b/drivers/scsi/scsi.c
 > @@ -350,6 +350,13 @@ static int scsi_get_vpd_size(struct scsi_device *sdev, u8 page)
 >   		if (result < SCSI_VPD_HEADER_SIZE)
 >   			return 0;
 >
 > +		if (result > sizeof(vpd)) {
 > +			dev_warn_once(&sdev->sdev_gendev,
 > +				      "%s: long VPD page 0 length: %d bytes\n",
 > +				      __func__, result);
 > +			result = sizeof(vpd);
 > +		}
 > +
 >   		result -= SCSI_VPD_HEADER_SIZE;
 >   		if (!memchr(&vpd[SCSI_VPD_HEADER_SIZE], page, result))
 >   			return 0;



I have built and tested Martin's patch against 6.8.4, 6.8.10, and 6.9.1, and it works fine 
and fixes my issue.

Tested-by: Peter Schneider <pschneider1968@googlemail.com>

In case anybody else is affected: The enclosure device in question with that buggy 
behaviour is that in a Supermicro 745BTQ-R920B server casing, with SAS/SATA Backplane 
"743 SAS BACKPLANE W/AMI MG9072", MG9072 being the controller chip by American Megatrends, 
Inc. according to the device documentation which can be found here:

https://www.supermicro.com/de/products/chassis/4u/745/sc745btq-r920b



Beste Grüße,
Peter Schneider
Bart Van Assche May 23, 2024, 10:57 p.m. UTC | #2
On 5/20/24 19:30, Martin K. Petersen wrote:
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 3e0c0381277a..f0464db3f9de 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -350,6 +350,13 @@ static int scsi_get_vpd_size(struct scsi_device *sdev, u8 page)
>   		if (result < SCSI_VPD_HEADER_SIZE)
>   			return 0;
>   
> +		if (result > sizeof(vpd)) {
> +			dev_warn_once(&sdev->sdev_gendev,
> +				      "%s: long VPD page 0 length: %d bytes\n",
> +				      __func__, result);
> +			result = sizeof(vpd);
> +		}
> +
>   		result -= SCSI_VPD_HEADER_SIZE;
>   		if (!memchr(&vpd[SCSI_VPD_HEADER_SIZE], page, result))
>   			return 0;

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Christoph Hellwig May 24, 2024, 8:10 a.m. UTC | #3
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 3e0c0381277a..f0464db3f9de 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -350,6 +350,13 @@  static int scsi_get_vpd_size(struct scsi_device *sdev, u8 page)
 		if (result < SCSI_VPD_HEADER_SIZE)
 			return 0;
 
+		if (result > sizeof(vpd)) {
+			dev_warn_once(&sdev->sdev_gendev,
+				      "%s: long VPD page 0 length: %d bytes\n",
+				      __func__, result);
+			result = sizeof(vpd);
+		}
+
 		result -= SCSI_VPD_HEADER_SIZE;
 		if (!memchr(&vpd[SCSI_VPD_HEADER_SIZE], page, result))
 			return 0;