Re: [PATCH v4 05/13] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer

On 8/24/26 4:05 PM, Jonathan Cameron wrote:
> On Mon, 24 Aug 2026 10:49:28 -0700
> Dave Jiang <[email protected]> wrote:
> 
>> extlog_print_pcie() casts pcie_err->aer_info straight to struct
>> aer_capability_regs *. That struct embeds struct pcie_tlp_log, whose
>> software-only header_len and flit fields sit at offset 84 - inside the
>> 96-byte aer_info buffer - so the cast fills them with raw firmware bytes.
>> pcie_print_tlp_log() uses both to bound a loop over dw[], and a large
>> header_len walks past the end of the array.
>>
>> Copy aer_info into a zeroed local struct aer_capability_regs and clear
>> header_len and flit before passing it on.
> 
> The existing code is very odd. It might be nice to clean it up more
> generally so that we can handle the rest of the aer_info buffer
> via a definition that matches the hardware spec. 
> 
> For now I'd be tempted to just copy the bit of the structure that
> is matching the spec - side effect being the rest ends up as zero
> including the two things you clear.
> 
> So just copy to first 4 dw of the header log. 

Ok will do.

> 
> Jonathan
> 
> 
> 
>>
>> Reported-by: [email protected]
>> Closes: https://lore.kernel.org/linux-cxl/[email protected]/
>> Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
>> Reviewed-by: Alison Schofield <[email protected]>
>> Reviewed-by: Shuai Xue <[email protected]>
>> Assisted-by: Claude:claude-sonnet-4-6
>> Signed-off-by: Dave Jiang <[email protected]>
>> ---
>> v4:
>> - Dropped the now-pointless aer pointer alias.
>> ---
>>  drivers/acpi/acpi_extlog.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
>> index f6e3da4e13e7..6d5532ec0920 100644
>> --- a/drivers/acpi/acpi_extlog.c
>> +++ b/drivers/acpi/acpi_extlog.c
>> @@ -137,7 +137,7 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>>  			      int severity)
>>  {
>>  #ifdef ACPI_APEI_PCIEAER
>> -	struct aer_capability_regs *aer;
>> +	struct aer_capability_regs aer_regs = {};
>>  	struct pci_dev *pdev;
>>  	unsigned int devfn;
>>  	unsigned int bus;
>> @@ -149,7 +149,11 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>>  		return;
>>  
>>  	aer_severity = cper_severity_to_aer(severity);
>> -	aer = (struct aer_capability_regs *)pcie_err->aer_info;
>> +
>> +	memcpy(&aer_regs, pcie_err->aer_info, sizeof(pcie_err->aer_info));
>> +	aer_regs.header_log.header_len = 0;
>> +	aer_regs.header_log.flit = false;
>> +
>>  	domain = pcie_err->device_id.segment;
>>  	bus = pcie_err->device_id.bus;
>>  	devfn = PCI_DEVFN(pcie_err->device_id.device,
>> @@ -158,7 +162,7 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>>  	if (!pdev)
>>  		return;
>>  
>> -	pci_print_aer(pdev, aer_severity, aer);
>> +	pci_print_aer(pdev, aer_severity, &aer_regs);
>>  	pci_dev_put(pdev);
>>  #endif
>>  }
>