Re: [PATCH] scsi: megaraid_sas: Limit NVMe request size to the PRP chain frame

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Preexisting: `megasas_make_prp_nvme` uses the virtual address instead of the physical address to detect NVMe page boundaries, which breaks PRP chain pointers if the NVMe page size exceeds the DMA pool alignment.
--

commit fd59af10f7e697e784da3de5d62a37975fb7023a
Author: Thomas Lamprecht <t.lamprecht@proxmox.com>

scsi: megaraid_sas: Limit NVMe request size to the PRP chain frame

This patch limits the max_hw_sectors for NVMe transfers in the megaraid_sas
driver to prevent PRP list construction from overrunning its DMA buffer
boundary. It calculates the capacity dynamically based on the adapter's
max_chain_frame_sz.

Link: https://lore.kernel.org/all/yq17bmzd5jr.fsf@ca-mkp.ca.oracle.com/

> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> index ecd365d78ae3c..d83abded2039b 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -1973,12 +1973,23 @@ megasas_set_nvme_device_properties(struct scsi_device *sdev,
>  {
>  	struct megasas_instance *instance;
>  	u32 mr_nvme_pg_size;
> +	u64 max_prp_io;
>  
>  	instance = (struct megasas_instance *)sdev->host->hostdata;
>  	mr_nvme_pg_size = max_t(u32, instance->nvme_page_size,
>  				MR_DEFAULT_NVME_PAGE_SIZE);
>  
> -	lim->max_hw_sectors = max_io_size / 512;
> +	/*
> +	 * megasas_make_prp_nvme() builds the PRP list in cmd->sg_frame without
> +	 * bounding it against that buffer, and spends one entry per page of
> +	 * it on the chain pointer. Cap the transfer at what the buffer holds,
> +	 * less one page for lists that start off a page boundary.
> +	 */

[Severity: Critical]
This is a pre-existing issue, but since this patch directly addresses how
megasas_make_prp_nvme() builds PRP chains and accounts for page boundaries,
does the chain boundary detection logic in that function safely calculate
the boundary?

Looking at megasas_make_prp_nvme() in
drivers/scsi/megaraid/megaraid_sas_fusion.c:

	page_mask_result = (uintptr_t)(ptr_sgl + 1) & page_mask;

This tests the virtual address (ptr_sgl) against the NVMe page mask instead
of the physical IOVA address. If the system uses an IOMMU and creates an
unaligned virtual mapping for the DMA pool (modulo the NVMe page size), this
will miscalculate physical NVMe page boundaries.

Could this result in the NVMe controller misinterpreting the PRP list,
treating data PRPs as chain pointers and reading or writing disk data to
arbitrary physical addresses provided by user data?

> +	max_prp_io = (u64)((instance->max_chain_frame_sz / sizeof(u64)) -
> +			   (instance->max_chain_frame_sz / mr_nvme_pg_size) - 1) *
> +		     mr_nvme_pg_size;
> +
> +	lim->max_hw_sectors = min_t(u64, max_io_size, max_prp_io) >> SECTOR_SHIFT;
>  	lim->virt_boundary_mask = mr_nvme_pg_size - 1;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827175743.734593-1-t.lamprecht@proxmox.com?part=1