[PATCH v2] nvme-pci: fix CMB mapping when CMBSZ Size field is zero

[PATCH v2] nvme-pci: fix CMB mapping when CMBSZ Size field is zero

[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. 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 | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 8438c904ec49..5c7b1e757e4d 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);
 }
 
@@ -2472,9 +2475,12 @@ 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);
+	if (check_mul_overflow(nvme_cmb_size_unit(dev), 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

Re: [PATCH v2] nvme-pci: fix CMB mapping when CMBSZ Size field is zero

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning 0 from `nvme_cmb_size_unit()` to reject invalid SZU values evaluates to a size of 0, bypassing the rejection and triggering the exact bug the patch attempts to fix.
--

commit efcd71708003c14038a1fdc79b6233cc4b6a8192
Author: kangfenglong <kangfenglong@huawei.com>

nvme-pci: fix CMB mapping when CMBSZ Size field is zero

This commit restores the check for a zero SZ field in the CMBSZ register,
limits the Size Units (SZU) to valid values, and adds an overflow check
when calculating the CMB size to avoid mapping errors.

> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index e6267a6aa3801..2794298aa9e66 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1814,6 +1814,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;
> +

[Severity: High]
If this boundary check returns 0 to reject an invalid SZU value, will this
0 value be used in subsequent calculations rather than triggering an abort?

>  	return 1ULL << (12 + 4 * szu);
>  }
>  
> @@ -1838,9 +1841,12 @@ 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);
> +	if (check_mul_overflow(nvme_cmb_size_unit(dev), 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);

[Severity: High]
If nvme_cmb_size_unit() returned 0, won't check_mul_overflow() compute
0 * sz = 0, return false (no overflow), and set size = 0 without returning
early?

Because size evaluates to 0, execution continues, and later in the function
pci_p2pdma_add_resource() will be called with size=0 and offset=0.

As noted in the commit message, calling pci_p2pdma_add_resource() with size=0
causes the P2PDMA subsystem to default to mapping the entire remaining BAR.
Does this defeat the SZU boundary check above and re-trigger the exact memory
mapping warning this patch is attempting to fix?

Should nvme_map_cmb() explicitly check if nvme_cmb_size_unit() == 0 or if the
resulting size == 0 before continuing?

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

答复: [PATCH v2] nvme-pci: fix CMB mapping when CMBSZ Size field is zero

[Kangfenglong]

Thanks for review!

Your feedback is reasonable, and I will fix it in the next patch！ 

-----邮件原件-----
发件人: sashiko-bot@kernel.org [mailto:sashiko-bot@kernel.org] 
发送时间: 2026年6月22日 9:29
收件人: Kangfenglong <kangfenglong@huawei.com>
抄送: linux-pci@vger.kernel.org
主题: Re: [PATCH v2] nvme-pci: fix CMB mapping when CMBSZ Size field is zero

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning 0 from `nvme_cmb_size_unit()` to reject invalid SZU values evaluates to a size of 0, bypassing the rejection and triggering the exact bug the patch attempts to fix.
--

commit efcd71708003c14038a1fdc79b6233cc4b6a8192
Author: kangfenglong <kangfenglong@huawei.com>

nvme-pci: fix CMB mapping when CMBSZ Size field is zero

This commit restores the check for a zero SZ field in the CMBSZ register, limits the Size Units (SZU) to valid values, and adds an overflow check when calculating the CMB size to avoid mapping errors.

> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 
> e6267a6aa3801..2794298aa9e66 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1814,6 +1814,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;
> +

[Severity: High]
If this boundary check returns 0 to reject an invalid SZU value, will this
0 value be used in subsequent calculations rather than triggering an abort?

>  	return 1ULL << (12 + 4 * szu);
>  }
>  
> @@ -1838,9 +1841,12 @@ 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);
> +	if (check_mul_overflow(nvme_cmb_size_unit(dev), 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);

[Severity: High]
If nvme_cmb_size_unit() returned 0, won't check_mul_overflow() compute
0 * sz = 0, return false (no overflow), and set size = 0 without returning early?

Because size evaluates to 0, execution continues, and later in the function
pci_p2pdma_add_resource() will be called with size=0 and offset=0.

As noted in the commit message, calling pci_p2pdma_add_resource() with size=0 causes the P2PDMA subsystem to default to mapping the entire remaining BAR.
Does this defeat the SZU boundary check above and re-trigger the exact memory mapping warning this patch is attempting to fix?

Should nvme_map_cmb() explicitly check if nvme_cmb_size_unit() == 0 or if the resulting size == 0 before continuing?

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