[PATCH 0/3] net: ethernat: ezchip: bug fixing and code improvments

[PATCH 0/3] net: ethernat: ezchip: bug fixing and code improvments

[Pavel Skripkin]

While manual code reviewing, I found some error in ezchip driver.
Two of them looks very dangerous:
  1. use-after-free in nps_enet_remove
      Accessing netdev private data after free_netdev()

  2. wrong error handling of platform_get_irq()
      It can cause passing negative irq to request_irq()

Also, in 2nd patch I removed redundant check to increase execution
speed and make code more straightforward.

Pavel Skripkin (3):
  net: ethernet: ezchip: fix UAF in nps_enet_remove
  net: ethernet: ezchip: remove redundant check
  net: ethernet: ezchip: fix error handling

 drivers/net/ethernet/ezchip/nps_enet.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

-- 
2.32.0

[PATCH 1/3] net: ethernet: ezchip: fix UAF in nps_enet_remove

[Pavel Skripkin]

priv is netdev private data, but it is used
after free_netdev(). It can cause use-after-free when accessing priv
pointer. So, fix it by moving free_netdev() after netif_napi_del()
call.

Fixes: 0dd077093636 ("NET: Add ezchip ethernet driver")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index e3954d8835e7..20d2c2bb26e4 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -642,8 +642,8 @@ static s32 nps_enet_remove(struct platform_device *pdev)
 	struct nps_enet_priv *priv = netdev_priv(ndev);
 
 	unregister_netdev(ndev);
-	free_netdev(ndev);
 	netif_napi_del(&priv->napi);
+	free_netdev(ndev);
 
 	return 0;
 }
-- 
2.32.0

[PATCH 2/3] net: ethernet: ezchip: remove redundant check

[Pavel Skripkin]

err varibale will be set everytime, when code gets
into this path. This check will just slowdown the execution
and that's all.

Fixes: 0dd077093636 ("NET: Add ezchip ethernet driver")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index 20d2c2bb26e4..c562a1e83913 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -630,8 +630,7 @@ static s32 nps_enet_probe(struct platform_device *pdev)
 out_netif_api:
 	netif_napi_del(&priv->napi);
 out_netdev:
-	if (err)
-		free_netdev(ndev);
+	free_netdev(ndev);
 
 	return err;
 }
-- 
2.32.0

[PATCH 3/3] net: ethernet: ezchip: fix error handling

[Pavel Skripkin]

As documented at drivers/base/platform.c for platform_get_irq:

 * Gets an IRQ for a platform device and prints an error message if finding the
 * IRQ fails. Device drivers should check the return value for errors so as to
 * not pass a negative integer value to the request_irq() APIs.

So, the driver should check that platform_get_irq() return value
is _negative_, not that it's equal to zero, because -ENXIO (return
value from request_irq() if irq was not found) will
pass this check and it leads to passing negative irq to request_irq()

Fixes: 0dd077093636 ("NET: Add ezchip ethernet driver")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index c562a1e83913..f9a288a6ec8c 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -607,7 +607,7 @@ static s32 nps_enet_probe(struct platform_device *pdev)
 
 	/* Get IRQ number */
 	priv->irq = platform_get_irq(pdev, 0);
-	if (!priv->irq) {
+	if (priv->irq < 0) {
 		dev_err(dev, "failed to retrieve <irq Rx-Tx> value from device tree\n");
 		err = -ENODEV;
 		goto out_netdev;
-- 
2.32.0

Re: [PATCH 0/3] net: ethernat: ezchip: bug fixing and code improvments

[patchwork-bot+netdevbpf]

Hello:

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

On Fri, 18 Jun 2021 19:14:23 +0300 you wrote:
> While manual code reviewing, I found some error in ezchip driver.
> Two of them looks very dangerous:
>   1. use-after-free in nps_enet_remove
>       Accessing netdev private data after free_netdev()
> 
>   2. wrong error handling of platform_get_irq()
>       It can cause passing negative irq to request_irq()
> 
> [...]

Here is the summary with links:
  - [1/3] net: ethernet: ezchip: fix UAF in nps_enet_remove
    https://git.kernel.org/netdev/net/c/e4b8700e07a8
  - [2/3] net: ethernet: ezchip: remove redundant check
    https://git.kernel.org/netdev/net/c/4ae85b23e1f0
  - [3/3] net: ethernet: ezchip: fix error handling
    https://git.kernel.org/netdev/net/c/0de449d59959

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