Skip to contentSkip to conversation

scsi: megaraid_sas: fix PRP list out-of-bounds write

PATCH In review [email protected] 7 participants 11 messages Mar 27 – Jul 29

Subsystems (3)
SCSI Regressions Megaraid SCSI/SAS drivers
lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/1 scsi: megaraid_sas: fix PRP list out-of-bounds write

megasas_make_prp_nvme() builds NVMe PRP lists in cmd->sg_frame, which is a DMA-pool allocation sized by instance->max_chain_frame_sz.

On the affected controller, max_chain_frame_sz is 4096 bytes. The function stores 64-bit PRP entries in that buffer and, at each PRP-list page boundary, uses the last slot for a chain pointer to the next page.

When ptr_sgl reaches offset 4088, the code stores the chain pointer there, increments ptr_sgl, and then writes the next PRP entry at offset 4096, past the end of the allocation.

On an affected system this reproduces reliably on 6.19.10 during normal I/O to an NVMe device behind a MegaRAID SAS39xx controller:

  BUG: unable to handle page fault for address: ff76d0e56380c000
  #PF: supervisor write access in kernel mode
  #PF: error_code(0x0002) - not-present page
  RIP: 0010:megasas_make_prp_nvme.isra.0+0x12f/0x220 [megaraid_sas]
  RAX: 0000000000000200

RAX=0x200 indicates the fault happens at the 512th 8-byte slot, i.e. exactly the 4096-byte boundary of the chain frame.

Fix this by checking that the chain frame still has room before writing:

- the page-boundary chain pointer plus at least one following PRP entry
- each PRP entry itself

If either check fails, return false and let the caller use the existing IEEE SGL fallback path.

Tested on:
- ASUS ESC8000A-E13
- 2x AMD EPYC 9335
- Broadcom MegaRAID 9560-16i / SAS39xx
- KIOXIA 14TB NVMe behind the controller

Before this patch, 6.19.10 crashed repeatedly during boot and normal disk I/O. After applying it, the system boots cleanly and completes 4GB direct-I/O reads without crashes.

Cc [email protected]
Signed-off-by Lukasz Magiera <[email protected]>
---
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
drivers/scsi/megaraid/megaraid_sas_fusion.c +19 -0
index 4e498a6..29fa7f1 100644
@@ -2225,6 +2225,18 @@ megasas_make_prp_nvme(struct megasas_instance *instance, struct scsi_cmnd *scmd,
 		/* Put PRP pointer due to page boundary*/
 		page_mask_result = (uintptr_t)(ptr_sgl + 1) & page_mask;
 		if (unlikely(!page_mask_result)) {
+			/*
+			 * Bounds check: if the chain frame buffer cannot
+			 * fit the chain pointer plus at least one more
+			 * PRP entry, bail out to IEEE SGL fallback.
+			 * This prevents writing past the end of the
+			 * DMA-allocated chain frame buffer.
+			 */
+			if ((num_prp_in_chain + 2) * sizeof(u64) >
+			    instance->max_chain_frame_sz) {
+				build_prp = false;
+				break;
+			}
 			scmd_printk(KERN_NOTICE,
 				    scmd, "page boundary ptr_sgl: 0x%p\n",
 				    ptr_sgl);

@@ -2234,6 +2246,13 @@ megasas_make_prp_nvme(struct megasas_instance *instance, struct scsi_cmnd *scmd,
 			num_prp_in_chain++;
 		}
 
+		/* Bounds check: ensure space for this PRP entry */
+		if ((num_prp_in_chain + 1) * sizeof(u64) >
+		    instance->max_chain_frame_sz) {
+			build_prp = false;
+			break;
+		}
+
 		*ptr_sgl = cpu_to_le64(sge_addr);
 		ptr_sgl++;
 		ptr_sgl_phys += 8;
Signature · 1 line
2.43.0
#1 lore raw
Quoted 9 lines · On 3/27/26 04:20, [email protected] wrote:

megasas_make_prp_nvme() builds NVMe PRP lists in cmd->sg_frame, which is a DMA-pool allocation sized by instance->max_chain_frame_sz. [...]
Before this patch, 6.19.10 crashed repeatedly during boot and normal disk I/O. After applying it, the system boots cleanly and completes 4GB direct-I/O reads without crashes.

Cc [email protected]
Signed-off-by Lukasz Magiera <[email protected]>

You CCed the regression list, but this lacks a Fixes: tag, which makes me wonder: what change caused the problem? That tag would also help the stable team to see where this needs to be applied, so it most likely is needed.

Quoted 1 line

[...]

Ciao, Thorsten

#2 lore raw
In reply to [email protected]
Quoted 3 lines

megasas_make_prp_nvme() builds NVMe PRP lists in cmd->sg_frame, which is a DMA-pool allocation sized by instance->max_chain_frame_sz.

Broadcom: Please comment and review!

Signature · 1 line
Martin K. Petersen
#3 lore raw
Quoted 5 lines · Martin K. Petersen
Quoted 3 lines

megasas_make_prp_nvme() builds NVMe PRP lists in cmd->sg_frame, which is a DMA-pool allocation sized by instance->max_chain_frame_sz.

Broadcom: Please comment and review!

Martin, do you know if someone there ever looked into this regression and the proposed fix? I'm wondering because Daniel and Mats reported problems under the same subject line (in new threads), but also didn't get a reply. So together with the one from Lukasz aka "me" we afaics have three reports now:

https://lore.kernel.org/all/GPhsSM0vkgyIrs0DIZ62qeUZX7X4RxwQXVKiuvMx-lHQVSPDxpztUyQOGS0xikqvJ-Z94hMV-dW_5KN_0CX2hsfV7kTf_t0MTf6vdAAaSEc=@magik.net/
https://lore.kernel.org/all/[email protected]/
https://lore.kernel.org/all/[email protected]/

Side note: I still wonders if we somehow could fix this by reverting something, but none of the reports afaics references which change introduced the problem. And given that the problem already exists in 6.18 a clean and quick revert is unlikely anayway. :-/

Ciao, Thorsten

#4 lore raw

Hi,

We've seen some similar looking logs, but since they didn't mention prp at all, we haven't responded here before. Instead we sent a mail to the linux-scsi list [0], but got no response so far.

In our case the issue was first introduced by commit:
9b8b84879d4a ("block: Increase BLK_DEF_MAX_SECTORS_CAP")

This is similar to an issue we previously reported with the mpt3sas driver [1].

At least for our tests and one of our users we can say with certainty that reducing the queue sectors back to the previous value fixed the issue. In our tests this was done manually, and for one of our users with their root on the disks, it was handled via udev rules.

echo 1280 > /sys/block/<dev>/queue/max_sectors_kb

[0]
https://lore.kernel.org/all/[email protected]/
[1]
https://lore.kernel.org/all/[email protected]/

#5 lore raw
Quoted 2 lines · Mira Limbeck

We've seen some similar looking logs, but since they didn't mention prp at all, we haven't responded here before.

Thx for chiming in here.

Quoted 5 lines

Instead we sent a mail
to the linux-scsi list [0], but got no response so far.

In our case the issue was first introduced by commit:
9b8b84879d4a ("block: Increase BLK_DEF_MAX_SECTORS_CAP")

Any reason why you didn't CC the author and the committer of that change? Even if the bug is in the driver that might have been a good idea, but before doing that, let's do something else first:

Quoted 9 lines

This is similar to an issue we previously reported with the mpt3sas driver [1].

At least for our tests and one of our users we can say with certainty that reducing the queue sectors back to the previous value fixed the issue. In our tests this was done manually, and for one of our users with their root on the disks, it was handled via udev rules.

echo 1280 > /sys/block/<dev>/queue/max_sectors_kb

You also in your [0] mentioned that 12da89e8844a ("block: open code bio_add_page and fix handling of mismatching P2P ranges") fixed things in v7.0. Make me wonder if that is the case for the others affected by this. Hence:

Lukasz, Mats, Daniel, have you checked if 7.1 or 7.2-rc2 are still affected?

Ciao, Thorsten

Quoted 4 lines

[0]
https://lore.kernel.org/all/[email protected]/
[1]
https://lore.kernel.org/all/[email protected]/

#6 lore raw
Quoted 25 lines · Thorsten Leemhuis
Quoted 2 lines · Mira Limbeck

We've seen some similar looking logs, but since they didn't mention prp at all, we haven't responded here before.

Thx for chiming in here.

Quoted 5 lines

Instead we sent a mail
to the linux-scsi list [0], but got no response so far.

In our case the issue was first introduced by commit:
9b8b84879d4a ("block: Increase BLK_DEF_MAX_SECTORS_CAP")

Any reason why you didn't CC the author and the committer of that change? Even if the bug is in the driver that might have been a good idea, but before doing that, let's do something else first:

Quoted 9 lines

This is similar to an issue we previously reported with the mpt3sas driver [1].

At least for our tests and one of our users we can say with certainty that reducing the queue sectors back to the previous value fixed the issue. In our tests this was done manually, and for one of our users with their root on the disks, it was handled via udev rules.

echo 1280 > /sys/block/<dev>/queue/max_sectors_kb

Quoted 8 lines

You also in your [0] mentioned that 12da89e8844a ("block: open code bio_add_page and fix handling of mismatching P2P ranges") fixed things in v7.0. Make me wonder if that is the case for the others affected by this. Hence:

Lukasz, Mats, Daniel, have you checked if 7.1 or 7.2-rc2 are still affected?

Ciao, Thorsten

Actually, this only helped in our test cases. Some of our users that were affected with kernel 6.17 had no issues with kernel 7.0, while others still experience those issues.
So those 2 patches only fixed the issues for a subset of users.

Originally we thought that it fixes the issue for those using KIOXIA NVMes, while those with Microns were still affected. But since then we had additional reports where users with KIOXIA NVMes also still experienced issues with kernel 7.0.
This we only found out last week though.

We're still in the process of finding a reproducer that isn't fixed by that change.

Quoted 6 lines
Quoted 5 lines

[0]
https://lore.kernel.org/all/[email protected]/
[1]
https://lore.kernel.org/all/[email protected]/

#7 lore raw
In reply to Thorsten Leemhuis

Thorsten,

Quoted 4 lines

Martin, do you know if someone there ever looked into this regression and the proposed fix? I'm wondering because Daniel and Mats reported problems under the same subject line (in new threads), but also didn't get a reply.

I think this problem is just an unfortunate side effect of the kernel being able to build bigger I/Os by default.

Typically NVMe SSDs report fairly modest maximum I/O sizes compared to SCSI devices. I suspect this is why we only see this issue with a few select models. My hunch is that these drives advertise a fairly large MDTS.

Since there appears to be no traction wrt. fixing the MR PRP vs. SGL chaining logic, I wonder if the patch below is sufficient?

Signature · 1 line
Martin K. Petersen
drivers/scsi/megaraid/megaraid_sas_base.c +2 -1
index ecd365d78ae3..b93a6d1180ff 100644
@@ -1978,7 +1978,8 @@ megasas_set_nvme_device_properties(struct scsi_device *sdev,
 	mr_nvme_pg_size = max_t(u32, instance->nvme_page_size,
 				MR_DEFAULT_NVME_PAGE_SIZE);
 
-	lim->max_hw_sectors = max_io_size / 512;
+	lim->max_hw_sectors =
+		min(SZ_2M >> SECTOR_SHIFT, max_io_size >> SECTOR_SHIFT);
 	lim->virt_boundary_mask = mr_nvme_pg_size - 1;
 }
#8 lore raw

Hi all,

Sorry for the late reply. I was unable to follow up on this earlier because I had to prioritize other issues.

In the meantime, we pinned our kernel to version 6.8.12-4-pve on Proxmox, which has been stable for our use case and hardware. Given the recent 7.x release, I will take another look this week and report back.

We are using KIOXIA NVMe SSDs, so we will see how it goes.

Thanks,
Daniel

Quoted 37 lines · Martin K. Petersen

Thorsten,

Quoted 4 lines

Martin, do you know if someone there ever looked into this regression and the proposed fix? I'm wondering because Daniel and Mats reported problems under the same subject line (in new threads), but also didn't get a reply.

I think this problem is just an unfortunate side effect of the kernel being able to build bigger I/Os by default.

Typically NVMe SSDs report fairly modest maximum I/O sizes compared to SCSI devices. I suspect this is why we only see this issue with a few select models. My hunch is that these drives advertise a fairly large MDTS.

Since there appears to be no traction wrt. fixing the MR PRP vs. SGL chaining logic, I wonder if the patch below is sufficient?

--
Martin K. Petersen

drivers/scsi/megaraid/megaraid_sas_base.c +2 -1
index ecd365d78ae3..b93a6d1180ff 100644
@@ -1978,7 +1978,8 @@ megasas_set_nvme_device_properties(struct scsi_device *sdev,
 	mr_nvme_pg_size = max_t(u32, instance->nvme_page_size,
 				MR_DEFAULT_NVME_PAGE_SIZE);
 
-	lim->max_hw_sectors = max_io_size / 512;
+	lim->max_hw_sectors =
+		min(SZ_2M >> SECTOR_SHIFT, max_io_size >> SECTOR_SHIFT);
 	lim->virt_boundary_mask = mr_nvme_pg_size - 1;
 }
#9 lore raw

Hi all,

Adding a data point from an HPE ProLiant DL360 Gen11 with the MR408i-o Gen11 controller (megaraid_sas), 960 GB SAS SSDs presented as sd*, running kernel 6.18.32 (Talos Linux v1.12.8). Without a cap the node panics within seconds of sustained block I/O to the controller, with the same megasas_make_prp_nvme signature discussed in this thread.

Capping max_sectors_kb=1280 on the sd* devices resolves it on this controller by itself. I pushed roughly 85 GB of dd bs=4M oflag=direct through it, so every request is split at the 1280 KB boundary, with zero panics, where the uncapped case dies almost immediately. On the MR416i-o you found the cap necessary but not sufficient because of a second problem in the SG advance path. I did not reach that second failure on the MR408i-o; the cap alone was enough here.

Both of our worker nodes have now run on kernel 6.18 throughout this testing with the cap applied by a udev rule at device add:

ACTION=="add|change", SUBSYSTEM=="block", KERNEL=="sd[a-z]", DRIVERS=="megaraid_sas", ATTR{queue/max_sectors_kb}="1280"

This lines up with Martin's suggestion to cap max_hw_sectors on the driver side for NVMe behind MR. If a driver side patch would help, I am happy to build and test it on this exact hardware and report back, and I can provide a full dmesg or a serial panic capture from the uncapped case if that is useful.

Thanks,
Mats

Quoted 55 lines · Daniel Fernau

Hi all,

Sorry for the late reply. I was unable to follow up on this earlier because I had to prioritize other issues.

In the meantime, we pinned our kernel to version 6.8.12-4-pve on Proxmox, which has been stable for our use case and hardware. Given the recent 7.x release, I will take another look this week and report back.

We are using KIOXIA NVMe SSDs, so we will see how it goes.

Thanks,
Daniel

Quoted 37 lines · Martin K. Petersen

Thorsten,

Quoted 4 lines

Martin, do you know if someone there ever looked into this regression and the proposed fix? I'm wondering because Daniel and Mats reported problems under the same subject line (in new threads), but also didn't get a reply.

I think this problem is just an unfortunate side effect of the kernel being able to build bigger I/Os by default.

Typically NVMe SSDs report fairly modest maximum I/O sizes compared to SCSI devices. I suspect this is why we only see this issue with a few select models. My hunch is that these drives advertise a fairly large MDTS.

Since there appears to be no traction wrt. fixing the MR PRP vs. SGL chaining logic, I wonder if the patch below is sufficient?

--
Martin K. Petersen

drivers/scsi/megaraid/megaraid_sas_base.c +2 -1
index ecd365d78ae3..b93a6d1180ff 100644
@@ -1978,7 +1978,8 @@ megasas_set_nvme_device_properties(struct scsi_device *sdev,
 	mr_nvme_pg_size = max_t(u32, instance->nvme_page_size,
 				MR_DEFAULT_NVME_PAGE_SIZE);
 
-	lim->max_hw_sectors = max_io_size / 512;
+	lim->max_hw_sectors =
+		min(SZ_2M >> SECTOR_SHIFT, max_io_size >> SECTOR_SHIFT);
 	lim->virt_boundary_mask = mr_nvme_pg_size - 1;
 }
#10 lore raw
Quoted 18 lines · Mats Topstad / Intility AS

Hi all,

Adding a data point from an HPE ProLiant DL360 Gen11 with the MR408i-o Gen11 controller (megaraid_sas), 960 GB SAS SSDs presented as sd*, running kernel 6.18.32 (Talos Linux v1.12.8). Without a cap the node panics within seconds of sustained block I/O to the controller, with the same megasas_make_prp_nvme signature discussed in this thread.

Capping max_sectors_kb=1280 on the sd* devices resolves it on this controller by itself. I pushed roughly 85 GB of dd bs=4M oflag=direct through it, so every request is split at the 1280 KB boundary, with zero panics, where the uncapped case dies almost immediately. On the MR416i-o you found the cap necessary but not sufficient because of a second problem in the SG advance path. I did not reach that second failure on the MR408i-o; the cap alone was enough here.

Both of our worker nodes have now run on kernel 6.18 throughout this testing with the cap applied by a udev rule at device add:

Quoted 6 lines

ACTION=="add|change", SUBSYSTEM=="block", KERNEL=="sd[a-z]", DRIVERS=="megaraid_sas", ATTR{queue/max_sectors_kb}="1280"

This lines up with Martin's suggestion to cap max_hw_sectors on the driver side for NVMe behind MR. If a driver side patch would help, I am happy to build and test it on this exact hardware and report back, and I can provide a full dmesg or a serial panic capture from the uncapped case if that is useful.

This looks about the same problem as what commit 04631f55afc5 ("scsi: mpt3sas:
Limit NVMe request size to 2 MiB") fixes.

I strongly suspect that all Broadcom drivers (mpi3mr and megaraid) all need a similar fix.

Signature · 2 lines
Damien Le Moal
Western Digital Research