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

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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.

---
 drivers/net/ethernet/ezchip/nps_enet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index e3954d8835e7..20d2c2bb26e4 100644
@@ -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

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

---
 drivers/net/ethernet/ezchip/nps_enet.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index 20d2c2bb26e4..c562a1e83913 100644
@@ -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

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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()

---
 drivers/net/ethernet/ezchip/nps_enet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index c562a1e83913..f9a288a6ec8c 100644
@@ -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

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees