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

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

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 <redacted>
---
 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

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 <redacted>
---
 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

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 <redacted>
---
 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

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