netdev - Re: [PATCH net-next 1/3] pds_core: add simple AER handler

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]

Message-ID: <aJL_VN0pvC9slprc@wunner.de>
Date: Wed, 6 Aug 2025 09:08:04 +0200
From: Lukas Wunner <lukas@...ner.de>
To: Brett Creeley <bcreeley@....com>
Cc: Shannon Nelson <shannon.nelson@....com>, netdev@...r.kernel.org,
	davem@...emloft.net, kuba@...nel.org, edumazet@...gle.com,
	pabeni@...hat.com, brett.creeley@....com, drivers@...sando.io
Subject: Re: [PATCH net-next 1/3] pds_core: add simple AER handler

On Wed, Aug 06, 2025 at 08:58:40AM +0200, Lukas Wunner wrote:
> My point is, you're calling pdsc_reset_prepare() but you're never calling
> pdsc_reset_done().  The former performs various teardown steps and calls
> pci_disable_device(), which disables MMIO access to the device.  Since
> you're never calling pdsc_reset_done(), you're not re-enabling MMIO
> access to the device and re-initializing the device.  So I'd expect
> any subsequent device access to fail.
> 
> Normally you'd have a ->slot_reset() callback which would call
> pdsc_reset_done().  Then the code would look sane.
> 
> Moreover, the AER driver in the PCI core performs an unconditional 
> Secondary Bus Reset on Fatal Errors (channel state pci_channel_io_frozen).
> You're performing an additional reset of the PCI Function in
> pdsc_pci_error_resume().  At least for Fatal Errors, this seems
> superfluous.

I note that the pensando ionic driver uses the same weird pattern,
see commit c3a910e1c47a ("ionic: fill out pci error handlers").

So I suppose it got copy-pasted from there to the pds_core driver.

Thanks,

Lukas