Skip to contentSkip to conversation

net: stmmac: fix probe error handling and phydev leaks

PATCH In review Johan Hovold 5 participants 12 messages Nov 30, 2016 – Dec 2, 2016

Subsystems (9)
Networking Network drivers Arm/Amlogic Meson SoC support Arm STM32 +5 more subsystems
lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

cover net: stmmac: fix probe error handling and phydev leaks

This series fixes a number of issues with the stmmac-driver probe error handling, which for example left clocks enabled after probe failures.

The final patch fixes a failure to deregister and free any fixed-link PHYs that were registered during probe on probe errors and on driver unbind. It also fixes a related of-node leak on late probe errors.

This series depends on the of_phy_deregister_fixed_link() helper that was just merged to net.

As mentioned earlier, one staging driver also suffers from a similar leak and can be fixed up once the above mentioned helper hits mainline.

Note that these patches have only been compile tested.

Johan

Johan Hovold (7):
net: ethernet: stmmac: dwmac-socfpga: fix use-after-free on probe
errors
net: ethernet: stmmac: dwmac-sti: fix probe error path
net: ethernet: stmmac: dwmac-rk: fix probe error path
net: ethernet: stmmac: dwmac-generic: fix probe error path
net: ethernet: stmmac: dwmac-meson8b: fix probe error path
net: ethernet: stmmac: platform: fix outdated function header
net: ethernet: stmmac: fix of-node and fixed-link-phydev leaks

 .../net/ethernet/stmicro/stmmac/dwmac-generic.c    | 17 ++++++++--
 .../net/ethernet/stmicro/stmmac/dwmac-ipq806x.c    | 25 ++++++++++----
 .../net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c    | 17 ++++++++--
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c  | 23 ++++++++++---
 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 32 +++++++++++++-----
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c     | 21 +++++++++---
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    | 39 ++++++++++++++--------
 drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c    | 23 ++++++++++---
 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c  | 19 ++++++++---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c  | 26 +++++++++++----
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  1 -
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 33 +++++++++++++++---
 .../net/ethernet/stmicro/stmmac/stmmac_platform.h  |  2 ++
 13 files changed, 215 insertions(+), 63 deletions(-)
Signature · 1 line
2.7.3
author
#1 lore raw

PATCH 1/7 net: ethernet: stmmac: dwmac-socfpga: fix use-after-free on probe errors

Make sure to call stmmac_dvr_remove() before returning on late probe errors so that memory is freed, clocks are disabled, and the netdev is deregistered before its resources go away.

Fixes 3c201b5a84ed ("net: stmmac: socfpga: Remove re-registration of

reset controller")

Signed-off-by Johan Hovold <[email protected]>
---
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    | 29 ++++++++++++++--------
 1 file changed, 19 insertions(+), 10 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c +19 -10
index bec6963ac71e..47db157da3e8 100644
@@ -304,6 +304,8 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	struct device		*dev = &pdev->dev;
 	int			ret;
 	struct socfpga_dwmac	*dwmac;
+	struct net_device	*ndev;
+	struct stmmac_priv	*stpriv;
 
 	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
 	if (ret)

@@ -327,19 +329,26 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret)
+		return ret;
 
-	if (!ret) {
-		struct net_device *ndev = platform_get_drvdata(pdev);
-		struct stmmac_priv *stpriv = netdev_priv(ndev);
+	ndev = platform_get_drvdata(pdev);
+	stpriv = netdev_priv(ndev);
 
-		/* The socfpga driver needs to control the stmmac reset to
-		 * set the phy mode. Create a copy of the core reset handel
-		 * so it can be used by the driver later.
-		 */
-		dwmac->stmmac_rst = stpriv->stmmac_rst;
+	/* The socfpga driver needs to control the stmmac reset to set the phy
+	 * mode. Create a copy of the core reset handle so it can be used by
+	 * the driver later.
+	 */
+	dwmac->stmmac_rst = stpriv->stmmac_rst;
 
-		ret = socfpga_dwmac_set_phy_mode(dwmac);
-	}
+	ret = socfpga_dwmac_set_phy_mode(dwmac);
+	if (ret)
+		goto err_dvr_remove;
+
+	return 0;
+
+err_dvr_remove:
+	stmmac_dvr_remove(&pdev->dev);
 
 	return ret;
 }
Signature · 1 line
2.7.3
author
#2 lore raw

PATCH 2/7 net: ethernet: stmmac: dwmac-sti: fix probe error path

In reply to Johan Hovold

Make sure to disable clocks before returning on late probe errors.

Fixes 8387ee21f972 ("stmmac: dwmac-sti: turn setup callback into a

probe function")

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c +10 -1
index 58c05acc2aab..a1ce018bf844 100644
@@ -365,7 +365,16 @@ static int sti_dwmac_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret)
+		goto err_dwmac_exit;
+
+	return 0;
+
+err_dwmac_exit:
+	sti_dwmac_exit(pdev, plat_dat->bsp_priv);
+
+	return ret;
 }
 
 static const struct sti_dwmac_of_data stih4xx_dwmac_data = {
Signature · 1 line
2.7.3
author
#3 lore raw

PATCH 3/7 net: ethernet: stmmac: dwmac-rk: fix probe error path

In reply to Johan Hovold

Make sure to disable runtime PM, power down the PHY, and disable clocks before returning on late probe errors.

Fixes 27ffefd2d109 ("stmmac: dwmac-rk: create a new probe function")
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +10 -1
index 3740a4417fa0..e7aabe56c15a 100644
@@ -988,7 +988,16 @@ static int rk_gmac_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret)
+		goto err_gmac_exit;
+
+	return 0;
+
+err_gmac_exit:
+	rk_gmac_exit(pdev, plat_dat->bsp_priv);
+
+	return ret;
 }
 
 static const struct of_device_id rk_gmac_dwmac_match[] = {
Signature · 1 line
2.7.3
author
#4 lore raw

PATCH 4/7 net: ethernet: stmmac: dwmac-generic: fix probe error path

In reply to Johan Hovold

Make sure to call any exit() callback to undo the effect of init() before returning on late probe errors.

Fixes cf3f047b9af4 ("stmmac: move hw init in the probe (v2)")
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c +11 -1
index b1e5f24708c9..05e46a82cdb1 100644
@@ -53,7 +53,17 @@ static int dwmac_generic_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret)
+		goto err_exit;
+
+	return 0;
+
+err_exit:
+	if (plat_dat->exit)
+		plat_dat->exit(pdev, plat_dat->bsp_priv);
+
+	return ret;
 }
 
 static const struct of_device_id dwmac_generic_match[] = {
Signature · 1 line
2.7.3
author
#5 lore raw

PATCH 5/7 net: ethernet: stmmac: dwmac-meson8b: fix probe error path

In reply to Johan Hovold

Make sure to disable clocks before returning on late probe errors.

Fixes 566e82516253 ("net: stmmac: add a glue driver for the Amlogic

Meson 8b / GXBB DWMAC")

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c +10 -1
index 250e4ceafc8d..45e7aaf0170d 100644
@@ -289,7 +289,16 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 
 	plat_dat->bsp_priv = dwmac;
 
-	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret)
+		goto err_clk_disable;
+
+	return 0;
+
+err_clk_disable:
+	clk_disable_unprepare(dwmac->m25_div_clk);
+
+	return ret;
 }
 
 static int meson8b_dwmac_remove(struct platform_device *pdev)
Signature · 1 line
2.7.3
author
#7 lore raw

PATCH 6/7 net: ethernet: stmmac: platform: fix outdated function header

In reply to Johan Hovold

Fix the OF-helper function header to reflect that the function no longer has a platform-data parameter.

Fixes b0003ead75f3 ("stmmac: make stmmac_probe_config_dt return the

platform data struct")

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
 1 file changed, 1 deletion(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +0 -1
index 0a0d6a86f397..bcbf123d5ba2 100644
@@ -200,7 +200,6 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
 /**
  * stmmac_probe_config_dt - parse device-tree driver parameters
  * @pdev: platform_device structure
- * @plat: driver data platform structure
  * @mac: MAC address to use
  * Description:
  * this function is to read the driver parameters from device-tree and
Signature · 1 line
2.7.3
author
#8 lore raw

PATCH 7/7 net: ethernet: stmmac: fix of-node and fixed-link-phydev leaks

In reply to Johan Hovold

Make sure to deregister and free any fixed-link phy registered during probe on probe errors and on driver unbind by adding a new glue helper function.

Drop the of-node reference taken in the same path also on late probe errors (and not just on driver unbind) by moving the put from stmmac_dvr_remove() to the new helper.

Fixes 277323814e49 ("stmmac: add fixed-link device-tree support")
Fixes 4613b279bee7 ("ethernet: stmicro: stmmac: add missing of_node_put

after calling of_parse_phandle")

Signed-off-by Johan Hovold <[email protected]>
---
 .../net/ethernet/stmicro/stmmac/dwmac-generic.c    |  5 +++-
 .../net/ethernet/stmicro/stmmac/dwmac-ipq806x.c    | 25 +++++++++++++----
 .../net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c    | 17 ++++++++++--
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c  | 23 ++++++++++++----
 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 21 +++++++++-----
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c     | 10 +++++--
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    | 12 +++++---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c    | 12 +++++---
 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c  | 19 +++++++++----
 drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c  | 26 +++++++++++++-----
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  1 -
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 32 ++++++++++++++++++++--
 .../net/ethernet/stmicro/stmmac/stmmac_platform.h  |  2 ++
 13 files changed, 156 insertions(+), 49 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c +4 -1
index 05e46a82cdb1..e6e6c2fcc4b7 100644
@@ -50,7 +50,7 @@ static int dwmac_generic_probe(struct platform_device *pdev)
 	if (plat_dat->init) {
 		ret = plat_dat->init(pdev, plat_dat->bsp_priv);
 		if (ret)
-			return ret;
+			goto err_remove_config_dt;
 	}
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);

@@ -62,6 +62,9 @@ static int dwmac_generic_probe(struct platform_device *pdev)
 err_exit:
 	if (plat_dat->exit)
 		plat_dat->exit(pdev, plat_dat->bsp_priv);
+err_remove_config_dt:
+	if (pdev->dev.of_node)
+		stmmac_remove_config_dt(pdev, plat_dat);
 
 	return ret;
 }
drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c +19 -6
index 36d3355f2fb0..866444b6c82f 100644
@@ -271,15 +271,17 @@ static int ipq806x_gmac_probe(struct platform_device *pdev)
 		return PTR_ERR(plat_dat);
 
 	gmac = devm_kzalloc(dev, sizeof(*gmac), GFP_KERNEL);
-	if (!gmac)
-		return -ENOMEM;
+	if (!gmac) {
+		err = -ENOMEM;
+		goto err_remove_config_dt;
+	}
 
 	gmac->pdev = pdev;
 
 	err = ipq806x_gmac_of_parse(gmac);
 	if (err) {
 		dev_err(dev, "device tree parsing error\n");
-		return err;
+		goto err_remove_config_dt;
 	}
 
 	regmap_write(gmac->qsgmii_csr, QSGMII_PCS_CAL_LCKDT_CTL,

@@ -300,7 +302,8 @@ static int ipq806x_gmac_probe(struct platform_device *pdev)
 	default:
 		dev_err(&pdev->dev, "Unsupported PHY mode: \"%s\"\n",
 			phy_modes(gmac->phy_mode));
-		return -EINVAL;
+		err = -EINVAL;
+		goto err_remove_config_dt;
 	}
 	regmap_write(gmac->nss_common, NSS_COMMON_GMAC_CTL(gmac->id), val);
 

@@ -319,7 +322,8 @@ static int ipq806x_gmac_probe(struct platform_device *pdev)
 	default:
 		dev_err(&pdev->dev, "Unsupported PHY mode: \"%s\"\n",
 			phy_modes(gmac->phy_mode));
-		return -EINVAL;
+		err = -EINVAL;
+		goto err_remove_config_dt;
 	}
 	regmap_write(gmac->nss_common, NSS_COMMON_CLK_SRC_CTRL, val);
 

@@ -346,7 +350,16 @@ static int ipq806x_gmac_probe(struct platform_device *pdev)
 	plat_dat->bsp_priv = gmac;
 	plat_dat->fix_mac_speed = ipq806x_gmac_fix_mac_speed;
 
-	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (err)
+		goto err_remove_config_dt;
+
+	return 0;
+
+err_remove_config_dt:
+	stmmac_remove_config_dt(pdev, plat_dat);
+
+	return err;
 }
 
 static const struct of_device_id ipq806x_gmac_dwmac_match[] = {
drivers/net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c +14 -3
index 78e9d1861896..3d3f43d91b98 100644
@@ -46,7 +46,8 @@ static int lpc18xx_dwmac_probe(struct platform_device *pdev)
 	reg = syscon_regmap_lookup_by_compatible("nxp,lpc1850-creg");
 	if (IS_ERR(reg)) {
 		dev_err(&pdev->dev, "syscon lookup failed\n");
-		return PTR_ERR(reg);
+		ret = PTR_ERR(reg);
+		goto err_remove_config_dt;
 	}
 
 	if (plat_dat->interface == PHY_INTERFACE_MODE_MII) {

@@ -55,13 +56,23 @@ static int lpc18xx_dwmac_probe(struct platform_device *pdev)
 		ethmode = LPC18XX_CREG_CREG6_ETHMODE_RMII;
 	} else {
 		dev_err(&pdev->dev, "Only MII and RMII mode supported\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_remove_config_dt;
 	}
 
 	regmap_update_bits(reg, LPC18XX_CREG_CREG6,
 			   LPC18XX_CREG_CREG6_ETHMODE_MASK, ethmode);
 
-	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret)
+		goto err_remove_config_dt;
+
+	return 0;
+
+err_remove_config_dt:
+	stmmac_remove_config_dt(pdev, plat_dat);
+
+	return ret;
 }
 
 static const struct of_device_id lpc18xx_dwmac_match[] = {
drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c +18 -5
index 309d99536a2c..7fdd1760a74c 100644
@@ -64,18 +64,31 @@ static int meson6_dwmac_probe(struct platform_device *pdev)
 		return PTR_ERR(plat_dat);
 
 	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
-	if (!dwmac)
-		return -ENOMEM;
+	if (!dwmac) {
+		ret = -ENOMEM;
+		goto err_remove_config_dt;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	dwmac->reg = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(dwmac->reg))
-		return PTR_ERR(dwmac->reg);
+	if (IS_ERR(dwmac->reg)) {
+		ret = PTR_ERR(dwmac->reg);
+		goto err_remove_config_dt;
+	}
 
 	plat_dat->bsp_priv = dwmac;
 	plat_dat->fix_mac_speed = meson6_dwmac_fix_mac_speed;
 
-	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret)
+		goto err_remove_config_dt;
+
+	return 0;
+
+err_remove_config_dt:
+	stmmac_remove_config_dt(pdev, plat_dat);
+
+	return ret;
 }
 
 static const struct of_device_id meson6_dwmac_match[] = {
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c +14 -7
index 45e7aaf0170d..ffaed1f35efe 100644
@@ -264,28 +264,33 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 		return PTR_ERR(plat_dat);
 
 	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
-	if (!dwmac)
-		return -ENOMEM;
+	if (!dwmac) {
+		ret = -ENOMEM;
+		goto err_remove_config_dt;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	dwmac->regs = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(dwmac->regs))
-		return PTR_ERR(dwmac->regs);
+	if (IS_ERR(dwmac->regs)) {
+		ret = PTR_ERR(dwmac->regs);
+		goto err_remove_config_dt;
+	}
 
 	dwmac->pdev = pdev;
 	dwmac->phy_mode = of_get_phy_mode(pdev->dev.of_node);
 	if (dwmac->phy_mode < 0) {
 		dev_err(&pdev->dev, "missing phy-mode property\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_remove_config_dt;
 	}
 
 	ret = meson8b_init_clk(dwmac);
 	if (ret)
-		return ret;
+		goto err_remove_config_dt;
 
 	ret = meson8b_init_prg_eth(dwmac);
 	if (ret)
-		return ret;
+		goto err_remove_config_dt;
 
 	plat_dat->bsp_priv = dwmac;
 

@@ -297,6 +302,8 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 
 err_clk_disable:
 	clk_disable_unprepare(dwmac->m25_div_clk);
+err_remove_config_dt:
+	stmmac_remove_config_dt(pdev, plat_dat);
 
 	return ret;
 }
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +7 -3
index e7aabe56c15a..d80c88bd2bba 100644
@@ -981,12 +981,14 @@ static int rk_gmac_probe(struct platform_device *pdev)
 	plat_dat->resume = rk_gmac_resume;
 
 	plat_dat->bsp_priv = rk_gmac_setup(pdev, data);
-	if (IS_ERR(plat_dat->bsp_priv))
-		return PTR_ERR(plat_dat->bsp_priv);
+	if (IS_ERR(plat_dat->bsp_priv)) {
+		ret = PTR_ERR(plat_dat->bsp_priv);
+		goto err_remove_config_dt;
+	}
 
 	ret = rk_gmac_init(pdev, plat_dat->bsp_priv);
 	if (ret)
-		return ret;
+		goto err_remove_config_dt;
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 	if (ret)

@@ -996,6 +998,8 @@ static int rk_gmac_probe(struct platform_device *pdev)
 
 err_gmac_exit:
 	rk_gmac_exit(pdev, plat_dat->bsp_priv);
+err_remove_config_dt:
+	stmmac_remove_config_dt(pdev, plat_dat);
 
 	return ret;
 }
drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c +8 -4
index 47db157da3e8..0c420e97de1e 100644
@@ -316,13 +316,15 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 		return PTR_ERR(plat_dat);
 
 	dwmac = devm_kzalloc(dev, sizeof(*dwmac), GFP_KERNEL);
-	if (!dwmac)
-		return -ENOMEM;
+	if (!dwmac) {
+		ret = -ENOMEM;
+		goto err_remove_config_dt;
+	}
 
 	ret = socfpga_dwmac_parse_data(dwmac, dev);
 	if (ret) {
 		dev_err(dev, "Unable to parse OF data\n");
-		return ret;
+		goto err_remove_config_dt;
 	}
 
 	plat_dat->bsp_priv = dwmac;

@@ -330,7 +332,7 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 	if (ret)
-		return ret;
+		goto err_remove_config_dt;
 
 	ndev = platform_get_drvdata(pdev);
 	stpriv = netdev_priv(ndev);

@@ -349,6 +351,8 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 
 err_dvr_remove:
 	stmmac_dvr_remove(&pdev->dev);
+err_remove_config_dt:
+	stmmac_remove_config_dt(pdev, plat_dat);
 
 	return ret;
 }
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c +8 -4
index a1ce018bf844..060b98c37a85 100644
@@ -345,13 +345,15 @@ static int sti_dwmac_probe(struct platform_device *pdev)
 		return PTR_ERR(plat_dat);
 
 	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
-	if (!dwmac)
-		return -ENOMEM;
+	if (!dwmac) {
+		ret = -ENOMEM;
+		goto err_remove_config_dt;
+	}
 
 	ret = sti_dwmac_parse_data(dwmac, pdev);
 	if (ret) {
 		dev_err(&pdev->dev, "Unable to parse OF data\n");
-		return ret;
+		goto err_remove_config_dt;
 	}
 
 	dwmac->fix_retime_src = data->fix_retime_src;

@@ -363,7 +365,7 @@ static int sti_dwmac_probe(struct platform_device *pdev)
 
 	ret = sti_dwmac_init(pdev, plat_dat->bsp_priv);
 	if (ret)
-		return ret;
+		goto err_remove_config_dt;
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 	if (ret)

@@ -373,6 +375,8 @@ static int sti_dwmac_probe(struct platform_device *pdev)
 
 err_dwmac_exit:
 	sti_dwmac_exit(pdev, plat_dat->bsp_priv);
+err_remove_config_dt:
+	stmmac_remove_config_dt(pdev, plat_dat);
 
 	return ret;
 }
drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c +14 -5
index e5a926b8bee7..61cb24810d10 100644
@@ -107,24 +107,33 @@ static int stm32_dwmac_probe(struct platform_device *pdev)
 		return PTR_ERR(plat_dat);
 
 	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
-	if (!dwmac)
-		return -ENOMEM;
+	if (!dwmac) {
+		ret = -ENOMEM;
+		goto err_remove_config_dt;
+	}
 
 	ret = stm32_dwmac_parse_data(dwmac, &pdev->dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Unable to parse OF data\n");
-		return ret;
+		goto err_remove_config_dt;
 	}
 
 	plat_dat->bsp_priv = dwmac;
 
 	ret = stm32_dwmac_init(plat_dat);
 	if (ret)
-		return ret;
+		goto err_remove_config_dt;
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 	if (ret)
-		stm32_dwmac_clk_disable(dwmac);
+		goto err_clk_disable;
+
+	return 0;
+
+err_clk_disable:
+	stm32_dwmac_clk_disable(dwmac);
+err_remove_config_dt:
+	stmmac_remove_config_dt(pdev, plat_dat);
 
 	return ret;
 }
drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c +19 -7
index adff46375a32..d07520fb969e 100644
@@ -120,22 +120,27 @@ static int sun7i_gmac_probe(struct platform_device *pdev)
 		return PTR_ERR(plat_dat);
 
 	gmac = devm_kzalloc(dev, sizeof(*gmac), GFP_KERNEL);
-	if (!gmac)
-		return -ENOMEM;
+	if (!gmac) {
+		ret = -ENOMEM;
+		goto err_remove_config_dt;
+	}
 
 	gmac->interface = of_get_phy_mode(dev->of_node);
 
 	gmac->tx_clk = devm_clk_get(dev, "allwinner_gmac_tx");
 	if (IS_ERR(gmac->tx_clk)) {
 		dev_err(dev, "could not get tx clock\n");
-		return PTR_ERR(gmac->tx_clk);
+		ret = PTR_ERR(gmac->tx_clk);
+		goto err_remove_config_dt;
 	}
 
 	/* Optional regulator for PHY */
 	gmac->regulator = devm_regulator_get_optional(dev, "phy");
 	if (IS_ERR(gmac->regulator)) {
-		if (PTR_ERR(gmac->regulator) == -EPROBE_DEFER)
-			return -EPROBE_DEFER;
+		if (PTR_ERR(gmac->regulator) == -EPROBE_DEFER) {
+			ret = -EPROBE_DEFER;
+			goto err_remove_config_dt;
+		}
 		dev_info(dev, "no regulator found\n");
 		gmac->regulator = NULL;
 	}

@@ -151,11 +156,18 @@ static int sun7i_gmac_probe(struct platform_device *pdev)
 
 	ret = sun7i_gmac_init(pdev, plat_dat->bsp_priv);
 	if (ret)
-		return ret;
+		goto err_remove_config_dt;
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 	if (ret)
-		sun7i_gmac_exit(pdev, plat_dat->bsp_priv);
+		goto err_gmac_exit;
+
+	return 0;
+
+err_gmac_exit:
+	sun7i_gmac_exit(pdev, plat_dat->bsp_priv);
+err_remove_config_dt:
+	stmmac_remove_config_dt(pdev, plat_dat);
 
 	return ret;
 }
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +0 -1
index 1f9ec02fa7f8..caf069a465f2 100644
@@ -3416,7 +3416,6 @@ int stmmac_dvr_remove(struct device *dev)
 	stmmac_set_mac(priv->ioaddr, false);
 	netif_carrier_off(ndev);
 	unregister_netdev(ndev);
-	of_node_put(priv->plat->phy_node);
 	if (priv->stmmac_rst)
 		reset_control_assert(priv->stmmac_rst);
 	clk_disable_unprepare(priv->pclk);
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +29 -3
index bcbf123d5ba2..a840818bf4df 100644
@@ -305,7 +305,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 		dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*dma_cfg),
 				       GFP_KERNEL);
 		if (!dma_cfg) {
-			of_node_put(plat->phy_node);
+			stmmac_remove_config_dt(pdev, plat);
 			return ERR_PTR(-ENOMEM);
 		}
 		plat->dma_cfg = dma_cfg;

@@ -328,14 +328,37 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 
 	return plat;
 }
+
+/**
+ * stmmac_remove_config_dt - undo the effects of stmmac_probe_config_dt()
+ * @pdev: platform_device structure
+ * @plat: driver data platform structure
+ *
+ * Release resources claimed by stmmac_probe_config_dt().
+ */
+void stmmac_remove_config_dt(struct platform_device *pdev,
+			     struct plat_stmmacenet_data *plat)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	if (of_phy_is_fixed_link(np))
+		of_phy_deregister_fixed_link(np);
+	of_node_put(plat->phy_node);
+}
 #else
 struct plat_stmmacenet_data *
 stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 {
 	return ERR_PTR(-ENOSYS);
 }
+
+void stmmac_remove_config_dt(struct platform_device *pdev,
+			     struct plat_stmmacenet_data *plat)
+{
+}
 #endif /* CONFIG_OF */
 EXPORT_SYMBOL_GPL(stmmac_probe_config_dt);
+EXPORT_SYMBOL_GPL(stmmac_remove_config_dt);
 
 int stmmac_get_platform_resources(struct platform_device *pdev,
 				  struct stmmac_resources *stmmac_res)

@@ -391,10 +414,13 @@ int stmmac_pltfr_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
+	struct plat_stmmacenet_data *plat = priv->plat;
 	int ret = stmmac_dvr_remove(&pdev->dev);
 
-	if (priv->plat->exit)
-		priv->plat->exit(pdev, priv->plat->bsp_priv);
+	if (plat->exit)
+		plat->exit(pdev, plat->bsp_priv);
+
+	stmmac_remove_config_dt(pdev, plat);
 
 	return ret;
 }
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h +2 -0
index 64e147f53a9c..b72eb0de57b7 100644
@@ -23,6 +23,8 @@
 
 struct plat_stmmacenet_data *
 stmmac_probe_config_dt(struct platform_device *pdev, const char **mac);
+void stmmac_remove_config_dt(struct platform_device *pdev,
+			     struct plat_stmmacenet_data *plat);
 
 int stmmac_get_platform_resources(struct platform_device *pdev,
 				  struct stmmac_resources *stmmac_res);
Signature · 1 line
2.7.3
Acked-by on v1 7/7
#9 lore raw

Re: [PATCH net 7/7] net: ethernet: stmmac: fix of-node and fixed-link-phydev leaks

Quoted 4 lines · Johan Hovold

Make sure to deregister and free any fixed-link phy registered during probe on probe errors and on driver unbind by adding a new glue helper function.

Quoted 8 lines

Drop the of-node reference taken in the same path also on late probe errors (and not just on driver unbind) by moving the put from stmmac_dvr_remove() to the new helper.

Fixes 277323814e49 ("stmmac: add fixed-link device-tree support")
Fixes 4613b279bee7 ("ethernet: stmicro: stmmac: add missing of_node_put

after calling of_parse_phandle")

Signed-off-by Johan Hovold <[email protected]>

For the sunxi part,

Acked-by Maxime Ripard <[email protected]>

Thanks!
Maxime

Signature · 3 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Acked-by on the v1 series
#10 lore raw
In reply to Johan Hovold
Quoted 7 lines · Johan Hovold

This series fixes a number of issues with the stmmac-driver probe error handling, which for example left clocks enabled after probe failures.

The final patch fixes a failure to deregister and free any fixed-link PHYs that were registered during probe on probe errors and on driver unbind. It also fixes a related of-node leak on late probe errors.

Quoted 7 lines

This series depends on the of_phy_deregister_fixed_link() helper that was just merged to net.

As mentioned earlier, one staging driver also suffers from a similar leak and can be fixed up once the above mentioned helper hits mainline.

Note that these patches have only been compile tested.

For common and STi part:

Acked-by Giuseppe Cavallaro <[email protected]>

thx
peppe

Quoted 29 lines

Johan

Johan Hovold (7):
net: ethernet: stmmac: dwmac-socfpga: fix use-after-free on probe
errors
net: ethernet: stmmac: dwmac-sti: fix probe error path
net: ethernet: stmmac: dwmac-rk: fix probe error path
net: ethernet: stmmac: dwmac-generic: fix probe error path
net: ethernet: stmmac: dwmac-meson8b: fix probe error path
net: ethernet: stmmac: platform: fix outdated function header
net: ethernet: stmmac: fix of-node and fixed-link-phydev leaks

 .../net/ethernet/stmicro/stmmac/dwmac-generic.c    | 17 ++++++++--
 .../net/ethernet/stmicro/stmmac/dwmac-ipq806x.c    | 25 ++++++++++----
 .../net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c    | 17 ++++++++--
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c  | 23 ++++++++++---
 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 32 +++++++++++++-----
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c     | 21 +++++++++---
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    | 39 ++++++++++++++--------
 drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c    | 23 ++++++++++---
 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c  | 19 ++++++++---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c  | 26 +++++++++++----
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  1 -
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 33 +++++++++++++++---
 .../net/ethernet/stmicro/stmmac/stmmac_platform.h  |  2 ++
 13 files changed, 215 insertions(+), 63 deletions(-)
maintainer
#11 lore raw
In reply to Johan Hovold

From: Johan Hovold <[email protected]>
Date: Wed, 30 Nov 2016 15:29:48 +0100

Quoted 7 lines

This series fixes a number of issues with the stmmac-driver probe error handling, which for example left clocks enabled after probe failures.

The final patch fixes a failure to deregister and free any fixed-link PHYs that were registered during probe on probe errors and on driver unbind. It also fixes a related of-node leak on late probe errors.

Quoted 7 lines

This series depends on the of_phy_deregister_fixed_link() helper that was just merged to net.

As mentioned earlier, one staging driver also suffers from a similar leak and can be fixed up once the above mentioned helper hits mainline.

Note that these patches have only been compile tested.

Series applied, thanks.