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

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

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