Re: [PATCH v3] ata: ahci: fail probe if BAR too small for claimed ports

[Damien Le Moal <dlemoal@kernel.org>]

On 4/27/26 2:37 PM, dayou5941@163.com wrote:
> From: liyouhong <liyouhong@kylinos.cn>
> 
> When an AHCI controller is disabled in BIOS, its HOST_CAP register may
> contain invalid values (e.g., 0xFFFFFFFF) indicating an impossibly large
> number of ports. If CAP.NP claims more ports than can physically fit
> within the mapped BAR region, accessing port registers beyond the BAR
> boundary causes a kernel panic.
> 
> Add validation in ahci_init_one() to check that the BAR size is
> sufficient for the number of ports claimed in CAP.NP. The check
> calculates the required MMIO size as:
> 
>   required_size = 0x100 (global registers) + max_ports * 0x80
> 
> If required_size exceeds the actual BAR size, the probe fails with
> -ENODEV, preventing the panic and providing a clear error message.
> 
> This solution follows the suggestion by Damien Le Moal and Niklas Cassel
> to detect and reject obviously broken controller configurations early.
> 
> Reported-by: liyouhong <liyouhong@kylinos.cn>
> Suggested-by: Damien Le Moal <dlemoal@kernel.org>
> Suggested-by: Niklas Cassel <cassel@kernel.org>
> Signed-off-by: liyouhong <liyouhong@kylinos.cn>
> ---
> v3:
> - Fix patch format: add "---" separator and move changelog to correct location
> - Change dev_err to dev_warn as suggested

Note for next time: please keep *all* versions notes in the changelog. Do not
keep only the description for the last changes.

> 
> ---
>  drivers/ata/ahci.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 1d73a53370cf..e5eff7e3d66b 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1888,6 +1888,23 @@ static ssize_t remapped_nvme_show(struct device *dev,
>  
>  static DEVICE_ATTR_RO(remapped_nvme);
>  
> +static int ahci_validate_bar_size(struct pci_dev *pdev, void __iomem *mmio)
> +{
> +	u32 cap = readl(mmio + HOST_CAP);
> +	unsigned int max_ports = ahci_nr_ports(cap);
> +	u32 last_port_end = 0x100 + (max_ports * 0x80);
> +	resource_size_t bar_size = pci_resource_len(pdev, AHCI_PCI_BAR_STANDARD);

Long line. Please break it after the "=":

	resource_size_t bar_size =
		pci_resource_len(pdev, AHCI_PCI_BAR_STANDARD);

> +
> +	if (last_port_end > bar_size) {
> +		dev_warn(&pdev->dev,
> +			 "BAR5 too small for %u ports (last port ends at %u, BAR %llu)\n",
> +			 max_ports, last_port_end, (unsigned long long)bar_size);

Here too, long line. Break it after last_port_end.

			max_ports, last_port_end,
			(unsigned long long)bar_size);


With that fixed, looks OK to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research