On Wed, May 21, 2025 at 09:52:18AM +0100, Jonathan Cameron wrote:
> On Tue, 20 May 2025 16:50:18 -0500
> Bjorn Helgaas <helg...@kernel.org> wrote:
>
> > From: Bjorn Helgaas <bhelg...@google.com>
> >
> > Previously the struct aer_err_info "info" was allocated on the stack
> > without being initialized, so it contained junk except for the fields we
> > explicitly set later.
> >
> > Initialize "info" at declaration so it starts as all zeros.
> >
> > Signed-off-by: Bjorn Helgaas <bhelg...@google.com>
> > Tested-by: Krzysztof Wilczyński <kwilczyn...@kernel.org>
> > Reviewed-by: Kuppuswamy Sathyanarayanan
> > <sathyanarayanan.kuppusw...@linux.intel.com>
>
> I chased this through a bit and looks like at least some unset fields would
> result in garbage prints. So maybe needs a fixes tag?
> info->tlp_header_valid is an easy one to follow as only set in some paths.
I don't see a current issue related to info->tlp_header_valid because
it's always cleared in the path from DPC:
dpc_process_error
dpc_get_aer_uncorrect_severity
info->severity = AER_FATAL or AER_NONFATAL
aer_get_device_error_info
info->status = 0;
info->tlp_header_valid = 0; # unconditional
pci_read_config_dword(PCI_ERR_UNCOR_STATUS, &info->status);
pci_read_config_dword(PCI_ERR_UNCOR_MASK, &info->mask);
info->first_error = PCI_ERR_CAP_FEP(...);
aer_print_error
However, only info->{severity,status,tlp_header_valid,mask,
first_error} are initialized before dpc_process_error() calls
aer_print_error(), and aer_print_error() uses info->{id,
error_dev_num}, which haven't been initialized.
AFAICT, this has been the case since
https://git.kernel.org/linus/8aefa9b0d910 ("PCI/DPC: Print AER status
in DPC event handling")
Here's aer_get_device_error_info() at that time:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pcie/aer.c?id=8aefa9b0d910#n867
and aer_print_error():
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pcie/aer.c?id=8aefa9b0d910#n530
At the time of 8aefa9b0d910, info->severity was also used without
initialization; this was fixed by 9f08a5d896ce ("PCI/DPC: Fix print
AER status in DPC event handling").
But I think the info->{id,error_dev_num} has been there since.
Anyway, I added:
Fixes: 8aefa9b0d910 ("PCI/DPC: Print AER status in DPC event handling")
> Otherwise absolutely makes sense.
> Reviewed-by: Jonathan Cameron <jonathan.came...@huawei.com>
> > ---
> > drivers/pci/pcie/dpc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> > index df42f15c9829..3daaf61c79c9 100644
> > --- a/drivers/pci/pcie/dpc.c
> > +++ b/drivers/pci/pcie/dpc.c
> > @@ -258,7 +258,7 @@ static int dpc_get_aer_uncorrect_severity(struct
> > pci_dev *dev,
> > void dpc_process_error(struct pci_dev *pdev)
> > {
> > u16 cap = pdev->dpc_cap, status, source, reason, ext_reason;
> > - struct aer_err_info info;
> > + struct aer_err_info info = {};
> >
> > pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
> > pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID, &source);
>
