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

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but missing checks for PCI device disconnect (0xFFFFFFFF) in MMIO reads cause a 128-second kernel thread stall.
--

--- Patch [1]: [PATCH] PCI: quirks: Fix out-of-bounds MMIO read in nvme_disable_and_flr() ---
commit df33f68f6dbf217384a1bdde080cb5742ca0e99d
Author: Mohamad Raizudeen <[email protected]>

PCI: quirks: Fix out-of-bounds MMIO read in nvme_disable_and_flr()

This commit fixes an out-of-bounds MMIO read in nvme_disable_and_flr() by
increasing the mapped area size to include the NVME_REG_CSTS register.

> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index b09f27f7846fc..ed03892cc960e 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(cfg));
>  	if (!bar)
>  		return -ENOTTY;
>  
> 	pci_read_config_word(dev, PCI_COMMAND, &cmd);
> 	pci_write_config_word(dev, PCI_COMMAND, cmd | PCI_COMMAND_MEMORY);
> 
> 	cfg = readl(bar + NVME_REG_CC);
> 
> 	/* Disable controller if enabled */
> 	if (cfg & NVME_CC_ENABLE) {
> 		u32 cap = readl(bar + NVME_REG_CAP);
> 		unsigned long timeout;

[ ... ]

> 		timeout = ((NVME_CAP_TIMEOUT(cap) + 1) * HZ / 2) + jiffies;
> 
> 		for (;;) {
> 			u32 status = readl(bar + NVME_REG_CSTS);
> 
> 			/* Ready status becomes zero on disable complete */
> 			if (!(status & NVME_CSTS_RDY))
> 				break;
> 
> 			msleep(100);

[Severity: High]
This is a pre-existing issue, but does this function properly handle PCI
device disconnects?

If a PCIe device drops off the bus or encounters a fatal error, MMIO reads
such as readl(bar + NVME_REG_CC) will return 0xFFFFFFFF. Because 0xFFFFFFFF
passes the cfg & NVME_CC_ENABLE check, the code will erroneously enter this
block.

It will then read 0xFFFFFFFF from NVME_REG_CAP, resulting in a maximum
timeout of 128 seconds. The loop continuously reads 0xFFFFFFFF for
NVME_REG_CSTS, which fails the NVME_CSTS_RDY check, causing the kernel
thread to sleep for 100ms in a loop for the full 128 seconds.

Could this cause system management threads like VFIO teardown or PCI hotplug
workers to stall?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1