Re: [PATCH v8 2/8] scsi: mvumi: reject an out-of-range handshake frame length

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: mvumi: missing endianness conversion for frame_length
--

commit 86284f2a4fc8c1a95cd41e7f1687f3c482b376ac
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

scsi: mvumi: reject an out-of-range handshake frame length

This patch adds bounds checking to the frame_length obtained from the firmware
handshake page to prevent out-of-bounds reads and hangs during checksum
calculations. It also uses READ_ONCE() to ensure the DMA value is fetched only
once.

> diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
> index 38a92f8c25f5b..2378b676a7a78 100644
> --- a/drivers/scsi/mvumi.c
> +++ b/drivers/scsi/mvumi.c
> @@ -1039,9 +1039,15 @@ static int mvumi_hs_process_page(struct mvumi_hba *mhba,
>  {
>  	struct mvumi_hs_page1 *hs_page1;
>  	unsigned char page_checksum;
> +	u16 frame_length = READ_ONCE(hs_header->frame_length);
>  
> -	page_checksum = mvumi_calculate_checksum(hs_header,
> -						hs_header->frame_length);
> +	if (frame_length > HSP_MAX_SIZE - sizeof(struct mvumi_hs_header)) {

[Severity: Medium]
Is an endianness conversion required here before the bounds check?

hs_header->frame_length is populated by the PCI device, which typically uses
little-endian byte order. On big-endian architectures, reading it directly into
a u16 without le16_to_cpu() results in a byte-swapped value.

This swapped value will cause the bounds check against HSP_MAX_SIZE to
spuriously fail the handshake and abort the device probe. While the driver may
lack complete endian conversions overall, does this new check introduce an
immediate probe failure on big-endian platforms?

> +		dev_err(&mhba->pdev->dev, "invalid handshake frame length %d\n",
> +			frame_length);
> +		return -1;
> +	}
> +
> +	page_checksum = mvumi_calculate_checksum(hs_header, frame_length);
>  	if (page_checksum != hs_header->checksum) {
>  		dev_err(&mhba->pdev->dev, "checksum error\n");
>  		return -1;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015022728.3936714-1-yhlee@isslab.korea.ac.kr?part=2