Re: [PATCH] r6040: Fix kmemleak in probe and remove

Re: [PATCH] r6040: Fix kmemleak in probe and remove

[Wei Yongjun]

On 2022/12/13 18:17, Li Zetao wrote:
> There is a memory leaks reported by kmemleak:
> 
>   unreferenced object 0xffff888116111000 (size 2048):
>     comm "modprobe", pid 817, jiffies 4294759745 (age 76.502s)
>     hex dump (first 32 bytes):
>       00 c4 0a 04 81 88 ff ff 08 10 11 16 81 88 ff ff  ................
>       08 10 11 16 81 88 ff ff 00 00 00 00 00 00 00 00  ................
>     backtrace:
>       [<ffffffff815bcd82>] kmalloc_trace+0x22/0x60
>       [<ffffffff827e20ee>] phy_device_create+0x4e/0x90
>       [<ffffffff827e6072>] get_phy_device+0xd2/0x220
>       [<ffffffff827e7844>] mdiobus_scan+0xa4/0x2e0
>       [<ffffffff827e8be2>] __mdiobus_register+0x482/0x8b0
>       [<ffffffffa01f5d24>] r6040_init_one+0x714/0xd2c [r6040]
>       ...
> 
> The problem occurs in probe process as follows:
>   r6040_init_one:
>     mdiobus_register
>       mdiobus_scan    <- alloc and register phy_device,
>                          the reference count of phy_device is 3
>     r6040_mii_probe
>       phy_connect     <- connect to the first phy_device,
>                          so the reference count of the first
>                          phy_device is 4, others are 3
>     register_netdev   <- fault inject succeeded, goto error handling path
> 
>     // error handling path
>     err_out_mdio_unregister:
>       mdiobus_unregister(lp->mii_bus);
>     err_out_mdio:
>       mdiobus_free(lp->mii_bus);    <- the reference count of the first
>                                        phy_device is 1, it is not released
>                                        and other phy_devices are released
>   // similarly, the remove process also has the same problem
> 
> The root cause is traced to the phy_device is not disconnected when
> removes one r6040 device in r6040_remove_one() or on error handling path
> after r6040_mii probed successfully. In r6040_mii_probe(), a net ethernet
> device is connected to the first PHY device of mii_bus, in order to
> notify the connected driver when the link status changes, which is the
> default behavior of the PHY infrastructure to handle everything.
> Therefore the phy_device should be disconnected when removes one r6040
> device or on error handling path.
> 
> Fix it by adding phy_disconnect() when removes one r6040 device or on
> error handling path after r6040_mii probed successfully.
> 
> Fixes: 3831861b4ad8 ("r6040: implement phylib")
> Signed-off-by: Li Zetao <lizetao1@huawei.com>
> ---
>  drivers/net/ethernet/rdc/r6040.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)


Please use [PATCH net] ... format in title.


> 
> diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
> index eecd52ed1ed2..95b682597da1 100644
> --- a/drivers/net/ethernet/rdc/r6040.c
> +++ b/drivers/net/ethernet/rdc/r6040.c
> @@ -1159,10 +1159,12 @@ static int r6040_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	err = register_netdev(dev);
>  	if (err) {
>  		dev_err(&pdev->dev, "Failed to register net device\n");
> -		goto err_out_mdio_unregister;
> +		goto err_out_r6040_mii_remove;

better to use something like 'err_out_phy_disconnect'

>  	}
>  	return 0;
>  
> +err_out_r6040_mii_remove:
> +	phy_disconnect(dev->phydev);
>  err_out_mdio_unregister:
>  	mdiobus_unregister(lp->mii_bus);
>  err_out_mdio:
> @@ -1186,6 +1188,7 @@ static void r6040_remove_one(struct pci_dev *pdev)
>  	struct r6040_private *lp = netdev_priv(dev);
>  
>  	unregister_netdev(dev);
> +	phy_disconnect(dev->phydev);
>  	mdiobus_unregister(lp->mii_bus);
>  	mdiobus_free(lp->mii_bus);
>  	netif_napi_del(&lp->napi);

[PATCH] r6040: Fix kmemleak in probe and remove

[Li Zetao]

There is a memory leaks reported by kmemleak:

  unreferenced object 0xffff888116111000 (size 2048):
    comm "modprobe", pid 817, jiffies 4294759745 (age 76.502s)
    hex dump (first 32 bytes):
      00 c4 0a 04 81 88 ff ff 08 10 11 16 81 88 ff ff  ................
      08 10 11 16 81 88 ff ff 00 00 00 00 00 00 00 00  ................
    backtrace:
      [<ffffffff815bcd82>] kmalloc_trace+0x22/0x60
      [<ffffffff827e20ee>] phy_device_create+0x4e/0x90
      [<ffffffff827e6072>] get_phy_device+0xd2/0x220
      [<ffffffff827e7844>] mdiobus_scan+0xa4/0x2e0
      [<ffffffff827e8be2>] __mdiobus_register+0x482/0x8b0
      [<ffffffffa01f5d24>] r6040_init_one+0x714/0xd2c [r6040]
      ...

The problem occurs in probe process as follows:
  r6040_init_one:
    mdiobus_register
      mdiobus_scan    <- alloc and register phy_device,
                         the reference count of phy_device is 3
    r6040_mii_probe
      phy_connect     <- connect to the first phy_device,
                         so the reference count of the first
                         phy_device is 4, others are 3
    register_netdev   <- fault inject succeeded, goto error handling path

    // error handling path
    err_out_mdio_unregister:
      mdiobus_unregister(lp->mii_bus);
    err_out_mdio:
      mdiobus_free(lp->mii_bus);    <- the reference count of the first
                                       phy_device is 1, it is not released
                                       and other phy_devices are released
  // similarly, the remove process also has the same problem

The root cause is traced to the phy_device is not disconnected when
removes one r6040 device in r6040_remove_one() or on error handling path
after r6040_mii probed successfully. In r6040_mii_probe(), a net ethernet
device is connected to the first PHY device of mii_bus, in order to
notify the connected driver when the link status changes, which is the
default behavior of the PHY infrastructure to handle everything.
Therefore the phy_device should be disconnected when removes one r6040
device or on error handling path.

Fix it by adding phy_disconnect() when removes one r6040 device or on
error handling path after r6040_mii probed successfully.

Fixes: 3831861b4ad8 ("r6040: implement phylib")
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
 drivers/net/ethernet/rdc/r6040.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index eecd52ed1ed2..95b682597da1 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -1159,10 +1159,12 @@ static int r6040_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	err = register_netdev(dev);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to register net device\n");
-		goto err_out_mdio_unregister;
+		goto err_out_r6040_mii_remove;
 	}
 	return 0;
 
+err_out_r6040_mii_remove:
+	phy_disconnect(dev->phydev);
 err_out_mdio_unregister:
 	mdiobus_unregister(lp->mii_bus);
 err_out_mdio:
@@ -1186,6 +1188,7 @@ static void r6040_remove_one(struct pci_dev *pdev)
 	struct r6040_private *lp = netdev_priv(dev);
 
 	unregister_netdev(dev);
+	phy_disconnect(dev->phydev);
 	mdiobus_unregister(lp->mii_bus);
 	mdiobus_free(lp->mii_bus);
 	netif_napi_del(&lp->napi);
-- 
2.31.1

[PATCH net v2] r6040: Fix kmemleak in probe and remove

[Li Zetao]

There is a memory leaks reported by kmemleak:

  unreferenced object 0xffff888116111000 (size 2048):
    comm "modprobe", pid 817, jiffies 4294759745 (age 76.502s)
    hex dump (first 32 bytes):
      00 c4 0a 04 81 88 ff ff 08 10 11 16 81 88 ff ff  ................
      08 10 11 16 81 88 ff ff 00 00 00 00 00 00 00 00  ................
    backtrace:
      [<ffffffff815bcd82>] kmalloc_trace+0x22/0x60
      [<ffffffff827e20ee>] phy_device_create+0x4e/0x90
      [<ffffffff827e6072>] get_phy_device+0xd2/0x220
      [<ffffffff827e7844>] mdiobus_scan+0xa4/0x2e0
      [<ffffffff827e8be2>] __mdiobus_register+0x482/0x8b0
      [<ffffffffa01f5d24>] r6040_init_one+0x714/0xd2c [r6040]
      ...

The problem occurs in probe process as follows:
  r6040_init_one:
    mdiobus_register
      mdiobus_scan    <- alloc and register phy_device,
                         the reference count of phy_device is 3
    r6040_mii_probe
      phy_connect     <- connect to the first phy_device,
                         so the reference count of the first
                         phy_device is 4, others are 3
    register_netdev   <- fault inject succeeded, goto error handling path

    // error handling path
    err_out_mdio_unregister:
      mdiobus_unregister(lp->mii_bus);
    err_out_mdio:
      mdiobus_free(lp->mii_bus);    <- the reference count of the first
                                       phy_device is 1, it is not released
                                       and other phy_devices are released
  // similarly, the remove process also has the same problem

The root cause is traced to the phy_device is not disconnected when
removes one r6040 device in r6040_remove_one() or on error handling path
after r6040_mii probed successfully. In r6040_mii_probe(), a net ethernet
device is connected to the first PHY device of mii_bus, in order to
notify the connected driver when the link status changes, which is the
default behavior of the PHY infrastructure to handle everything.
Therefore the phy_device should be disconnected when removes one r6040
device or on error handling path.

Fix it by adding phy_disconnect() when removes one r6040 device or on
error handling path after r6040_mii probed successfully.

Fixes: 3831861b4ad8 ("r6040: implement phylib")
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
v1 -> v2: change the subject prefix "PATCH" to "PATCH net" and change
the goto label name "err_out_r6040_mii_remove" to "err_out_phy_disconnect"

 drivers/net/ethernet/rdc/r6040.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index eecd52ed1ed2..f4d434c379e7 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -1159,10 +1159,12 @@ static int r6040_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	err = register_netdev(dev);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to register net device\n");
-		goto err_out_mdio_unregister;
+		goto err_out_phy_disconnect;
 	}
 	return 0;
 
+err_out_phy_disconnect:
+	phy_disconnect(dev->phydev);
 err_out_mdio_unregister:
 	mdiobus_unregister(lp->mii_bus);
 err_out_mdio:
@@ -1186,6 +1188,7 @@ static void r6040_remove_one(struct pci_dev *pdev)
 	struct r6040_private *lp = netdev_priv(dev);
 
 	unregister_netdev(dev);
+	phy_disconnect(dev->phydev);
 	mdiobus_unregister(lp->mii_bus);
 	mdiobus_free(lp->mii_bus);
 	netif_napi_del(&lp->napi);
-- 
2.31.1

Re: [PATCH net v2] r6040: Fix kmemleak in probe and remove

[Leon Romanovsky]

On Tue, Dec 13, 2022 at 08:56:14PM +0800, Li Zetao wrote:
> There is a memory leaks reported by kmemleak:
> 
>   unreferenced object 0xffff888116111000 (size 2048):
>     comm "modprobe", pid 817, jiffies 4294759745 (age 76.502s)
>     hex dump (first 32 bytes):
>       00 c4 0a 04 81 88 ff ff 08 10 11 16 81 88 ff ff  ................
>       08 10 11 16 81 88 ff ff 00 00 00 00 00 00 00 00  ................
>     backtrace:
>       [<ffffffff815bcd82>] kmalloc_trace+0x22/0x60
>       [<ffffffff827e20ee>] phy_device_create+0x4e/0x90
>       [<ffffffff827e6072>] get_phy_device+0xd2/0x220
>       [<ffffffff827e7844>] mdiobus_scan+0xa4/0x2e0
>       [<ffffffff827e8be2>] __mdiobus_register+0x482/0x8b0
>       [<ffffffffa01f5d24>] r6040_init_one+0x714/0xd2c [r6040]
>       ...
> 
> The problem occurs in probe process as follows:
>   r6040_init_one:
>     mdiobus_register
>       mdiobus_scan    <- alloc and register phy_device,
>                          the reference count of phy_device is 3
>     r6040_mii_probe
>       phy_connect     <- connect to the first phy_device,
>                          so the reference count of the first
>                          phy_device is 4, others are 3
>     register_netdev   <- fault inject succeeded, goto error handling path
> 
>     // error handling path
>     err_out_mdio_unregister:
>       mdiobus_unregister(lp->mii_bus);
>     err_out_mdio:
>       mdiobus_free(lp->mii_bus);    <- the reference count of the first
>                                        phy_device is 1, it is not released
>                                        and other phy_devices are released
>   // similarly, the remove process also has the same problem
> 
> The root cause is traced to the phy_device is not disconnected when
> removes one r6040 device in r6040_remove_one() or on error handling path
> after r6040_mii probed successfully. In r6040_mii_probe(), a net ethernet
> device is connected to the first PHY device of mii_bus, in order to
> notify the connected driver when the link status changes, which is the
> default behavior of the PHY infrastructure to handle everything.
> Therefore the phy_device should be disconnected when removes one r6040
> device or on error handling path.
> 
> Fix it by adding phy_disconnect() when removes one r6040 device or on
> error handling path after r6040_mii probed successfully.
> 
> Fixes: 3831861b4ad8 ("r6040: implement phylib")
> Signed-off-by: Li Zetao <lizetao1@huawei.com>
> ---
> v1 -> v2: change the subject prefix "PATCH" to "PATCH net" and change
> the goto label name "err_out_r6040_mii_remove" to "err_out_phy_disconnect"
> 
>  drivers/net/ethernet/rdc/r6040.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

Please don't send new patches as reply-to.

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Re: [PATCH net v2] r6040: Fix kmemleak in probe and remove

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (master)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 13 Dec 2022 20:56:14 +0800 you wrote:
> There is a memory leaks reported by kmemleak:
> 
>   unreferenced object 0xffff888116111000 (size 2048):
>     comm "modprobe", pid 817, jiffies 4294759745 (age 76.502s)
>     hex dump (first 32 bytes):
>       00 c4 0a 04 81 88 ff ff 08 10 11 16 81 88 ff ff  ................
>       08 10 11 16 81 88 ff ff 00 00 00 00 00 00 00 00  ................
>     backtrace:
>       [<ffffffff815bcd82>] kmalloc_trace+0x22/0x60
>       [<ffffffff827e20ee>] phy_device_create+0x4e/0x90
>       [<ffffffff827e6072>] get_phy_device+0xd2/0x220
>       [<ffffffff827e7844>] mdiobus_scan+0xa4/0x2e0
>       [<ffffffff827e8be2>] __mdiobus_register+0x482/0x8b0
>       [<ffffffffa01f5d24>] r6040_init_one+0x714/0xd2c [r6040]
>       ...
> 
> [...]

Here is the summary with links:
  - [net,v2] r6040: Fix kmemleak in probe and remove
    https://git.kernel.org/netdev/net/c/7e43039a49c2

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html