linux-kernel - Re: [PATCH net-next v3 2/3] net: stmmac: Add glue driver for Motorcomm YT6801 ethernet controller

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <7e024db2557a4d5822a0dd409ae678d10d815d9c.camel@mailbox.org>
Date: Mon, 08 Dec 2025 11:47:23 +0100
From: Philipp Stanner <phasta@...lbox.org>
To: "Russell King (Oracle)" <linux@...linux.org.uk>, phasta@...nel.org
Cc: Bjorn Helgaas <helgaas@...nel.org>, Thomas Gleixner
 <tglx@...utronix.de>,  Yao Zi <ziyao@...root.org>, Bjorn Helgaas
 <bhelgaas@...gle.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David S.
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Frank
 <Frank.Sae@...or-comm.com>, Heiner Kallweit <hkallweit1@...il.com>,
 Vladimir Oltean <vladimir.oltean@....com>, Choong Yong Liang
 <yong.liang.choong@...ux.intel.com>, Chen-Yu Tsai <wens@...e.org>, Jisheng
 Zhang <jszhang@...nel.org>, Furong Xu <0x1207@...il.com>, 
 linux-kernel@...r.kernel.org, netdev@...r.kernel.org, Mingcong Bai
 <jeffbai@...c.io>, Kexy Biscuit <kexybiscuit@...c.io>, Runhua He
 <hua@...c.io>,  Xi Ruoyao <xry111@...111.site>
Subject: Re: [PATCH net-next v3 2/3] net: stmmac: Add glue driver for
 Motorcomm YT6801 ethernet controller

On Mon, 2025-12-08 at 10:15 +0000, Russell King (Oracle) wrote:
> On Mon, Dec 08, 2025 at 10:54:36AM +0100, Philipp Stanner wrote:
> > The bad news is that it's not super trivial to remove. I looked into it
> > about two times and decided I can't invest that time currently. You
> > need to go over all drivers again to see who uses pcim_enable_device(),
> > then add free_irq_vecs() for them all and so on…
> 
> So that I can confirm, you're saying that all drivers that call
> pci_alloc_irq_vectors() should call pci_free_irq_vectors() in their
> ->remove() method and not rely on the devres behaviour that
> pcim_enable_device() will permit.

"permit" is kind of a generous word. This behavior is dangerous and
there were bugs because of that in the past, because it confused
programmers. See:

f00059b4c1b0 drm/vboxvideo: fix mapping leaks

pcim_enable_device() used to switch all sorts of functions into managed
mode. As far as I could figure out through git, back in 2009 it was
intended that ALL pci functions are switched into managed mode that
way. That's also how it was documented.

The ecosystem then fractured, however. Some functions were always
managed (pcim_), some never, and some sometimes.

I removed all "sometimes managed" functions since 2024. The last
remainder is MSI.

If we want to remove that, we need to:
   1. Find all drivers that rely on pci_free_irq_vectors() being run
      automatically. IOW those that use pcim_enable_device() + wrappers
      around pci_setup_msi_context().
   2. Port those drivers to do the free_irq_vecs manually, if it's not
      a problem if it's called twice. If that were a problem, those
      drivers would also need to replace pcim_enable_device() with
      pci_enable_device().
   3. Once all drivers are ported, remove the devres code from msi.c
   4. Do associated cleanup work in PCI.

> 
> In terms of whether it's safe to call this twice, pci_free_irq_vectors()
> calls pci_disable_msix() and pci_disable_msi().
> 
> pci_disable_msix() checks:
> 
>         if (!pci_msi_enabled() || !dev || !dev->msix_enabled)
>                 return;
> 
> which will set dev->msix_enabled to 0 via pci_msix_shutdown().
> 
> pci_disable_msi() does a similar check:
> 
>         if (!pci_msi_enabled() || !dev || !dev->msi_enabled)
>                 return;
> 
> and similarly pci_msi_shutdown() sets dev->msi_enabled to 0.
> 
> So my conclusion is it's safe to call pci_free_irq_vectors() twice for
> the same device.
> 

Hm. Looks good.

P.