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

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.

---

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(-)

index ecd365d78ae3c14f4682d61063b6f216974daa7d..d83abded2039ba9ffae23d0c78a79e155b32424f 100644
@@ -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

Am 27.08.26 um 20:21 schrieb [email protected]:

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.

---
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(-)

index 056cbe50e19e..fad747206ed1 100644
@@ -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",

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

Follow-up from our earlier note on pinning 6.8.12-4-pve.

We have now been running the HPE MR416i-o Gen11 cluster on Proxmox kernel 7.0.14-15-pve without the previous megasas_make_prp_nvme overflow.

Hardware / firmware
- HPE MR416i-o Gen11 (Broadcom PCI 0x1000:0x10e2)
- HPE FW package 52.32.3-6333 / controller FW 5.320.03-4207
- megaraid_sas 07.734.00.00-rc1
- KIOXIA NVMe behind the controller

7.0.14-15-pve includes
"scsi: megaraid_sas: Limit NVMe request size to the PRP chain frame" (pve-kernel 46a6f4f). That is the driver-side max_hw_sectors cap Martin suggested and the megaraid equivalent of Damien's 04631f55afc5 ("scsi: mpt3sas: Limit NVMe request size to 2 MiB"), not Lukasz's original bounds check in megasas_make_prp_nvme().

On this hardware the cap is enough. We no longer need the max_sectors_kb udev workaround, and we do not see the page-boundary ptr_sgl fault (RAX=0x200 / 512th PRP slot) under production load.

Thanks,
Daniel

Daniel,

Great, thanks for testing!

Martin K. Petersen