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

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

[Thomas Lamprecht]

megasas_make_prp_nvme() builds a command's PRP list in cmd->sg_frame, a
DMA pool buffer of instance->max_chain_frame_sz bytes, spending one entry
per NVMe page of the transfer plus one per page of the buffer for the
chain pointer. The loop runs until the transfer is described and never
checks the buffer bound.

max_hw_sectors comes straight from the MDTS the firmware reports for the
drive. On drives with a large MDTS the only thing keeping the list inside
the buffer was the block layer default of 1280 KiB, which needs 320
entries, which fit into a 4 KiB frame as that holds 512. But since commit
9b8b84879d4a ("block: Increase BLK_DEF_MAX_SECTORS_CAP") that default is
4 MiB, and such a transfer needs 1025 entries, so the list runs a full
page past the end of the frame:

  sd 1:0:1:0: [sdb] tag#630 page boundary ptr_sgl: 0x00000000ba62d13f
  BUG: unable to handle page fault for address: ff663bcb81e7c000
  #PF: supervisor write access in kernel mode
  #PF: error_code(0x0002) - not-present page
  RIP: 0010:megasas_build_and_issue_cmd_fusion+0xeaa/0x1870 [megaraid_sas]

If the page after the frame happens to be mapped, the overrun does not
fault but silently corrupts the neighbouring pool entry, which is another
in-flight command's PRP list.

Cap max_hw_sectors at what the chain frame can describe, less one page
for transfers that do not start on a page boundary and so need one entry
more. This is the megaraid_sas counterpart of commit 04631f55afc5 ("scsi:
mpt3sas: Limit NVMe request size to 2 MiB"), but derives the limit from
max_chain_frame_sz rather than hardcoding it.

Cc: stable@vger.kernel.org
Fixes: 9b8b84879d4a ("block: Increase BLK_DEF_MAX_SECTORS_CAP")
Reported-by: Lukasz Magiera <me@magik.net>
Closes: https://lore.kernel.org/all/GPhsSM0vkgyIrs0DIZ62qeUZX7X4RxwQXVKiuvMx-lHQVSPDxpztUyQOGS0xikqvJ-Z94hMV-dW_5KN_0CX2hsfV7kTf_t0MTf6vdAAaSEc=@magik.net/
Reported-by: Mira Limbeck <m.limbeck@proxmox.com>
Closes: https://lore.kernel.org/all/d171cc76-bf25-48ce-b482-d344669dfc24@proxmox.com/
Suggested-by: Martin K. Petersen <martin.petersen@oracle.com>
Link: https://lore.kernel.org/all/yq17bmzd5jr.fsf@ca-mkp.ca.oracle.com/
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

Based on current master, but mostly tested on our downstream 7.0-based
kernel.

The derived cap was checked against a replay of the pointer arithmetic in
megasas_make_prp_nvme(), for chain frames of 1024
(MEGASAS_CHAIN_FRAME_SZ_MIN) to 15872 bytes and NVMe page sizes of 4 and
8 KiB, at every 512 byte start offset, with both fully coalesced and
one-entry-per-page scatterlists.

 drivers/scsi/megaraid/megaraid_sas_base.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index ecd365d78ae3c14f4682d61063b6f216974daa7d..d83abded2039ba9ffae23d0c78a79e155b32424f 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.
+	 */
+	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;
 }
 

base-commit: 18fbf5151d2c0bfe433c7428eef03cabf5fdb2fa
-- 
2.47.3

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

[sashiko-bot]

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

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

[Thomas Lamprecht]

Am 27.08.26 um 20:21 schrieb sashiko-bot@kernel.org:
> [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.

The observation is correct FWICT, but does not require a change to this
patch and probably also not in practice as no > 4K NVMes/controllers exist
AFAIK.

The sg_frame pool is created with an alignment of MR_DEFAULT_NVME_PAGE_SIZE:

	fusion->sg_dma_pool =
		dma_pool_create("mr_sg", &instance->pdev->dev,
				instance->max_chain_frame_sz,
				MR_DEFAULT_NVME_PAGE_SIZE, 0);

while the mask the loop tests against comes from

	mr_nvme_pg_size = max_t(u32, instance->nvme_page_size,
				MR_DEFAULT_NVME_PAGE_SIZE);

dma_pool backs its blocks with allocations of max(size, PAGE_SIZE), so the
CPU and DMA bases of a pool page are both PAGE_SIZE aligned and the blocks
sit at the same offset in each. The two tests agree as long as
mr_nvme_pg_size divides PAGE_SIZE, and can diverge once it is larger. The
driver then places the chain pointer where it sees a boundary and the
controller looks for one somewhere else, so it would read a data PRP as a
list pointer.

The misalignment can also introduce a chain pointer where an aligned buffer
has none: for a 4 KiB chain frame with an 8 KiB NVMe page size, a buffer at
0 modulo 8 KiB never trips the test, while one at 4 KiB modulo 8 KiB trips
it at offset 4088.

Reaching it needs instance->nvme_page_size above PAGE_SIZE, so above 4096 on
x86. The encoding allows it, 1 << (scratch_pad_3 & MR_NVME_PAGE_SIZE_MASK)
with a shift of at least 12, but NVMe puts the minimum memory page size at 4
KiB and the OCP datacenter SSD spec requires drives to report exactly that,
so I would not expect a larger value here. I have no data on what the MR
firmware actually reports, though. The driver logs the value at probe,

	dev_info(&instance->pdev->dev, "NVME page size\t: (%d)\n",
		 instance->nvme_page_size);

so a dmesg grep for "NVME page size" on any of the affected machines would
settle it. If one of them reports more than 4 KiB then this is live there,
and I would like to know.

This patch neither introduces nor widens that: it bounds how large a
transfer may be and does not touch where the chain pointers land.

The term that reserves room for the chain pointers,

	instance->max_chain_frame_sz / mr_nvme_pg_size

does share the assumption though: it counts boundaries as if the frame base
were page aligned, just as the test in megasas_make_prp_nvme() does, so with
a misaligned base it can be one short. I replayed the loop for chain frames
of 1024 to 15872 bytes, NVMe page sizes of 4, 8 and 16 KiB, every 4 KiB base
offset within a page, every 512 byte transfer start offset, and both
coalesced and one-entry-per-page scatterlists. The list never leaves the
frame, but above 4 KiB the worst case fills it exactly instead of leaving
the slot of slack it leaves today. At 4 KiB, where the pool alignment makes
the count exact, the margin stays.

So I would keep it out of this fix. The test wants ptr_sgl_phys instead;
that patch is below the scissors.

-- >8 --
Subject: [PATCH] scsi: megaraid_sas: Test PRP boundaries on DMA address

megasas_make_prp_nvme() decides where the chain pointer that continues a
PRP list into the next page goes by testing the CPU address of the next
entry:

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

The controller walks the same list by its DMA address. dma_pool backs its
blocks with allocations of max(size, PAGE_SIZE), so the CPU and DMA bases
of a pool page are both PAGE_SIZE aligned and the blocks sit at the same
offset in each. The two therefore agree modulo mr_nvme_pg_size only while
that divides PAGE_SIZE. Above it the driver can place the chain pointer
where the controller does not look for one, and the controller then
follows a data PRP as a list pointer.

The misalignment can add a boundary rather than move it: for a 4 KiB chain
frame with an 8 KiB NVMe page size a buffer at 0 modulo 8 KiB never trips
the test, while one at 4 KiB modulo 8 KiB trips it at offset 4088.

ptr_sgl_phys advances in lockstep with ptr_sgl, so test that instead.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-scsi/20260827182106.535D61F000E9@smtp.kernel.org
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
NOTE: not really tested end-to-end as I do not have any controller
reporting an NVMe page size above 4096 here, not even sure one exists?

 drivers/scsi/megaraid/megaraid_sas_fusion.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 056cbe50e19e..fad747206ed1 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -2221,7 +2221,7 @@ megasas_make_prp_nvme(struct megasas_instance *instance, struct scsi_cmnd *scmd,
 		offset = (u32)(sge_addr & page_mask);
 
 		/* Put PRP pointer due to page boundary*/
-		page_mask_result = (uintptr_t)(ptr_sgl + 1) & page_mask;
+		page_mask_result = (ptr_sgl_phys + 8) & page_mask;
 		if (unlikely(!page_mask_result)) {
 			scmd_printk(KERN_NOTICE,
 				    scmd, "page boundary ptr_sgl: 0x%p\n",

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

[Martin K. Petersen (Oracle)]

On Thu, 27 Aug 2026 19:24:24 +0200, Thomas Lamprecht wrote:

> megasas_make_prp_nvme() builds a command's PRP list in cmd->sg_frame, a
> DMA pool buffer of instance->max_chain_frame_sz bytes, spending one entry
> per NVMe page of the transfer plus one per page of the buffer for the
> chain pointer. The loop runs until the transfer is described and never
> checks the buffer bound.
> 
> max_hw_sectors comes straight from the MDTS the firmware reports for the
> drive. On drives with a large MDTS the only thing keeping the list inside
> the buffer was the block layer default of 1280 KiB, which needs 320
> entries, which fit into a 4 KiB frame as that holds 512. But since commit
> 9b8b84879d4a ("block: Increase BLK_DEF_MAX_SECTORS_CAP") that default is
> 4 MiB, and such a transfer needs 1025 entries, so the list runs a full
> page past the end of the frame:
> 
> [...]

Applied to 7.3/scsi-fixes, thanks!

[1/1] scsi: megaraid_sas: Limit NVMe request size to the PRP chain frame
      https://git.kernel.org/mkp/scsi/c/af8c27375733

-- 
Martin K. Petersen