[PATCH v3] PCI: quirks: Fix out-of-bounds MMIO read in nvme_disable_and_flr()

Mohamad Raizudeen posted 1 patch 3 weeks, 1 day ago
drivers/pci/quirks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v3] PCI: quirks: Fix out-of-bounds MMIO read in nvme_disable_and_flr()
Posted by Mohamad Raizudeen 3 weeks, 1 day ago
In nvme_disable_and_flr(), the PCI bar is mapped using
NVME_REG_CC + sizeof(cfg) which is (0x14 + 4 = 0x18 bytes)

However, the function later reads the controller status from
NVME_REG_CSTS - offset 0x1C, which is outside the mapped 0x18 byte
boundary. This violates the API contract.

Fix this by increasing the mapping size to include NVME_REG_CSTS. Also
change sizeof(cfg) to sizeof(u32) to make the intent clearer.

Fixes: ffb0863426eb9 ("PCI: Disable Samsung SM961/PM961 NVMe before FLR")
Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
---
Changes in v3:
- Changed sizeof(cfg) to sizeof(u32) to decouple it from the mapping
  size as suggested by Alex Williamson.
- Updated the commit message to use "violates the API contract" instead
  of "undefined behavior" to avoid overstating the risk.

Link for v1: https://lore.kernel.org/all/20260817092448.4395-1-raizudeen.kerneldev@gmail.com/T/
Link for v2: https://lore.kernel.org/all/20260903040049.5460-1-raizudeen.kerneldev@gmail.com/T/

 drivers/pci/quirks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index b09f27f7846f..06436a96db30 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4090,7 +4090,7 @@ static int nvme_disable_and_flr(struct pci_dev *dev, bool probe)
 	if (probe)
 		return 0;
 
-	bar = pci_iomap(dev, 0, NVME_REG_CC + sizeof(cfg));
+	bar = pci_iomap(dev, 0, NVME_REG_CSTS + sizeof(u32));
 	if (!bar)
 		return -ENOTTY;
 
-- 
2.53.0
Re: [PATCH v3] PCI: quirks: Fix out-of-bounds MMIO read in nvme_disable_and_flr()
Posted by Alex Williamson 3 weeks, 1 day ago
On Thu,  3 Sep 2026 22:16:15 +0530
Mohamad Raizudeen <raizudeen.kerneldev@gmail.com> wrote:

> In nvme_disable_and_flr(), the PCI bar is mapped using
> NVME_REG_CC + sizeof(cfg) which is (0x14 + 4 = 0x18 bytes)
> 
> However, the function later reads the controller status from
> NVME_REG_CSTS - offset 0x1C, which is outside the mapped 0x18 byte
> boundary. This violates the API contract.
> 
> Fix this by increasing the mapping size to include NVME_REG_CSTS. Also
> change sizeof(cfg) to sizeof(u32) to make the intent clearer.
> 
> Fixes: ffb0863426eb9 ("PCI: Disable Samsung SM961/PM961 NVMe before FLR")
> Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
> ---
> Changes in v3:
> - Changed sizeof(cfg) to sizeof(u32) to decouple it from the mapping
>   size as suggested by Alex Williamson.
> - Updated the commit message to use "violates the API contract" instead
>   of "undefined behavior" to avoid overstating the risk.
> 
> Link for v1: https://lore.kernel.org/all/20260817092448.4395-1-raizudeen.kerneldev@gmail.com/T/
> Link for v2: https://lore.kernel.org/all/20260903040049.5460-1-raizudeen.kerneldev@gmail.com/T/
> 
>  drivers/pci/quirks.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index b09f27f7846f..06436a96db30 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4090,7 +4090,7 @@ static int nvme_disable_and_flr(struct pci_dev *dev, bool probe)
>  	if (probe)
>  		return 0;
>  
> -	bar = pci_iomap(dev, 0, NVME_REG_CC + sizeof(cfg));
> +	bar = pci_iomap(dev, 0, NVME_REG_CSTS + sizeof(u32));
>  	if (!bar)
>  		return -ENOTTY;
>  

Reviewed-by: Alex Williamson <alex@shazbot.org>

Thanks!