[PATCH v2] tg3: clean up PHYLIB resources on probe failure

Myeonghun Pak posted 1 patch 1 week, 3 days ago
There is a newer version of this series
drivers/net/ethernet/broadcom/tg3.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH v2] tg3: clean up PHYLIB resources on probe failure
Posted by Myeonghun Pak 1 week, 3 days ago
tg3_get_invariants() can register an MDIO bus and connect a PHY for
USE_PHYLIB devices. If tg3_init_one() later fails, its common error path
releases the mappings and netdev without undoing those PHYLIB resources.

Disconnect the PHY and unregister the MDIO bus before the remaining
teardown. The existing IS_CONNECTED and MDIOBUS_INITED flags make both
helpers safe when initialization only completed partially.

This issue was identified during our ongoing static-analysis research while
reviewing kernel code.

Fixes: 158d7abdae85 ("tg3: Add mdio bus registration")
Assisted-by: OpenAI:GPT-5.6
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
Changes in v2:
- Remove the redundant USE_PHYLIB guard; both cleanup helpers already
  check their initialization flags, as discussed with Andrew Lunn.

 drivers/net/ethernet/broadcom/tg3.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 73a4b569b..91c7ad997 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -18047,6 +18047,9 @@ static int tg3_init_one(struct pci_dev *pdev,
 	return 0;
 
 err_out_apeunmap:
+	tg3_phy_fini(tp);
+	tg3_mdio_fini(tp);
+
 	if (tp->aperegs) {
 		iounmap(tp->aperegs);
 		tp->aperegs = NULL;
-- 
2.47.1
Re: [PATCH v2] tg3: clean up PHYLIB resources on probe failure
Posted by Andrew Lunn 1 week, 2 days ago
On Mon, Sep 14, 2026 at 06:22:46PM -0400, Myeonghun Pak wrote:
> tg3_get_invariants() can register an MDIO bus and connect a PHY for
> USE_PHYLIB devices. If tg3_init_one() later fails, its common error path
> releases the mappings and netdev without undoing those PHYLIB resources.
> 
> Disconnect the PHY and unregister the MDIO bus before the remaining
> teardown. The existing IS_CONNECTED and MDIOBUS_INITED flags make both
> helpers safe when initialization only completed partially.
> 
> This issue was identified during our ongoing static-analysis research while
> reviewing kernel code.
> 
> Fixes: 158d7abdae85 ("tg3: Add mdio bus registration")
> Assisted-by: OpenAI:GPT-5.6
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
> Changes in v2:
> - Remove the redundant USE_PHYLIB guard; both cleanup helpers already
>   check their initialization flags, as discussed with Andrew Lunn.

That is not exactly what i said. I pointed out that tg3_mdio_init() is
not guarded by USE_PHYLIB. tg3_phy_init() is guarded by
USE_PHYLIB. Generally, you keep code symmetrical, the conditions you
apply to init should be also applied to fini. However, the existing
code is not great, tg3_remove_one() is not symmetrical. However, there
is no reason to make it worse.


    Andrew

---
pw-bot: cr
Re: [PATCH v2] tg3: clean up PHYLIB resources on probe failure
Posted by Myeonghun Pak 1 week ago
Hello Andrew,

Sorry for the misunderstanding. I will restore the guard only around
tg3_phy_fini() in v3.

Best regards,
Myeonghun Pak

2026년 9월 15일 (화) 오전 11:37, Andrew Lunn <andrew@lunn.ch>님이 작성:
>
> On Mon, Sep 14, 2026 at 06:22:46PM -0400, Myeonghun Pak wrote:
> > tg3_get_invariants() can register an MDIO bus and connect a PHY for
> > USE_PHYLIB devices. If tg3_init_one() later fails, its common error path
> > releases the mappings and netdev without undoing those PHYLIB resources.
> >
> > Disconnect the PHY and unregister the MDIO bus before the remaining
> > teardown. The existing IS_CONNECTED and MDIOBUS_INITED flags make both
> > helpers safe when initialization only completed partially.
> >
> > This issue was identified during our ongoing static-analysis research while
> > reviewing kernel code.
> >
> > Fixes: 158d7abdae85 ("tg3: Add mdio bus registration")
> > Assisted-by: OpenAI:GPT-5.6
> > Co-developed-by: Ijae Kim <ae878000@gmail.com>
> > Signed-off-by: Ijae Kim <ae878000@gmail.com>
> > Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> > ---
> > Changes in v2:
> > - Remove the redundant USE_PHYLIB guard; both cleanup helpers already
> >   check their initialization flags, as discussed with Andrew Lunn.
>
> That is not exactly what i said. I pointed out that tg3_mdio_init() is
> not guarded by USE_PHYLIB. tg3_phy_init() is guarded by
> USE_PHYLIB. Generally, you keep code symmetrical, the conditions you
> apply to init should be also applied to fini. However, the existing
> code is not great, tg3_remove_one() is not symmetrical. However, there
> is no reason to make it worse.
>
>
>     Andrew
>
> ---
> pw-bot: cr
>