[PATCH 0/2] net: fix use-after-free bugs

I've added new checker to smatch yesterday. It warns about using
netdev_priv() pointer after free_{netdev,candev}() call. I hope, it will
get into next smatch release.

Some of the reported bugs are fixed and upstreamed already, but Dan ran new
smatch with allmodconfig and found 2 more. Big thanks to Dan for doing it,
because I totally forgot to do it.

Pavel Skripkin (2):
  net: fec: fix use-after-free in fec_drv_remove
  net: vxge: fix use-after-free in vxge_device_unregister

 drivers/net/ethernet/freescale/fec_main.c      | 2 +-
 drivers/net/ethernet/neterion/vxge/vxge-main.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.32.0

Smatch says:
	drivers/net/ethernet/freescale/fec_main.c:3994 fec_drv_remove() error: Using fep after free_{netdev,candev}(ndev);
	drivers/net/ethernet/freescale/fec_main.c:3995 fec_drv_remove() error: Using fep after free_{netdev,candev}(ndev);

Since fep pointer is netdev private data, accessing it after free_netdev()
call can cause use-after-free bug. Fix it by moving free_netdev() call at
the end of the function

Reported-by: Dan Carpenter <redacted>
Fixes: a31eda65ba21 ("net: fec: fix clock count mis-match")
Signed-off-by: Pavel Skripkin <redacted>
---
 drivers/net/ethernet/freescale/fec_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 8aea707a65a7..7e4c4980ced7 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c

@@ -3843,13 +3843,13 @@ fec_drv_remove(struct platform_device *pdev)
 	if (of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
 	of_node_put(fep->phy_node);
-	free_netdev(ndev);
 
 	clk_disable_unprepare(fep->clk_ahb);
 	clk_disable_unprepare(fep->clk_ipg);
 	pm_runtime_put_noidle(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
+	free_netdev(ndev);
 	return 0;
 }
 

-- 
2.32.0

Smatch says:
drivers/net/ethernet/neterion/vxge/vxge-main.c:3518 vxge_device_unregister() error: Using vdev after free_{netdev,candev}(dev);
drivers/net/ethernet/neterion/vxge/vxge-main.c:3518 vxge_device_unregister() error: Using vdev after free_{netdev,candev}(dev);
drivers/net/ethernet/neterion/vxge/vxge-main.c:3520 vxge_device_unregister() error: Using vdev after free_{netdev,candev}(dev);
drivers/net/ethernet/neterion/vxge/vxge-main.c:3520 vxge_device_unregister() error: Using vdev after free_{netdev,candev}(dev);

Since vdev pointer is netdev private data accessing it after free_netdev()
call can cause use-after-free bug. Fix it by moving free_netdev() call at
the end of the function

Fixes: 6cca200362b4 ("vxge: cleanup probe error paths")
Reported-by: Dan Carpenter <redacted>
Signed-off-by: Pavel Skripkin <redacted>
---
 drivers/net/ethernet/neterion/vxge/vxge-main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c b/drivers/net/ethernet/neterion/vxge/vxge-main.c
index 82eef4c72f01..7abd13e69471 100644
--- a/drivers/net/ethernet/neterion/vxge/vxge-main.c
+++ b/drivers/net/ethernet/neterion/vxge/vxge-main.c

@@ -3512,13 +3512,13 @@ static void vxge_device_unregister(struct __vxge_hw_device *hldev)
 
 	kfree(vdev->vpaths);
 
-	/* we are safe to free it now */
-	free_netdev(dev);
-
 	vxge_debug_init(vdev->level_trace, "%s: ethernet device unregistered",
 			buf);
 	vxge_debug_entryexit(vdev->level_trace,	"%s: %s:%d  Exiting...", buf,
 			     __func__, __LINE__);
+
+	/* we are safe to free it now */
+	free_netdev(dev);
 }
 
 /*

-- 
2.32.0

On 8/4/2021 8:48 AM, Pavel Skripkin wrote:

I've added new checker to smatch yesterday. It warns about using
netdev_priv() pointer after free_{netdev,candev}() call. I hope, it will
get into next smatch release.

Some of the reported bugs are fixed and upstreamed already, but Dan ran new
smatch with allmodconfig and found 2 more. Big thanks to Dan for doing it,
because I totally forgot to do it.

Pavel Skripkin (2):
  net: fec: fix use-after-free in fec_drv_remove
  net: vxge: fix use-after-free in vxge_device_unregister

 drivers/net/ethernet/freescale/fec_main.c      | 2 +-
 drivers/net/ethernet/neterion/vxge/vxge-main.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

Looks like a good new check! For the series:

Reviewed-by: Jesse Brandeburg <redacted>

-----Original Message-----
From: Pavel Skripkin <redacted>
Sent: 2021年8月4日 23:52
To: davem@davemloft.net; kuba@kernel.org; Joakim Zhang
[off-list ref]; hslester96@gmail.com; fugang.duan@nxp.com
Cc: dan.carpenter@oracle.com; netdev@vger.kernel.org;
linux-kernel@vger.kernel.org; Pavel Skripkin [off-list ref]
Subject: [PATCH 1/2] net: fec: fix use-after-free in fec_drv_remove

Smatch says:
	drivers/net/ethernet/freescale/fec_main.c:3994 fec_drv_remove() error:
Using fep after free_{netdev,candev}(ndev);
	drivers/net/ethernet/freescale/fec_main.c:3995 fec_drv_remove() error:
Using fep after free_{netdev,candev}(ndev);

Since fep pointer is netdev private data, accessing it after free_netdev() call can
cause use-after-free bug. Fix it by moving free_netdev() call at the end of the
function

Reported-by: Dan Carpenter <redacted>
Fixes: a31eda65ba21 ("net: fec: fix clock count mis-match")
Signed-off-by: Pavel Skripkin <redacted>
---

Thanks.

Reviewed-by: Joakim Zhang <redacted>

Best Regards,
Joakim Zhang

Hello:

This series was applied to netdev/net.git (refs/heads/master):

On Wed,  4 Aug 2021 18:48:57 +0300 you wrote:

I've added new checker to smatch yesterday. It warns about using
netdev_priv() pointer after free_{netdev,candev}() call. I hope, it will
get into next smatch release.

Some of the reported bugs are fixed and upstreamed already, but Dan ran new
smatch with allmodconfig and found 2 more. Big thanks to Dan for doing it,
because I totally forgot to do it.

[...]

Here is the summary with links:
  - [1/2] net: fec: fix use-after-free in fec_drv_remove
    https://git.kernel.org/netdev/net/c/44712965bf12
  - [2/2] net: vxge: fix use-after-free in vxge_device_unregister
    https://git.kernel.org/netdev/net/c/942e560a3d38

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