linux-kernel - Re: [PATCH v1] PCI/AER: Handle Multi UnCorrectable/Correctable errors properly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 18 Mar 2022 13:28:16 -0700
From:   Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
To:     Bjorn Helgaas <helgaas@...nel.org>,
        Eric Badger <ebadger@...estorage.com>
Cc:     "Raj, Ashok" <ashok.raj@...el.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Russell Currey <ruscur@...sell.cc>,
        Oliver OHalloran <oohall@...il.com>,
        Kuppuswamy Sathyanarayanan <knsathya@...nel.org>,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v1] PCI/AER: Handle Multi UnCorrectable/Correctable errors
 properly



On 3/17/22 3:59 PM, Bjorn Helgaas wrote:
> Thanks for the additional details!
> 
> After this patch, I guess aer_irq() still reads 0x2
> (PCI_ERR_ROOT_MULTI_COR_RCV), but now it writes 0x2 back which clears
> PCI_ERR_ROOT_MULTI_COR_RCV.
> 
> In addition, aer_irq() will continue on to read PCI_ERR_ROOT_ERR_SRC,
> which probably contains either 0 or junk left over from being captured
> when PCI_ERR_ROOT_COR_RCV was set.
> 
> And aer_irq() will queue an e_src record with status ==
> PCI_ERR_ROOT_MULTI_COR_RCV.  But since PCI_ERR_ROOT_COR_RCV is not set
> in status, aer_isr_one_error() will do nothing, right?
> 
> That might not be*terrible*  and is definitely better than not being
> able to handle future interrupts.  But we basically threw away the
> information that multiple errors occurred, and we queued an e_src
> record that occupies space without being used for anything.

Yes, you are correct.  One other way to minimize this race window is to
clear the Root status register as soon as possible. Maybe we can move
it before source ID read as below.

--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1204,8 +1204,8 @@ static irqreturn_t aer_irq(int irq, void *context)
         if (!(e_src.status & AER_ERR_STATUS_MASK))
                 return IRQ_NONE;

-       pci_read_config_dword(rp, aer + PCI_ERR_ROOT_ERR_SRC, &e_src.id);
         pci_write_config_dword(rp, aer + PCI_ERR_ROOT_STATUS, 
e_src.status);
+       pci_read_config_dword(rp, aer + PCI_ERR_ROOT_ERR_SRC, &e_src.id)

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer