netdev - [PATCH net-next] net: fec: Use a proper ID allocation scheme

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 17 Feb 2020 19:36:51 -0300
From:   Fabio Estevam <festevam@...il.com>
To:     davem@...emloft.net
Cc:     fugang.duan@....com, netdev@...r.kernel.org, andrew@...n.ch,
        Fabio Estevam <festevam@...il.com>
Subject: [PATCH net-next] net: fec: Use a proper ID allocation scheme

Currently when performing an unbind/bind operation network is no
longer functional:

# echo 2188000.ethernet > /sys/bus/platform/drivers/fec/unbind
# echo 2188000.ethernet > /sys/bus/platform/drivers/fec/bind
[   10.756519] pps pps0: new PPS source ptp0
[   10.792626] libphy: fec_enet_mii_bus: probed
[   10.799330] fec 2188000.ethernet eth0: registered PHC device 1
# udhcpc -i eth0
udhcpc: started, v1.31.1
[   14.985211] fec 2188000.ethernet eth0: no PHY, assuming direct connection to switch
[   14.993140] libphy: PHY fixed-0:00 not found
[   14.997643] fec 2188000.ethernet eth0: could not attach to PHY
ifconfig: SIOCSIFFLAGS: No such device

The reason for the failure is that fec_drv_remove() does not
decrement the dev_id variable.

Instead of using such poor mechanism for counting the network interfaces
IDs, use a proper allocation scheme, such as IDR.

This fixes the network behavior after unbind/bind.

Tested on a imx6qp-sabresd board.

Suggested-by: David Miller <davem@...emloft.net>
Signed-off-by: Fabio Estevam <festevam@...il.com>
---
Hi,

There was a prior attempt to fix this problem:
https://www.spinics.net/lists/netdev/msg616487.html

, but it was rejected and David suggested the usage of IDR.

 drivers/net/ethernet/freescale/fec.h      |  1 +
 drivers/net/ethernet/freescale/fec_main.c | 15 +++++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index f79e57f735b3..0d718545b9a2 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -528,6 +528,7 @@ struct fec_enet_private {
 	struct	platform_device *pdev;
 
 	int	dev_id;
+	struct  idr idr;
 
 	/* Phylib and MDIO interface */
 	struct	mii_bus *mii_bus;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 4432a59904c7..77b63ecf96b4 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -62,6 +62,7 @@
 #include <linux/if_vlan.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/prefetch.h>
+#include <linux/idr.h>
 #include <soc/imx/cpuidle.h>
 
 #include <asm/cacheflush.h>
@@ -1949,7 +1950,6 @@ static int fec_enet_mii_probe(struct net_device *ndev)
 	char mdio_bus_id[MII_BUS_ID_SIZE];
 	char phy_name[MII_BUS_ID_SIZE + 3];
 	int phy_id;
-	int dev_id = fep->dev_id;
 
 	if (fep->phy_node) {
 		phy_dev = of_phy_connect(ndev, fep->phy_node,
@@ -1964,8 +1964,6 @@ static int fec_enet_mii_probe(struct net_device *ndev)
 		for (phy_id = 0; (phy_id < PHY_MAX_ADDR); phy_id++) {
 			if (!mdiobus_is_registered_device(fep->mii_bus, phy_id))
 				continue;
-			if (dev_id--)
-				continue;
 			strlcpy(mdio_bus_id, fep->mii_bus->id, MII_BUS_ID_SIZE);
 			break;
 		}
@@ -3406,7 +3404,6 @@ fec_probe(struct platform_device *pdev)
 	struct net_device *ndev;
 	int i, irq, ret = 0;
 	const struct of_device_id *of_id;
-	static int dev_id;
 	struct device_node *np = pdev->dev.of_node, *phy_node;
 	int num_tx_qs;
 	int num_rx_qs;
@@ -3451,7 +3448,13 @@ fec_probe(struct platform_device *pdev)
 	}
 
 	fep->pdev = pdev;
-	fep->dev_id = dev_id++;
+	idr_init(&fep->idr);
+
+	ret = idr_alloc_cyclic(&fep->idr, fep, 0, INT_MAX, GFP_KERNEL);
+	if (ret < 0)
+		return ret;
+
+	fep->dev_id = ret;
 
 	platform_set_drvdata(pdev, ndev);
 
@@ -3632,7 +3635,6 @@ fec_probe(struct platform_device *pdev)
 		of_phy_deregister_fixed_link(np);
 	of_node_put(phy_node);
 failed_phy:
-	dev_id--;
 failed_ioremap:
 	free_netdev(ndev);
 
@@ -3661,6 +3663,7 @@ 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);
+	idr_destroy(&fep->idr);
 	free_netdev(ndev);
 
 	clk_disable_unprepare(fep->clk_ahb);
-- 
2.17.1