Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH v3] nvme-pci: fix CMB mapping when CMBSZ Size field is zero
@ 2026-06-22  2:41 ` kangfenglong
  2026-06-22  2:47   ` sashiko-bot
                     ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: kangfenglong @ 2026-06-22  2:41 UTC (permalink / raw)
  To: kbusch; +Cc: linux-nvme, linux-pci, liuyonglong, seven.wangyu, kangfenglong

The controller memory buffer size is defined by the SZ field in the
CMBSZ register (bits 31:12). According to the NVMe specification, a
value of zero in the SZ field indicates that no CMB is present.

Commit f65efd6dfe4e ("nvme-pci: clean up CMB initialization") replaced
the check for a zero SZ field with a check for a zero CMBSZ register
value, under the assumption that a zero register implies no CMB.
However, a CMBSZ register can be non-zero while the SZ field is zero,
for example when Size Units (SZU) is set to a non-zero value but the
actual size is zero (e.g. CMBSZ = 0x100: SZU = 1, SZ = 0).

When this happens, nvme_map_cmb() proceeds to compute a size of zero,
passes the alignment checks (zero is always aligned), and calls
pci_p2pdma_add_resource() with size=0. The P2PDMA subsystem then
defaults size to the entire remaining BAR, which may not be properly
aligned for memory hotplug, triggering:

  WARNING: CPU: 0 PID: 10 at mm/memory_hotplug.c:396 __add_pages
  Misaligned __add_pages start: 0x494a0000 end: 0x494a000f

Fix this by:
1. Restoring the check for a zero SZ field, which explicitly verifies
   that the controller actually has a CMB before proceeding.
2. Adding a boundary check in nvme_cmb_size_unit() to reject SZU values
   greater than 6. The SZU field is 4 bits, but values 7-15 are reserved
   by the NVMe specification and produce a shift count >= 40 in the
   left-shift operation (1ULL << (12 + 4 * szu) when szu >= 7), which
   exceeds the maximum 64 GB unit size defined by the spec.
3. Checking the return value of nvme_cmb_size_unit() for zero before
   proceeding, and using check_mul_overflow() for the size calculation
   to detect integer overflow that could wrap to a non-zero value and
   bypass the SZ check.

Fixes: f65efd6dfe4e ("nvme-pci: clean up CMB initialization")
Signed-off-by: kangfenglong <kangfenglong@huawei.com>
---
 drivers/nvme/host/pci.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 8438c904ec49..ee2d782f4957 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2448,6 +2448,9 @@ static u64 nvme_cmb_size_unit(struct nvme_dev *dev)
 {
 	u8 szu = (dev->cmbsz >> NVME_CMBSZ_SZU_SHIFT) & NVME_CMBSZ_SZU_MASK;
 
+	if (szu > 6)
+		return 0;
+
 	return 1ULL << (12 + 4 * szu);
 }
 
@@ -2458,7 +2461,7 @@ static u32 nvme_cmb_size(struct nvme_dev *dev)
 
 static void nvme_map_cmb(struct nvme_dev *dev)
 {
-	u64 size, offset;
+	u64 size, unit_size, offset;
 	resource_size_t bar_size;
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	int bar;
@@ -2472,9 +2475,15 @@ static void nvme_map_cmb(struct nvme_dev *dev)
 	dev->cmbsz = readl(dev->bar + NVME_REG_CMBSZ);
 	if (!dev->cmbsz)
 		return;
+	if (!nvme_cmb_size(dev))
+		return;
 	dev->cmbloc = readl(dev->bar + NVME_REG_CMBLOC);
 
-	size = nvme_cmb_size_unit(dev) * nvme_cmb_size(dev);
+	unit_size = nvme_cmb_size_unit(dev);
+	if (!unit_size)
+		return;
+	if (check_mul_overflow(unit_size, nvme_cmb_size(dev), &size))
+		return;
 	offset = nvme_cmb_size_unit(dev) * NVME_CMB_OFST(dev->cmbloc);
 	bar = NVME_CMB_BIR(dev->cmbloc);
 	bar_size = pci_resource_len(pdev, bar);
-- 
2.12.0.windows.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] nvme-pci: fix CMB mapping when CMBSZ Size field is zero
  2026-06-22  2:41 ` [PATCH v3] nvme-pci: fix CMB mapping when CMBSZ Size field is zero kangfenglong
@ 2026-06-22  2:47   ` sashiko-bot
  2026-09-02  7:45   ` Kangfenglong
  2026-09-03  3:53   ` Kanchan Joshi
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-06-22  2:47 UTC (permalink / raw)
  To: kangfenglong; +Cc: linux-pci

> The controller memory buffer size is defined by the SZ field in the
> CMBSZ register (bits 31:12). According to the NVMe specification, a
> value of zero in the SZ field indicates that no CMB is present.
> 
> Commit f65efd6dfe4e ("nvme-pci: clean up CMB initialization") replaced
> the check for a zero SZ field with a check for a zero CMBSZ register
> value, under the assumption that a zero register implies no CMB.
> However, a CMBSZ register can be non-zero while the SZ field is zero,
> for example when Size Units (SZU) is set to a non-zero value but the
> actual size is zero (e.g. CMBSZ = 0x100: SZU = 1, SZ = 0).
> 
> When this happens, nvme_map_cmb() proceeds to compute a size of zero,
> passes the alignment checks (zero is always aligned), and calls
> pci_p2pdma_add_resource() with size=0. The P2PDMA subsystem then
> defaults size to the entire remaining BAR, which may not be properly
> [ ... ]
>    to detect integer overflow that could wrap to a non-zero value and
>    bypass the SZ check.
> 
> Fixes: f65efd6dfe4e ("nvme-pci: clean up CMB initialization")
> Signed-off-by: kangfenglong <kangfenglong@huawei.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622024129.40800-1-kangfenglong@huawei.com?part=1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3] nvme-pci: fix CMB mapping when CMBSZ Size field is zero
  2026-06-22  2:41 ` [PATCH v3] nvme-pci: fix CMB mapping when CMBSZ Size field is zero kangfenglong
  2026-06-22  2:47   ` sashiko-bot
@ 2026-09-02  7:45   ` Kangfenglong
  2026-09-03  3:53   ` Kanchan Joshi
  2 siblings, 0 replies; 4+ messages in thread
From: Kangfenglong @ 2026-09-02  7:45 UTC (permalink / raw)
  To: kbusch@kernel.org
  Cc: linux-nvme@lists.infradead.org, linux-pci@vger.kernel.org,
	liuyonglong, Wangyu (Eric)

Friendly ping.
It has been over two months since this patch was submitted. I would highly
 appreciate it if someone could take a quick look or provide some feedback.
Please let me know if any modifications are needed.
Thanks!
-----邮件原件-----
发件人: Kangfenglong <kangfenglong@huawei.com> 
发送时间: 2026年6月22日 10:41
收件人: kbusch@kernel.org
抄送: linux-nvme@lists.infradead.org; linux-pci@vger.kernel.org; liuyonglong <liuyonglong@huawei.com>; Wangyu (Eric) <seven.wangyu@huawei.com>; Kangfenglong <kangfenglong@huawei.com>
主题: [PATCH v3] nvme-pci: fix CMB mapping when CMBSZ Size field is zero

The controller memory buffer size is defined by the SZ field in the CMBSZ register (bits 31:12). According to the NVMe specification, a value of zero in the SZ field indicates that no CMB is present.

Commit f65efd6dfe4e ("nvme-pci: clean up CMB initialization") replaced the check for a zero SZ field with a check for a zero CMBSZ register value, under the assumption that a zero register implies no CMB.
However, a CMBSZ register can be non-zero while the SZ field is zero, for example when Size Units (SZU) is set to a non-zero value but the actual size is zero (e.g. CMBSZ = 0x100: SZU = 1, SZ = 0).

When this happens, nvme_map_cmb() proceeds to compute a size of zero, passes the alignment checks (zero is always aligned), and calls
pci_p2pdma_add_resource() with size=0. The P2PDMA subsystem then defaults size to the entire remaining BAR, which may not be properly aligned for memory hotplug, triggering:

  WARNING: CPU: 0 PID: 10 at mm/memory_hotplug.c:396 __add_pages
  Misaligned __add_pages start: 0x494a0000 end: 0x494a000f

Fix this by:
1. Restoring the check for a zero SZ field, which explicitly verifies
   that the controller actually has a CMB before proceeding.
2. Adding a boundary check in nvme_cmb_size_unit() to reject SZU values
   greater than 6. The SZU field is 4 bits, but values 7-15 are reserved
   by the NVMe specification and produce a shift count >= 40 in the
   left-shift operation (1ULL << (12 + 4 * szu) when szu >= 7), which
   exceeds the maximum 64 GB unit size defined by the spec.
3. Checking the return value of nvme_cmb_size_unit() for zero before
   proceeding, and using check_mul_overflow() for the size calculation
   to detect integer overflow that could wrap to a non-zero value and
   bypass the SZ check.

Fixes: f65efd6dfe4e ("nvme-pci: clean up CMB initialization")
Signed-off-by: kangfenglong <kangfenglong@huawei.com>
---
 drivers/nvme/host/pci.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 8438c904ec49..ee2d782f4957 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2448,6 +2448,9 @@ static u64 nvme_cmb_size_unit(struct nvme_dev *dev)  {
 	u8 szu = (dev->cmbsz >> NVME_CMBSZ_SZU_SHIFT) & NVME_CMBSZ_SZU_MASK;
 
+	if (szu > 6)
+		return 0;
+
 	return 1ULL << (12 + 4 * szu);
 }
 
@@ -2458,7 +2461,7 @@ static u32 nvme_cmb_size(struct nvme_dev *dev)
 
 static void nvme_map_cmb(struct nvme_dev *dev)  {
-	u64 size, offset;
+	u64 size, unit_size, offset;
 	resource_size_t bar_size;
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	int bar;
@@ -2472,9 +2475,15 @@ static void nvme_map_cmb(struct nvme_dev *dev)
 	dev->cmbsz = readl(dev->bar + NVME_REG_CMBSZ);
 	if (!dev->cmbsz)
 		return;
+	if (!nvme_cmb_size(dev))
+		return;
 	dev->cmbloc = readl(dev->bar + NVME_REG_CMBLOC);
 
-	size = nvme_cmb_size_unit(dev) * nvme_cmb_size(dev);
+	unit_size = nvme_cmb_size_unit(dev);
+	if (!unit_size)
+		return;
+	if (check_mul_overflow(unit_size, nvme_cmb_size(dev), &size))
+		return;
 	offset = nvme_cmb_size_unit(dev) * NVME_CMB_OFST(dev->cmbloc);
 	bar = NVME_CMB_BIR(dev->cmbloc);
 	bar_size = pci_resource_len(pdev, bar);
--
2.12.0.windows.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] nvme-pci: fix CMB mapping when CMBSZ Size field is zero
  2026-06-22  2:41 ` [PATCH v3] nvme-pci: fix CMB mapping when CMBSZ Size field is zero kangfenglong
  2026-06-22  2:47   ` sashiko-bot
  2026-09-02  7:45   ` Kangfenglong
@ 2026-09-03  3:53   ` Kanchan Joshi
  2 siblings, 0 replies; 4+ messages in thread
From: Kanchan Joshi @ 2026-09-03  3:53 UTC (permalink / raw)
  To: kangfenglong, kbusch; +Cc: linux-nvme, linux-pci, liuyonglong, seven.wangyu

On 6/22/2026 8:11 AM, kangfenglong wrote:
>   static void nvme_map_cmb(struct nvme_dev *dev)
>   {
> -	u64 size, offset;
> +	u64 size, unit_size, offset;
>   	resource_size_t bar_size;
>   	struct pci_dev *pdev = to_pci_dev(dev->dev);
>   	int bar;
> @@ -2472,9 +2475,15 @@ static void nvme_map_cmb(struct nvme_dev *dev)
>   	dev->cmbsz = readl(dev->bar + NVME_REG_CMBSZ);
>   	if (!dev->cmbsz)
>   		return;
> +	if (!nvme_cmb_size(dev))
> +		return;
>   	dev->cmbloc = readl(dev->bar + NVME_REG_CMBLOC);
>   
> -	size = nvme_cmb_size_unit(dev) * nvme_cmb_size(dev);
> +	unit_size = nvme_cmb_size_unit(dev);
> +	if (!unit_size)
> +		return;
> +	if (check_mul_overflow(unit_size, nvme_cmb_size(dev), &size))
> +		return;
>   	offset = nvme_cmb_size_unit(dev) * NVME_CMB_OFST(dev->cmbloc);

nit: now unit_size can be used in this line.

With that, LGTM.

Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-03  3:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20260622031353epcas5p20f6585c6c8c9b2753e6884d99fe6670a@epcas5p2.samsung.com>
2026-06-22  2:41 ` [PATCH v3] nvme-pci: fix CMB mapping when CMBSZ Size field is zero kangfenglong
2026-06-22  2:47   ` sashiko-bot
2026-09-02  7:45   ` Kangfenglong
2026-09-03  3:53   ` Kanchan Joshi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox