[PATCH net v3 1/7] net: macb: manage the netdev lifetime with devres

[PATCH net v3 1/7] net: macb: manage the netdev lifetime with devres

[Jiale Yao]

macb_remove() frees the netdev while its managed IRQs are only
released after the remove callback returns. An interrupt in that window
can dereference the freed netdev or queue data.

Allocate the netdev with devres as well. Since the IRQs are registered
later, devres releases them before freeing the netdev and closes the
lifetime gap.

This issue was found by a static analysis method used in our research.

Fixes: 0a4acf08ea62 ("net: macb: Use devm_request_irq()")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b8234ac4b602..ebf6ffb1cc4f 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -5812,7 +5812,8 @@ static int macb_probe(struct platform_device *pdev)
 		goto err_disable_clocks;
 	}
 
-	netdev = alloc_etherdev_mq(sizeof(*bp), num_queues);
+	netdev = devm_alloc_etherdev_mqs(&pdev->dev, sizeof(*bp),
+					 num_queues, num_queues);
 	if (!netdev) {
 		err = -ENOMEM;
 		goto err_disable_clocks;
@@ -5859,7 +5860,7 @@ static int macb_probe(struct platform_device *pdev)
 			IS_ENABLED(CONFIG_MACB_USE_HWSTAMP)) {
 		dev_err(&pdev->dev, "Timer adjust mode is not supported\n");
 		err = -EINVAL;
-		goto err_out_free_netdev;
+		goto err_disable_clocks;
 	}
 
 	/* By default we set to partial store and forward mode for zynqmp.
@@ -5893,7 +5894,7 @@ static int macb_probe(struct platform_device *pdev)
 		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(44));
 		if (err) {
 			dev_err(&pdev->dev, "failed to set DMA mask\n");
-			goto err_out_free_netdev;
+			goto err_disable_clocks;
 		}
 		bp->caps |= MACB_CAPS_DMA_64B;
 	}
@@ -5903,7 +5904,7 @@ static int macb_probe(struct platform_device *pdev)
 	netdev->irq = platform_get_irq(pdev, 0);
 	if (netdev->irq < 0) {
 		err = netdev->irq;
-		goto err_out_free_netdev;
+		goto err_disable_clocks;
 	}
 
 	/* MTU range: 68 - 1518 or 10240 */
@@ -5932,7 +5933,7 @@ static int macb_probe(struct platform_device *pdev)
 
 	err = of_get_ethdev_address(np, bp->netdev);
 	if (err == -EPROBE_DEFER)
-		goto err_out_free_netdev;
+		goto err_disable_clocks;
 	else if (err)
 		macb_get_hwaddr(bp);
 
@@ -5946,7 +5947,7 @@ static int macb_probe(struct platform_device *pdev)
 	/* IP specific init */
 	err = macb_init(pdev, macb_config);
 	if (err)
-		goto err_out_free_netdev;
+		goto err_disable_clocks;
 
 	err = macb_mii_init(bp);
 	if (err)
@@ -5988,9 +5989,6 @@ static int macb_probe(struct platform_device *pdev)
 err_out_phy_exit:
 	phy_exit(bp->phy);
 
-err_out_free_netdev:
-	free_netdev(netdev);
-
 err_disable_clocks:
 	macb_clks_disable(pclk, hclk, tx_clk, rx_clk, tsu_clk);
 	pm_runtime_disable(&pdev->dev);
@@ -6024,7 +6022,6 @@ static void macb_remove(struct platform_device *pdev)
 		pm_runtime_dont_use_autosuspend(&pdev->dev);
 		pm_runtime_set_suspended(&pdev->dev);
 		phylink_destroy(bp->phylink);
-		free_netdev(netdev);
 	}
 }
 
-- 
2.34.1

[PATCH net v3 2/7] net: fec: release IRQs before dependent resources

[Jiale Yao]

fec_drv_remove() leaves the managed IRQs active until after the remove
callback returns. The handler can then run after the device has been
unregistered, its clocks have been disabled, and its other resources have
been torn down.

Mask the hardware interrupt sources and disable each IRQ before
unregistering the netdev. Mask the sources again afterwards because
fec_stop() restores the default interrupt mask. Also perform the same
cleanup on probe failures, and manage the netdev with devres so it remains
alive until the IRQ resources are released.

This issue was found by a static analysis method used in our research.

Fixes: 0d9b2ab1c376 ("fec: Use devm_request_irq()")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 28 ++++++++++++++---------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 794ec427b0ee..b0fc5b39c748 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -5219,8 +5219,9 @@ fec_probe(struct platform_device *pdev)
 	fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);
 
 	/* Init network device */
-	ndev = alloc_etherdev_mqs(sizeof(struct fec_enet_private) +
-				  FEC_STATS_SIZE, num_tx_qs, num_rx_qs);
+	ndev = devm_alloc_etherdev_mqs(&pdev->dev,
+				       sizeof(struct fec_enet_private) +
+				       FEC_STATS_SIZE, num_tx_qs, num_rx_qs);
 	if (!ndev)
 		return -ENOMEM;
 
@@ -5247,10 +5248,8 @@ fec_probe(struct platform_device *pdev)
 	pinctrl_pm_select_default_state(&pdev->dev);
 
 	fep->hwp = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(fep->hwp)) {
-		ret = PTR_ERR(fep->hwp);
-		goto failed_ioremap;
-	}
+	if (IS_ERR(fep->hwp))
+		return PTR_ERR(fep->hwp);
 
 	fep->pdev = pdev;
 	fep->dev_id = dev_id++;
@@ -5455,6 +5454,9 @@ fec_probe(struct platform_device *pdev)
 	fec_enet_mii_remove(fep);
 failed_mii_init:
 failed_irq:
+	fec_irqs_disable(ndev);
+	while (i--)
+		disable_irq(fep->irq[i]);
 	fec_enet_deinit(ndev);
 failed_init:
 	if (fep->bufdesc_ex)
@@ -5479,9 +5481,6 @@ fec_probe(struct platform_device *pdev)
 failed_ipc_init:
 failed_phy:
 	dev_id--;
-failed_ioremap:
-	free_netdev(ndev);
-
 	return ret;
 }
 
@@ -5491,7 +5490,7 @@ fec_drv_remove(struct platform_device *pdev)
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct fec_enet_private *fep = netdev_priv(ndev);
 	struct device_node *np = pdev->dev.of_node;
-	int ret;
+	int i, irq_cnt, ret;
 
 	ret = pm_runtime_get_sync(&pdev->dev);
 	if (ret < 0)
@@ -5502,7 +5501,15 @@ fec_drv_remove(struct platform_device *pdev)
 	cancel_work_sync(&fep->tx_timeout_work);
 	if (fep->bufdesc_ex)
 		fec_ptp_stop(pdev);
+	if (ret >= 0)
+		fec_irqs_disable(ndev);
+	irq_cnt = fec_enet_get_irq_cnt(pdev);
+	for (i = 0; i < irq_cnt; i++)
+		disable_irq(fep->irq[i]);
 	unregister_netdev(ndev);
+	/* fec_stop() enables the default interrupt mask. */
+	if (ret >= 0)
+		fec_irqs_disable(ndev);
 	fec_enet_mii_remove(fep);
 	if (fep->reg_phy)
 		regulator_disable(fep->reg_phy);
@@ -5522,7 +5529,6 @@ fec_drv_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 
 	fec_enet_deinit(ndev);
-	free_netdev(ndev);
 }
 
 static int fec_suspend(struct device *dev)
-- 
2.34.1

[PATCH net v3 3/7] net: hip04: manage the netdev lifetime with devres

[Jiale Yao]

hip04_remove() frees the netdev before the managed IRQ is released
after the remove callback. Since the IRQ handler receives the netdev as
its data pointer, a late interrupt can access freed memory.

Use a managed netdev allocation. Devres then releases the IRQ, which is
registered later, before releasing the netdev.

This issue was found by a static analysis method used in our research.

Fixes: a41ea46a9a12 ("net: hisilicon: new hip04 ethernet driver")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/net/ethernet/hisilicon/hip04_eth.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
index fc2c47dcfaab..4a643dff22ab 100644
--- a/drivers/net/ethernet/hisilicon/hip04_eth.c
+++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
@@ -905,7 +905,7 @@ static int hip04_mac_probe(struct platform_device *pdev)
 	int irq;
 	int ret;
 
-	ndev = alloc_etherdev(sizeof(struct hip04_priv));
+	ndev = devm_alloc_etherdev(d, sizeof(struct hip04_priv));
 	if (!ndev)
 		return -ENOMEM;
 
@@ -1021,7 +1021,6 @@ static int hip04_mac_probe(struct platform_device *pdev)
 	hip04_free_ring(ndev, d);
 init_fail:
 	of_node_put(priv->phy_node);
-	free_netdev(ndev);
 	return ret;
 }
 
@@ -1038,7 +1037,6 @@ static void hip04_remove(struct platform_device *pdev)
 	unregister_netdev(ndev);
 	of_node_put(priv->phy_node);
 	cancel_work_sync(&priv->tx_timeout_task);
-	free_netdev(ndev);
 }
 
 static const struct of_device_id hip04_mac_match[] = {
-- 
2.34.1

[PATCH net v3 4/7] net: hisi_femac: manage the netdev lifetime with devres

[Jiale Yao]

hisi_femac_drv_remove() frees the netdev while the shared managed IRQ
remains registered until devres cleanup. Its handler uses the netdev as
private data, so an interrupt in this window can dereference freed
memory.

Manage the netdev allocation with devres so the later IRQ resource is
released before the netdev.

This issue was found by a static analysis method used in our research.

Fixes: 542ae60af24f ("net: hisilicon: Add Fast Ethernet MAC driver")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/net/ethernet/hisilicon/hisi_femac.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c
index d244a40df430..d369824fa4e8 100644
--- a/drivers/net/ethernet/hisilicon/hisi_femac.c
+++ b/drivers/net/ethernet/hisilicon/hisi_femac.c
@@ -774,7 +774,7 @@ static int hisi_femac_drv_probe(struct platform_device *pdev)
 	struct phy_device *phy;
 	int ret;
 
-	ndev = alloc_etherdev(sizeof(*priv));
+	ndev = devm_alloc_etherdev(dev, sizeof(*priv));
 	if (!ndev)
 		return -ENOMEM;
 
@@ -788,26 +788,26 @@ static int hisi_femac_drv_probe(struct platform_device *pdev)
 	priv->port_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->port_base)) {
 		ret = PTR_ERR(priv->port_base);
-		goto out_free_netdev;
+		goto out_return;
 	}
 
 	priv->glb_base = devm_platform_ioremap_resource(pdev, 1);
 	if (IS_ERR(priv->glb_base)) {
 		ret = PTR_ERR(priv->glb_base);
-		goto out_free_netdev;
+		goto out_return;
 	}
 
 	priv->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(priv->clk)) {
 		dev_err(dev, "failed to get clk\n");
 		ret = -ENODEV;
-		goto out_free_netdev;
+		goto out_return;
 	}
 
 	ret = clk_prepare_enable(priv->clk);
 	if (ret) {
 		dev_err(dev, "failed to enable clk %d\n", ret);
-		goto out_free_netdev;
+		goto out_return;
 	}
 
 	priv->mac_rst = devm_reset_control_get(dev, "mac");
@@ -887,9 +887,7 @@ static int hisi_femac_drv_probe(struct platform_device *pdev)
 	phy_disconnect(phy);
 out_disable_clk:
 	clk_disable_unprepare(priv->clk);
-out_free_netdev:
-	free_netdev(ndev);
-
+out_return:
 	return ret;
 }
 
@@ -903,7 +901,6 @@ static void hisi_femac_drv_remove(struct platform_device *pdev)
 
 	phy_disconnect(ndev->phydev);
 	clk_disable_unprepare(priv->clk);
-	free_netdev(ndev);
 }
 
 #ifdef CONFIG_PM
-- 
2.34.1

[PATCH net v3 5/7] net: hix5hd2: manage the netdev lifetime with devres

[Jiale Yao]

hix5hd2_dev_remove() manually frees the netdev before devres releases
the IRQ. The interrupt handler receives that netdev as its data pointer
and can access it during this teardown window.

Allocate the netdev through devres so its later registered IRQ is
released first.

This issue was found by a static analysis method used in our research.

Fixes: 57c5bc9ad7d7 ("net: hisilicon: add hix5hd2 mac driver")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
index 02282dc86faf..ffcb281230eb 100644
--- a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
@@ -1100,7 +1100,7 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 	struct mii_bus *bus;
 	int ret;
 
-	ndev = alloc_etherdev(sizeof(struct hix5hd2_priv));
+	ndev = devm_alloc_etherdev(dev, sizeof(struct hix5hd2_priv));
 	if (!ndev)
 		return -ENOMEM;
 
@@ -1115,26 +1115,26 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 	priv->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->base)) {
 		ret = PTR_ERR(priv->base);
-		goto out_free_netdev;
+		goto out_return;
 	}
 
 	priv->ctrl_base = devm_platform_ioremap_resource(pdev, 1);
 	if (IS_ERR(priv->ctrl_base)) {
 		ret = PTR_ERR(priv->ctrl_base);
-		goto out_free_netdev;
+		goto out_return;
 	}
 
 	priv->mac_core_clk = devm_clk_get(&pdev->dev, "mac_core");
 	if (IS_ERR(priv->mac_core_clk)) {
 		netdev_err(ndev, "failed to get mac core clk\n");
 		ret = -ENODEV;
-		goto out_free_netdev;
+		goto out_return;
 	}
 
 	ret = clk_prepare_enable(priv->mac_core_clk);
 	if (ret < 0) {
 		netdev_err(ndev, "failed to enable mac core clk %d\n", ret);
-		goto out_free_netdev;
+		goto out_return;
 	}
 
 	priv->mac_ifc_clk = devm_clk_get(&pdev->dev, "mac_ifc");
@@ -1271,9 +1271,7 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 	clk_disable_unprepare(priv->mac_ifc_clk);
 out_disable_mac_core_clk:
 	clk_disable_unprepare(priv->mac_core_clk);
-out_free_netdev:
-	free_netdev(ndev);
-
+out_return:
 	return ret;
 }
 
@@ -1291,7 +1289,6 @@ static void hix5hd2_dev_remove(struct platform_device *pdev)
 	hix5hd2_destroy_hw_desc_queue(priv);
 	of_node_put(priv->phy_node);
 	cancel_work_sync(&priv->tx_timeout_task);
-	free_netdev(ndev);
 }
 
 static const struct of_device_id hix5hd2_of_match[] = {
-- 
2.34.1

[PATCH net v3 6/7] net: ravb: release managed IRQs before freeing netdev

[Jiale Yao]

ravb_remove() frees the netdev before devres releases the managed IRQs.
The handlers use the netdev as their data pointer, so an interrupt during
that window can access freed memory. Probe error paths have the same
ordering problem.

Keep the netdev manually managed and place only the IRQ resources in a
dedicated devres group. Release the group after unregistering the netdev
and before freeing it, and release it on probe failures as well. This
keeps the existing runtime PM error handling unchanged.

This issue was found by a static analysis method used in our research.

Fixes: 32f012b8c01c ("net: ravb: Move getting/requesting IRQs in the probe() method")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/net/ethernet/renesas/ravb_main.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index ea1c7e536791..ab4703888778 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2963,28 +2963,35 @@ static int ravb_probe(struct platform_device *pdev)
 		priv->num_rx_ring[RAVB_NC] = NC_RX_RING_SIZE;
 	}
 
+	if (!devres_open_group(&pdev->dev, priv, GFP_KERNEL)) {
+		error = -ENOMEM;
+		goto out_reset_assert;
+	}
+
 	error = ravb_setup_irqs(priv);
 	if (error)
-		goto out_reset_assert;
+		goto out_release_irq_group;
+
+	devres_close_group(&pdev->dev, priv);
 
 	priv->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(priv->clk)) {
 		error = PTR_ERR(priv->clk);
-		goto out_reset_assert;
+		goto out_release_irq_group;
 	}
 
 	if (info->gptp_ref_clk) {
 		priv->gptp_clk = devm_clk_get(&pdev->dev, "gptp");
 		if (IS_ERR(priv->gptp_clk)) {
 			error = PTR_ERR(priv->gptp_clk);
-			goto out_reset_assert;
+			goto out_release_irq_group;
 		}
 	}
 
 	priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
 	if (IS_ERR(priv->refclk)) {
 		error = PTR_ERR(priv->refclk);
-		goto out_reset_assert;
+		goto out_release_irq_group;
 	}
 	clk_prepare(priv->refclk);
 
@@ -3124,6 +3131,8 @@ static int ravb_probe(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	clk_unprepare(priv->refclk);
+out_release_irq_group:
+	devres_release_group(&pdev->dev, priv);
 out_reset_assert:
 	reset_control_assert(rstc);
 out_free_netdev:
@@ -3144,6 +3153,7 @@ static void ravb_remove(struct platform_device *pdev)
 		return;
 
 	unregister_netdev(ndev);
+	devres_release_group(dev, priv);
 	if (info->nc_queues)
 		netif_napi_del(&priv->napi[RAVB_NC]);
 	netif_napi_del(&priv->napi[RAVB_BE]);
-- 
2.34.1

[PATCH net v3 7/7] net: sxgbe: manage IRQ data lifetimes with devres

[Jiale Yao]

sxgbe_drv_remove() frees the netdev and hardware operations while
managed IRQs remain registered until the remove callback returns. The
handlers dereference these objects, so an interrupt in that window can
access freed memory.

Allocate both objects with devres. They are acquired before the IRQs and
are consequently released only after the IRQ resources have been
removed.

This issue was found by a static analysis method used in our research.

Fixes: 1edb9ca69e8a ("net: sxgbe: add basic framework for Samsung 10Gb ethernet driver")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 .../net/ethernet/samsung/sxgbe/sxgbe_main.c   | 26 +++++++------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c b/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c
index 70cf3619555f..ada851477302 100644
--- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c
+++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c
@@ -2007,7 +2007,7 @@ static int sxgbe_hw_init(struct sxgbe_priv_data * const priv)
 {
 	u32 ctrl_ids;
 
-	priv->hw = kmalloc_obj(*priv->hw);
+	priv->hw = devm_kmalloc(priv->device, sizeof(*priv->hw), GFP_KERNEL);
 	if(!priv->hw)
 		return -ENOMEM;
 
@@ -2058,7 +2058,7 @@ static int sxgbe_sw_reset(void __iomem *addr)
  * @plat_dat: platform data pointer
  * @addr: iobase memory address
  * Description: this is the main probe function used to
- * call the alloc_etherdev, allocate the priv structure.
+ * allocate the netdev and priv structure.
  */
 struct sxgbe_priv_data *sxgbe_drv_probe(struct device *device,
 					struct sxgbe_plat_data *plat_dat,
@@ -2069,8 +2069,8 @@ struct sxgbe_priv_data *sxgbe_drv_probe(struct device *device,
 	int ret;
 	u8 queue_num;
 
-	ndev = alloc_etherdev_mqs(sizeof(struct sxgbe_priv_data),
-				  SXGBE_TX_QUEUES, SXGBE_RX_QUEUES);
+	ndev = devm_alloc_etherdev_mqs(device, sizeof(struct sxgbe_priv_data),
+				       SXGBE_TX_QUEUES, SXGBE_RX_QUEUES);
 	if (!ndev)
 		return NULL;
 
@@ -2086,7 +2086,7 @@ struct sxgbe_priv_data *sxgbe_drv_probe(struct device *device,
 
 	ret = sxgbe_sw_reset(priv->ioaddr);
 	if (ret)
-		goto error_free_netdev;
+		goto error_return;
 
 	/* Verify driver arguments */
 	sxgbe_verify_args();
@@ -2094,16 +2094,16 @@ struct sxgbe_priv_data *sxgbe_drv_probe(struct device *device,
 	/* Init MAC and get the capabilities */
 	ret = sxgbe_hw_init(priv);
 	if (ret)
-		goto error_free_netdev;
+		goto error_return;
 
 	/* allocate memory resources for Descriptor rings */
 	ret = txring_mem_alloc(priv);
 	if (ret)
-		goto error_free_hw;
+		goto error_return;
 
 	ret = rxring_mem_alloc(priv);
 	if (ret)
-		goto error_free_hw;
+		goto error_return;
 
 	ndev->netdev_ops = &sxgbe_netdev_ops;
 
@@ -2191,11 +2191,7 @@ struct sxgbe_priv_data *sxgbe_drv_probe(struct device *device,
 	clk_put(priv->sxgbe_clk);
 error_napi_del:
 	netif_napi_del(&priv->napi);
-error_free_hw:
-	kfree(priv->hw);
-error_free_netdev:
-	free_netdev(ndev);
-
+error_return:
 	return NULL;
 }
 
@@ -2229,10 +2225,6 @@ void sxgbe_drv_remove(struct net_device *ndev)
 	clk_put(priv->sxgbe_clk);
 
 	netif_napi_del(&priv->napi);
-
-	kfree(priv->hw);
-
-	free_netdev(ndev);
 }
 
 #ifdef CONFIG_PM
-- 
2.34.1

Re: [PATCH net v3 1/7] net: macb: manage the netdev lifetime with devres

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v3 6/7] net: ravb: release managed IRQs before freeing netdev

[Niklas Söderlund]

Hi Jiale,

On 2026-10-03 16:59:37 +0800, Jiale Yao wrote:
> ravb_remove() frees the netdev before devres releases the managed IRQs.
> The handlers use the netdev as their data pointer, so an interrupt during
> that window can access freed memory. Probe error paths have the same
> ordering problem.
> 
> Keep the netdev manually managed and place only the IRQ resources in a
> dedicated devres group. Release the group after unregistering the netdev
> and before freeing it, and release it on probe failures as well. This
> keeps the existing runtime PM error handling unchanged.
> 
> This issue was found by a static analysis method used in our research.

What happened to switching to use devm_alloc_etherdev_mqs() instead of 
adding this complex thing, as we discussed in v2?

Nacked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> 
> Fixes: 32f012b8c01c ("net: ravb: Move getting/requesting IRQs in the probe() method")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  drivers/net/ethernet/renesas/ravb_main.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index ea1c7e536791..ab4703888778 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -2963,28 +2963,35 @@ static int ravb_probe(struct platform_device *pdev)
>  		priv->num_rx_ring[RAVB_NC] = NC_RX_RING_SIZE;
>  	}
>  
> +	if (!devres_open_group(&pdev->dev, priv, GFP_KERNEL)) {
> +		error = -ENOMEM;
> +		goto out_reset_assert;
> +	}
> +
>  	error = ravb_setup_irqs(priv);
>  	if (error)
> -		goto out_reset_assert;
> +		goto out_release_irq_group;
> +
> +	devres_close_group(&pdev->dev, priv);
>  
>  	priv->clk = devm_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(priv->clk)) {
>  		error = PTR_ERR(priv->clk);
> -		goto out_reset_assert;
> +		goto out_release_irq_group;
>  	}
>  
>  	if (info->gptp_ref_clk) {
>  		priv->gptp_clk = devm_clk_get(&pdev->dev, "gptp");
>  		if (IS_ERR(priv->gptp_clk)) {
>  			error = PTR_ERR(priv->gptp_clk);
> -			goto out_reset_assert;
> +			goto out_release_irq_group;
>  		}
>  	}
>  
>  	priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
>  	if (IS_ERR(priv->refclk)) {
>  		error = PTR_ERR(priv->refclk);
> -		goto out_reset_assert;
> +		goto out_release_irq_group;
>  	}
>  	clk_prepare(priv->refclk);
>  
> @@ -3124,6 +3131,8 @@ static int ravb_probe(struct platform_device *pdev)
>  	pm_runtime_disable(&pdev->dev);
>  	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  	clk_unprepare(priv->refclk);
> +out_release_irq_group:
> +	devres_release_group(&pdev->dev, priv);
>  out_reset_assert:
>  	reset_control_assert(rstc);
>  out_free_netdev:
> @@ -3144,6 +3153,7 @@ static void ravb_remove(struct platform_device *pdev)
>  		return;
>  
>  	unregister_netdev(ndev);
> +	devres_release_group(dev, priv);
>  	if (info->nc_queues)
>  		netif_napi_del(&priv->napi[RAVB_NC]);
>  	netif_napi_del(&priv->napi[RAVB_BE]);
> -- 
> 2.34.1
> 

-- 
Kind Regards,
Niklas Söderlund

Re:Re: [PATCH net v3 6/7] net: ravb: release managed IRQs before freeing netdev

[jiale yao]

At 2026-10-03 17:59:21, "Niklas Söderlund" <niklas.soderlund@ragnatech.se> wrote:
>Hi Jiale,
>
>On 2026-10-03 16:59:37 +0800, Jiale Yao wrote:
>> ravb_remove() frees the netdev before devres releases the managed IRQs.
>> The handlers use the netdev as their data pointer, so an interrupt during
>> that window can access freed memory. Probe error paths have the same
>> ordering problem.
>> 
>> Keep the netdev manually managed and place only the IRQ resources in a
>> dedicated devres group. Release the group after unregistering the netdev
>> and before freeing it, and release it on probe failures as well. This
>> keeps the existing runtime PM error handling unchanged.
>> 
>> This issue was found by a static analysis method used in our research.
>
>What happened to switching to use devm_alloc_etherdev_mqs() instead of 
>adding this complex thing, as we discussed in v2?

You're right. I missed this point while working through a large number
of patches...>_<
I should take a break and post an updated revision after the
24-hour waiting period.

>
>Nacked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>
>> 
>> Fixes: 32f012b8c01c ("net: ravb: Move getting/requesting IRQs in the probe() method")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jiale Yao <yaojiale02@163.com>
>> ---
>>  drivers/net/ethernet/renesas/ravb_main.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>> index ea1c7e536791..ab4703888778 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -2963,28 +2963,35 @@ static int ravb_probe(struct platform_device *pdev)
>>  		priv->num_rx_ring[RAVB_NC] = NC_RX_RING_SIZE;
>>  	}
>>  
>> +	if (!devres_open_group(&pdev->dev, priv, GFP_KERNEL)) {
>> +		error = -ENOMEM;
>> +		goto out_reset_assert;
>> +	}
>> +
>>  	error = ravb_setup_irqs(priv);
>>  	if (error)
>> -		goto out_reset_assert;
>> +		goto out_release_irq_group;
>> +
>> +	devres_close_group(&pdev->dev, priv);
>>  
>>  	priv->clk = devm_clk_get(&pdev->dev, NULL);
>>  	if (IS_ERR(priv->clk)) {
>>  		error = PTR_ERR(priv->clk);
>> -		goto out_reset_assert;
>> +		goto out_release_irq_group;
>>  	}
>>  
>>  	if (info->gptp_ref_clk) {
>>  		priv->gptp_clk = devm_clk_get(&pdev->dev, "gptp");
>>  		if (IS_ERR(priv->gptp_clk)) {
>>  			error = PTR_ERR(priv->gptp_clk);
>> -			goto out_reset_assert;
>> +			goto out_release_irq_group;
>>  		}
>>  	}
>>  
>>  	priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
>>  	if (IS_ERR(priv->refclk)) {
>>  		error = PTR_ERR(priv->refclk);
>> -		goto out_reset_assert;
>> +		goto out_release_irq_group;
>>  	}
>>  	clk_prepare(priv->refclk);
>>  
>> @@ -3124,6 +3131,8 @@ static int ravb_probe(struct platform_device *pdev)
>>  	pm_runtime_disable(&pdev->dev);
>>  	pm_runtime_dont_use_autosuspend(&pdev->dev);
>>  	clk_unprepare(priv->refclk);
>> +out_release_irq_group:
>> +	devres_release_group(&pdev->dev, priv);
>>  out_reset_assert:
>>  	reset_control_assert(rstc);
>>  out_free_netdev:
>> @@ -3144,6 +3153,7 @@ static void ravb_remove(struct platform_device *pdev)
>>  		return;
>>  
>>  	unregister_netdev(ndev);
>> +	devres_release_group(dev, priv);
>>  	if (info->nc_queues)
>>  		netif_napi_del(&priv->napi[RAVB_NC]);
>>  	netif_napi_del(&priv->napi[RAVB_BE]);
>> -- 
>> 2.34.1
>> 
>
>-- 
>Kind Regards,
>Niklas Söderlund

Re: [PATCH net v3 1/7] net: macb: manage the netdev lifetime with devres

[Théo Lebrun]

Hello Jiale,

Those LLM bugs are code churn, that's why you are seeing pushback.
Please don't ignore the pushback. For example on V2 you got asked to
reply to an automated message, which you didn't do.

https://lore.kernel.org/netdev/20260927153020.5311dba6@kernel.org/

On Sat Oct 3, 2026 at 10:59 AM CEST, Jiale Yao wrote:
> macb_remove() frees the netdev while its managed IRQs are only
> released after the remove callback returns. An interrupt in that window
> can dereference the freed netdev or queue data.

Please indicate how an interrupt could land in that window.
Thinking about it for a brief instant, I cannot think of one.

> Allocate the netdev with devres as well. Since the IRQs are registered
> later, devres releases them before freeing the netdev and closes the
> lifetime gap.
>
> This issue was found by a static analysis method used in our research.
>
> Fixes: 0a4acf08ea62 ("net: macb: Use devm_request_irq()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)

Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>

I still give my Rb because the patch is valid. The wasted time is on net
maintainers though; they'll decide if they want it or not.

For this MACB patch, it could land in net-next as I don't see a
practical bug here (in light of the recent pushback about the # of
fixes in net).

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH net v3 1/7] net: macb: manage the netdev lifetime with devres

[jiale yao]

At 2026-10-04 16:45:58, "Théo Lebrun" <theo.lebrun@bootlin.com> wrote:
>Hello Jiale,
>
>Those LLM bugs are code churn, that's why you are seeing pushback.
>Please don't ignore the pushback. For example on V2 you got asked to
>reply to an automated message, which you didn't do.
>
>https://lore.kernel.org/netdev/20260927153020.5311dba6@kernel.org/
>
>On Sat Oct 3, 2026 at 10:59 AM CEST, Jiale Yao wrote:
>> macb_remove() frees the netdev while its managed IRQs are only
>> released after the remove callback returns. An interrupt in that window
>> can dereference the freed netdev or queue data.
>
>Please indicate how an interrupt could land in that window.
>Thinking about it for a brief instant, I cannot think of one.

I reproduced this on QEMU aarch64 virt + KASAN:
I added a macb node via an extended device tree, with its interrupt
line shared with virtio-rng. Keeping the rng busy triggers a steady
stream of interrupts during unbind/rebind, and macb_interrupt() hits the
window after `free_netdev()` on the first attempt. 

---
[    4.457077] BUG: KASAN: use-after-free in macb_interrupt+0xc40/0x1074
[    4.457569] Read of size 8 at addr ffff00000f8acac8 by task poc/82
[    4.457614] 
[    4.457938] CPU: 0 UID: 0 PID: 82 Comm: poc Not tainted 7.3.0-rc4 #1 PREEMPT 
[    4.458025] Hardware name: linux,dummy-virt (DT)
[    4.458167] Call trace:
[    4.458258]  show_stack+0x18/0x24 (C)
[    4.458321]  dump_stack_lvl+0x78/0x90
[    4.458340]  print_report+0x114/0x5cc
[    4.458353]  kasan_report+0xa4/0xf0
[    4.458363]  __asan_report_load8_noabort+0x20/0x2c
[    4.458374]  macb_interrupt+0xc40/0x1074
[    4.458385]  __handle_irq_event_percpu+0xc8/0x340
[    4.458398]  handle_irq_event+0xb0/0x1d8
[    4.458407]  handle_fasteoi_irq+0x298/0x6a0
[    4.458419]  handle_irq_desc+0xc4/0x104
[    4.458454]  generic_handle_domain_irq+0x18/0x24
[    4.458485]  gic_handle_irq+0x54/0x194
[    4.458498]  call_on_irq_stack+0x30/0x48
[    4.458511]  do_interrupt_handler+0xf0/0x130
[    4.458523]  el1_interrupt+0x3c/0x60
[    4.458538]  el1h_64_irq_handler+0x18/0x24
[    4.458550]  el1h_64_irq+0x6c/0x70
[    4.458619]  get_pfnblock_migratetype+0xd0/0x144 (P)
[    4.458638]  __free_frozen_pages+0x2d8/0xec4
[    4.458650]  free_frozen_pages+0x14/0x20
[    4.458661]  free_large_kmalloc+0xa0/0x120
[    4.458674]  kfree+0x84/0x424
[    4.458684]  kvfree+0x3c/0x4c
[    4.458694]  netdev_release+0x70/0x98
[    4.458707]  device_release+0x104/0x210
[    4.458720]  kobject_put+0x140/0x240
[    4.458731]  put_device+0x14/0x24
[    4.458740]  free_netdev+0x414/0x6c4
[    4.458752]  macb_remove+0x14c/0x19c
[    4.458763]  platform_remove+0x58/0x78
[    4.458774]  device_remove+0xb0/0x14c
[    4.458785]  device_release_driver_internal+0x2fc/0x468
[    4.458795]  device_driver_detach+0x3c/0x54
[    4.458804]  unbind_store+0xec/0x100
[    4.458814]  drv_attr_store+0x60/0x9c
[    4.458824]  sysfs_kf_write+0x170/0x1e8
[    4.458838]  kernfs_fop_write_iter+0x298/0x404
[    4.458848]  vfs_write+0x648/0x8cc
[    4.458859]  ksys_write+0xf0/0x1e0
[    4.458868]  __arm64_sys_write+0x70/0xa0
[    4.458877]  invoke_syscall+0x70/0x24c
[    4.458887]  el0_svc_common.constprop.0+0xa8/0x22c
[    4.458896]  do_el0_svc+0x44/0x5c
[    4.458905]  el0_svc+0x58/0xd0
[    4.458917]  el0t_64_sync_handler+0xa0/0xe4
[    4.458929]  el0t_64_sync+0x198/0x19c
[    4.459007] 
[    4.459053] The buggy address belongs to the physical page:
[    4.459261] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x4f8ac
[    4.459384] flags: 0x3fffe0000000000(node=0|zone=0|lastcpupid=0x1ffff)
[    4.459721] raw: 03fffe0000000000 0000000000000000 dead000000000122 0000000000000000
[    4.459737] raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
[    4.459781] page dumped because: kasan: bad access detected
[    4.459792] 
[    4.459799] Memory state around the buggy address:
[    4.459912]  ffff00000f8ac980: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[    4.459935]  ffff00000f8aca00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[    4.459951] >ffff00000f8aca80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[    4.459962]                                               ^
[    4.459996]  ffff00000f8acb00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[    4.460002]  ffff00000f8acb80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[    4.460020] ==================================================================
---
With the fix applied, several thousand cycles produce no report at all.
>
>> Allocate the netdev with devres as well. Since the IRQs are registered
>> later, devres releases them before freeing the netdev and closes the
>> lifetime gap.
>>
>> This issue was found by a static analysis method used in our research.
>>
>> Fixes: 0a4acf08ea62 ("net: macb: Use devm_request_irq()")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jiale Yao <yaojiale02@163.com>
>> ---
>>  drivers/net/ethernet/cadence/macb_main.c | 17 +++++++----------
>>  1 file changed, 7 insertions(+), 10 deletions(-)
>
>Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>
>
>I still give my Rb because the patch is valid. The wasted time is on net
>maintainers though; they'll decide if they want it or not.
>
>For this MACB patch, it could land in net-next as I don't see a
>practical bug here (in light of the recent pushback about the # of
>fixes in net).
>
>Thanks,
>
>--
>Théo Lebrun, Bootlin
>Embedded Linux and Kernel engineering
>https://bootlin.com

Re: [PATCH net v3 1/7] net: macb: manage the netdev lifetime with devres

[Théo Lebrun]

Hello Jiale,

On Sun Oct 4, 2026 at 2:14 PM CEST, jiale yao wrote:
> At 2026-10-04 16:45:58, "Théo Lebrun" <theo.lebrun@bootlin.com> wrote:
>>Hello Jiale,
>>
>>Those LLM bugs are code churn, that's why you are seeing pushback.
>>Please don't ignore the pushback. For example on V2 you got asked to
>>reply to an automated message, which you didn't do.
>>
>>https://lore.kernel.org/netdev/20260927153020.5311dba6@kernel.org/

You've skipped over part of my message.

>>On Sat Oct 3, 2026 at 10:59 AM CEST, Jiale Yao wrote:
>>> macb_remove() frees the netdev while its managed IRQs are only
>>> released after the remove callback returns. An interrupt in that window
>>> can dereference the freed netdev or queue data.
>>
>>Please indicate how an interrupt could land in that window.
>>Thinking about it for a brief instant, I cannot think of one.
>
> I reproduced this on QEMU aarch64 virt + KASAN:
> I added a macb node via an extended device tree, with its interrupt
> line shared with virtio-rng. Keeping the rng busy triggers a steady
> stream of interrupts during unbind/rebind, and macb_interrupt() hits the
> window after `free_netdev()` on the first attempt. 

But that never happens in the real world. We use shared interrupts
because we want to supports boards with a single interrupt lane for
all queues.

I've been thinking about removing the IRQF_SHARED flag recently, because
it tricks Sashiko (and you) into thinking that we might share our IRQ
lane with anything and anyone.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com