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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A 64-bit shift overflow in `nvme_cmb_size_unit()` causes undefined behavior and integer overflow, allowing a malicious device or surprise removal to bypass the patch's zero-size check.
--

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

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

This commit restores a check for a zero SZ field to verify the controller
actually has a CMB before proceeding with mapping. This prevents size
from defaulting to the entire remaining BAR and triggering memory
hotplug alignment warnings.

> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index e6267a6aa3801..14f0f141df5bb 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1838,6 +1838,8 @@ 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);

[Severity: High]
This is a pre-existing issue, but could this multiplication still silently
overflow to zero and bypass the new zero-size check?

If a device advertises a Size Units (SZU) of 12 and a Size (SZ) of 16,
nvme_cmb_size_unit() calculates 1ULL << 60. The multiplication here
then becomes (1ULL << 60) * 16, which mathematically equals 1ULL << 64.
This wraps around to exactly 0 in a 64-bit unsigned integer. Since SZ is 16,
the new nvme_cmb_size(dev) check is passed, but size still becomes 0,
potentially triggering the same pci_p2pdma_add_resource() issue this
patch aims to fix.

Additionally, if the PCIe device is surprise-removed, readl() returns
0xFFFFFFFF, resulting in SZU = 15. The shift calculation in
nvme_cmb_size_unit():

    return 1ULL << (12 + 4 * szu);

evaluates to 1ULL << 72. This is undefined behavior and can result in an
unaligned size being passed down.

Does this calculation need further bounds checking or validation to handle
integer overflows and surprise removals?

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