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

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

[dayou5941]

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.

v2:
- Complete rewrite based on community feedback
- Move check from libahci.c to ahci.c
- Fail probe early instead of attempting to work around invalid state
- Implement BAR size validation as suggested

Reported-by: liyouhong <liyouhong@kylinos.cn>
Suggested-by: Damien Le Moal <dlemoal@kernel.org>
Suggested-by: Niklas Cassel <niklas.soderlund@corigine.com>
Signed-off-by: liyouhong <liyouhong@kylinos.cn>

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 1d73a53370cf..09026ea12cde 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);
+
+	if (last_port_end > bar_size) {
+		dev_err(&pdev->dev,
+			"AHCI: BAR5 too small for %u ports (last port ends at %u, BAR %llu)\n",
+			max_ports, last_port_end, (unsigned long long)bar_size);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	unsigned int board_id = ent->driver_data;
@@ -1988,6 +2005,10 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (!hpriv->mmio)
 		return -ENOMEM;
 
+	rc = ahci_validate_bar_size(pdev, hpriv->mmio);
+	if (rc)
+		return rc;
+
 	/* detect remapped nvme devices */
 	ahci_remap_check(pdev, ahci_pci_bar, hpriv);
 
-- 
2.25.1

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

[Damien Le Moal]

On 4/25/26 3:55 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.
> 
> v2:
> - Complete rewrite based on community feedback
> - Move check from libahci.c to ahci.c
> - Fail probe early instead of attempting to work around invalid state
> - Implement BAR size validation as suggested
> 
> Reported-by: liyouhong <liyouhong@kylinos.cn>
> Suggested-by: Damien Le Moal <dlemoal@kernel.org>
> Suggested-by: Niklas Cassel <niklas.soderlund@corigine.com>
> Signed-off-by: liyouhong <liyouhong@kylinos.cn>

Your patch is not formatted correctly: the changelog should not be part of the
commit message but should come between the "---" separator after the tags and
the first "diff" line of the patch proper. The "---" separator is missing here
too. Did you generate this patch with "git format-patch" ?

> 
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 1d73a53370cf..09026ea12cde 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);
> +
> +	if (last_port_end > bar_size) {

It may be good to check also that max_ports is not zero here.

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

Please make this a warning (dev_warn()).

> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
>  static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  {
>  	unsigned int board_id = ent->driver_data;
> @@ -1988,6 +2005,10 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (!hpriv->mmio)
>  		return -ENOMEM;
>  
> +	rc = ahci_validate_bar_size(pdev, hpriv->mmio);
> +	if (rc)
> +		return rc;
> +
>  	/* detect remapped nvme devices */
>  	ahci_remap_check(pdev, ahci_pci_bar, hpriv);
>  


-- 
Damien Le Moal
Western Digital Research

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

[李佑鸿]

At 2026-04-27 07:46:12, "Damien Le Moal" <dlemoal@kernel.org> wrote:
>On 4/25/26 3:55 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.
>> 
>> v2:
>> - Complete rewrite based on community feedback
>> - Move check from libahci.c to ahci.c
>> - Fail probe early instead of attempting to work around invalid state
>> - Implement BAR size validation as suggested
>> 
>> Reported-by: liyouhong <liyouhong@kylinos.cn>
>> Suggested-by: Damien Le Moal <dlemoal@kernel.org>
>> Suggested-by: Niklas Cassel <niklas.soderlund@corigine.com>
>> Signed-off-by: liyouhong <liyouhong@kylinos.cn>
>
>Your patch is not formatted correctly: the changelog should not be part of the
>commit message but should come between the "---" separator after the tags and
>the first "diff" line of the patch proper. The "---" separator is missing here
>too. Did you generate this patch with "git format-patch" ?
>
>> 
>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
>> index 1d73a53370cf..09026ea12cde 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);
>> +
>> +	if (last_port_end > bar_size) {
>
>It may be good to check also that max_ports is not zero here.
>
>> +		dev_err(&pdev->dev,
>> +			"AHCI: BAR5 too small for %u ports (last port ends at %u, BAR %llu)\n",
>> +			max_ports, last_port_end, (unsigned long long)bar_size);
>

>Please make this a warning (dev_warn()).


Thank you for the review. Here's v3 with the requested changes:


Fixed patch format (added "---" separator, moved changelog).
Changed dev_err to dev_warn.


Regarding the max_ports check: The ahci_nr_ports() helper function is defined as:
static inline unsigned int ahci_nr_ports(u32 cap)
{
    return (cap & 0x1f) + 1;  // Note the +1
}


This function always returns a value ≥ 1, so checking for 0 is not needed.
The hardware could theoretically report CAP.NP=0, but that indicates 1 port
, not 0 ports.


Please let me know if you'd like additional validation.



Best regards,
liyouhong

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

[Damien Le Moal]

On 4/27/26 11:07 AM, 李佑鸿 wrote:
> 
> 
> 
> 
> At 2026-04-27 07:46:12, "Damien Le Moal" <dlemoal@kernel.org> wrote:
>> On 4/25/26 3:55 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.
>>>
>>> v2:
>>> - Complete rewrite based on community feedback
>>> - Move check from libahci.c to ahci.c
>>> - Fail probe early instead of attempting to work around invalid state
>>> - Implement BAR size validation as suggested
>>>
>>> Reported-by: liyouhong <liyouhong@kylinos.cn>
>>> Suggested-by: Damien Le Moal <dlemoal@kernel.org>
>>> Suggested-by: Niklas Cassel <niklas.soderlund@corigine.com>
>>> Signed-off-by: liyouhong <liyouhong@kylinos.cn>
>>
>> Your patch is not formatted correctly: the changelog should not be part of the
>> commit message but should come between the "---" separator after the tags and
>> the first "diff" line of the patch proper. The "---" separator is missing here
>> too. Did you generate this patch with "git format-patch" ?
>>
>>>
>>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
>>> index 1d73a53370cf..09026ea12cde 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);
>>> +
>>> +	if (last_port_end > bar_size) {
>>
>> It may be good to check also that max_ports is not zero here.
>>
>>> +		dev_err(&pdev->dev,
>>> +			"AHCI: BAR5 too small for %u ports (last port ends at %u, BAR %llu)\n",
>>> +			max_ports, last_port_end, (unsigned long long)bar_size);
>>
> 
>> Please make this a warning (dev_warn()).
> 
> 
> Thank you for the review. Here's v3 with the requested changes:

What v3 patch ? I did not receive anything.


-- 
Damien Le Moal
Western Digital Research