2021-08-04 15:56:12

by Pavel Skripkin

[permalink] [raw]
Subject: [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


2021-08-04 19:16:46

by Pavel Skripkin

[permalink] [raw]
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 <[email protected]>
Fixes: a31eda65ba21 ("net: fec: fix clock count mis-match")
Signed-off-by: Pavel Skripkin <[email protected]>
---
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

2021-08-04 19:16:48

by Pavel Skripkin

[permalink] [raw]
Subject: [PATCH 2/2] net: vxge: fix use-after-free in vxge_device_unregister

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 <[email protected]>
Signed-off-by: Pavel Skripkin <[email protected]>
---
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

2021-08-04 20:55:38

by Jesse Brandeburg

[permalink] [raw]
Subject: Re: [PATCH 0/2] net: fix use-after-free bugs

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 <[email protected]>


2021-08-05 04:58:14

by Joakim Zhang

[permalink] [raw]
Subject: RE: [PATCH 1/2] net: fec: fix use-after-free in fec_drv_remove


> -----Original Message-----
> From: Pavel Skripkin <[email protected]>
> Sent: 2021??8??4?? 23:52
> To: [email protected]; [email protected]; Joakim Zhang
> <[email protected]>; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; Pavel Skripkin <[email protected]>
> 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 <[email protected]>
> Fixes: a31eda65ba21 ("net: fec: fix clock count mis-match")
> Signed-off-by: Pavel Skripkin <[email protected]>
> ---
Thanks.

Reviewed-by: Joakim Zhang <[email protected]>

Best Regards,
Joakim Zhang

2021-08-05 14:57:56

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH 0/2] net: fix use-after-free bugs

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