[PATCH] net: sungem: disable PCI device on remove

[PATCH] net: sungem: disable PCI device on remove

[Guangshuo Li]

gem_init_one() enables the PCI device with pci_enable_device().

The probe failure path balances this operation with
pci_disable_device(). However, after a successful probe,
gem_remove_one() releases the PCI resources and netdev without
disabling the PCI device.

Call pci_disable_device() during removal to balance the successful
pci_enable_device() call.

This issue was found by manual code inspection.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/net/ethernet/sun/sungem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/sun/sungem.c b/drivers/net/ethernet/sun/sungem.c
index dc5638d105db..6abf4d292802 100644
--- a/drivers/net/ethernet/sun/sungem.c
+++ b/drivers/net/ethernet/sun/sungem.c
@@ -2806,6 +2806,7 @@ static void gem_remove_one(struct pci_dev *pdev)
 		iounmap(gp->regs);
 		pci_release_regions(pdev);
 		free_netdev(dev);
+		pci_disable_device(pdev);
 	}
 }
 
-- 
2.43.0

RE: [PATCH] net: sungem: disable PCI device on remove

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Guangshuo Li <lgs201920130244@gmail.com>
> Sent: Wednesday, September 16, 2026 7:59 AM
> To: Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Petr
> Machata <petrm@nvidia.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; Keller, Jacob E
> <jacob.e.keller@intel.com>; Jijie Shao <shaojijie@huawei.com>; Uwe
> Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>; Ruoyu
> Wang <ruoyuw560@gmail.com>; Guangshuo Li <lgs201920130244@gmail.com>;
> Jakub Raczynski <j.raczynski@samsung.com>; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Cc: stable@vger.kernel.org
> Subject: [PATCH] net: sungem: disable PCI device on remove
> 
> gem_init_one() enables the PCI device with pci_enable_device().
> 
> The probe failure path balances this operation with
> pci_disable_device(). However, after a successful probe,
> gem_remove_one() releases the PCI resources and netdev without
> disabling the PCI device.
> 
> Call pci_disable_device() during removal to balance the successful
> pci_enable_device() call.
> 
> This issue was found by manual code inspection.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/net/ethernet/sun/sungem.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/sun/sungem.c
> b/drivers/net/ethernet/sun/sungem.c
> index dc5638d105db..6abf4d292802 100644
> --- a/drivers/net/ethernet/sun/sungem.c
> +++ b/drivers/net/ethernet/sun/sungem.c
> @@ -2806,6 +2806,7 @@ static void gem_remove_one(struct pci_dev *pdev)
>  		iounmap(gp->regs);
>  		pci_release_regions(pdev);
>  		free_netdev(dev);
> +		pci_disable_device(pdev);
>  	}
>  }
> 
> --
> 2.43.0

The fix is correct, but please resend targeting the net tree, and document testing.
Please also clarify the practical impact supporting the stable backport request.

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

Re: [PATCH] net: sungem: disable PCI device on remove

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] The patch only balances the probe-time pci_enable_device(),
  but the driver takes a *second* PCI enable reference per interface-up…

--

net: sungem: disable PCI device on remove

The patch adds a pci_disable_device() call at the end of gem_remove_one()
so that the pci_enable_device() done in gem_init_one() is balanced after a
successful probe. Previously only the probe failure path disabled the
device.

> diff --git a/drivers/net/ethernet/sun/sungem.c b/drivers/net/ethernet/sun/sungem.c
> index dc5638d105dbb..6abf4d292802c 100644
> --- a/drivers/net/ethernet/sun/sungem.c
> +++ b/drivers/net/ethernet/sun/sungem.c
> @@ -2806,6 +2806,7 @@ static void gem_remove_one(struct pci_dev *pdev)
>  		iounmap(gp->regs);
>  		pci_release_regions(pdev);
>  		free_netdev(dev);
> +		pci_disable_device(pdev);
>  	}
>  }

[Severity: Medium]
Is one unconditional pci_disable_device() here enough to guarantee the
device is actually disabled at unbind?

sungem takes a second enable reference every time the interface is brought
up, in gem_open():

	/* Make sure PCI access and bus master are enabled */
	rc = pci_enable_device(gp->pdev);
	if (rc) {
		...
		gem_put_cell(gp);
		return -ENXIO;
	}
	return gem_do_start(dev);

If gem_do_start() fails, that reference does not appear to be released.
The request_irq() failure path there unwinds only the chip state and the
cell:

	rc = request_irq(gp->pdev->irq, gem_interrupt,
			 IRQF_SHARED, dev->name, (void *)dev);
	if (rc) {
		netdev_err(dev, "failed to request irq !\n");

		gem_reset(gp);
		gem_clean_rings(gp);
		gem_put_cell(gp);
		return rc;
	}

and gem_open() returns that error verbatim.

__dev_open() in net/core/dev.c does not call ndo_stop when ndo_open fails:

	if (!ret && ops->ndo_open)
		ret = ops->ndo_open(dev);

	netpoll_poll_enable(dev);

	if (ret)
		clear_bit(__LINK_STATE_START, &dev->state);

so gem_close(), which holds the matching pci_disable_device(gp->pdev), is
never reached and pdev->enable_cnt stays at 2 for the rest of the binding.

In that case, does the new call in gem_remove_one() only drop the count
from 2 to 1?  pci_disable_device() in drivers/pci/pci.c is reference
counted:

	if (atomic_dec_return(&dev->enable_cnt) != 0)
		return;

	pci_host_bridge_disable_device(dev);
	do_pci_disable_device(dev);
	dev->is_busmaster = 0;

which would leave memory decoding and bus mastering enabled on a device
with no driver bound, and no warning, since the dev_WARN_ONCE there only
fires for enable_cnt <= 0.

Should the gem_open()/gem_do_start() failure path also drop its enable
reference, for example a pci_disable_device() in the request_irq() error
block of gem_do_start(), or in gem_open() when gem_do_start() returns an
error?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916055913.2910349-1-lgs201920130244%40gmail.com