Skip to contentSkip to conversation

fix possible deadlock during WED attach

PATCH Applied Lorenzo Bianconi 3 participants 6 messages Dec 7, 2022 – Dec 9, 2022

Subsystems (4)
Networking Network drivers Arm MediaTek MediaTek Ethernet driver
lore
Share & tools
lore

Lorenzo Bianconi

author maintainer
#0 lore raw

v3 cover fix possible deadlock during WED attach

Fix a possible deadlock in mtk_wed_attach if mtk_wed_wo_init routine fails. Check wo pointer is properly allocated before running mtk_wed_wo_reset() and mtk_wed_wo_deinit().

Changes sice v2:
- add WARN_ON in mtk_wed_mcu_msg_update()
- split in two patches
Changes since v1:
- move wo pointer checks in __mtk_wed_detach()

Lorenzo Bianconi (2):
net: ethernet: mtk_wed: fix some possible NULL pointer dereferences
net: ethernet: mtk_wed: fix possible deadlock if mtk_wed_wo_init fails

 drivers/net/ethernet/mediatek/mtk_wed.c     | 30 ++++++++++++++-------
 drivers/net/ethernet/mediatek/mtk_wed_mcu.c |  3 +++
 2 files changed, 23 insertions(+), 10 deletions(-)
Signature · 1 line
2.38.1
author maintainer
#1 lore raw

PATCH v3 1/2 net: ethernet: mtk_wed: fix some possible NULL pointer dereferences

Fix possible NULL pointer dereference in mtk_wed_detach routine checking wo pointer is properly allocated before running mtk_wed_wo_reset() and mtk_wed_wo_deinit().
Even if it is just a theoretical issue at the moment check wo pointer is not NULL in mtk_wed_mcu_msg_update.
Moreover, honor mtk_wed_mcu_send_msg return value in mtk_wed_wo_reset()

Fixes 799684448e3e ("net: ethernet: mtk_wed: introduce wed wo support")
Fixes 4c5de09eb0d0 ("net: ethernet: mtk_wed: add configure wed wo support")
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/ethernet/mediatek/mtk_wed.c     | 13 ++++++++-----
 drivers/net/ethernet/mediatek/mtk_wed_mcu.c |  3 +++
 2 files changed, 11 insertions(+), 5 deletions(-)
drivers/net/ethernet/mediatek/mtk_wed.c +8 -5
index 06b6cc53fa02..4ef23eadd69e 100644
@@ -174,9 +174,10 @@ mtk_wed_wo_reset(struct mtk_wed_device *dev)
 	mtk_wdma_tx_reset(dev);
 	mtk_wed_reset(dev, MTK_WED_RESET_WED);
 
-	mtk_wed_mcu_send_msg(wo, MTK_WED_MODULE_ID_WO,
-			     MTK_WED_WO_CMD_CHANGE_STATE, &state,
-			     sizeof(state), false);
+	if (mtk_wed_mcu_send_msg(wo, MTK_WED_MODULE_ID_WO,
+				 MTK_WED_WO_CMD_CHANGE_STATE, &state,
+				 sizeof(state), false))
+		return;
 
 	if (readx_poll_timeout(mtk_wed_wo_read_status, dev, val,
 			       val == MTK_WED_WOIF_DISABLE_DONE,

@@ -590,9 +591,11 @@ mtk_wed_detach(struct mtk_wed_device *dev)
 	mtk_wed_free_tx_rings(dev);
 
 	if (mtk_wed_get_rx_capa(dev)) {
-		mtk_wed_wo_reset(dev);
+		if (hw->wed_wo)
+			mtk_wed_wo_reset(dev);
 		mtk_wed_free_rx_rings(dev);
-		mtk_wed_wo_deinit(hw);
+		if (hw->wed_wo)
+			mtk_wed_wo_deinit(hw);
 	}
 
 	if (dev->wlan.bus_type == MTK_WED_BUS_PCIE) {
drivers/net/ethernet/mediatek/mtk_wed_mcu.c +3 -0
index f9539e6233c9..6bad0d262f28 100644
@@ -207,6 +207,9 @@ int mtk_wed_mcu_msg_update(struct mtk_wed_device *dev, int id, void *data,
 	if (dev->hw->version == 1)
 		return 0;
 
+	if (WARN_ON(!wo))
+		return -ENODEV;
+
 	return mtk_wed_mcu_send_msg(wo, MTK_WED_MODULE_ID_WO, id, data, len,
 				    true);
 }
Signature · 1 line
2.38.1
Reviewed-by on v3 1/2
#2 lore raw

Re: [PATCH v3 net-next 1/2] net: ethernet: mtk_wed: fix some possible NULL pointer dereferences

Quoted 7 lines · Lorenzo Bianconi

Fix possible NULL pointer dereference in mtk_wed_detach routine checking wo pointer is properly allocated before running mtk_wed_wo_reset() and mtk_wed_wo_deinit().
Even if it is just a theoretical issue at the moment check wo pointer is not NULL in mtk_wed_mcu_msg_update.
Moreover, honor mtk_wed_mcu_send_msg return value in mtk_wed_wo_reset()

Quoted 8 lines
Fixes 799684448e3e ("net: ethernet: mtk_wed: introduce wed wo support")
Fixes 4c5de09eb0d0 ("net: ethernet: mtk_wed: add configure wed wo support")
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/ethernet/mediatek/mtk_wed.c     | 13 ++++++++-----
 drivers/net/ethernet/mediatek/mtk_wed_mcu.c |  3 +++
 2 files changed, 11 insertions(+), 5 deletions(-)

Thanks,

Reviewed-by Leon Romanovsky <[email protected]>
author maintainer
#3 lore raw

PATCH v3 2/2 net: ethernet: mtk_wed: fix possible deadlock if mtk_wed_wo_init fails

In reply to Lorenzo Bianconi

Introduce __mtk_wed_detach() in order to avoid a deadlock in mtk_wed_attach routine if mtk_wed_wo_init fails since both mtk_wed_attach and mtk_wed_detach run holding hw_lock mutex.

Fixes 4c5de09eb0d0 ("net: ethernet: mtk_wed: add configure wed wo support")
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/ethernet/mediatek/mtk_wed.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
drivers/net/ethernet/mediatek/mtk_wed.c +12 -5
index 4ef23eadd69e..a6271449617f 100644
@@ -577,12 +577,10 @@ mtk_wed_deinit(struct mtk_wed_device *dev)
 }
 
 static void
-mtk_wed_detach(struct mtk_wed_device *dev)
+__mtk_wed_detach(struct mtk_wed_device *dev)
 {
 	struct mtk_wed_hw *hw = dev->hw;
 
-	mutex_lock(&hw_lock);
-
 	mtk_wed_deinit(dev);
 
 	mtk_wdma_rx_reset(dev);

@@ -615,6 +613,13 @@ mtk_wed_detach(struct mtk_wed_device *dev)
 	module_put(THIS_MODULE);
 
 	hw->wed_dev = NULL;
+}
+
+static void
+mtk_wed_detach(struct mtk_wed_device *dev)
+{
+	mutex_lock(&hw_lock);
+	__mtk_wed_detach(dev);
 	mutex_unlock(&hw_lock);
 }
 

@@ -1497,8 +1502,10 @@ mtk_wed_attach(struct mtk_wed_device *dev)
 		ret = mtk_wed_wo_init(hw);
 	}
 out:
-	if (ret)
-		mtk_wed_detach(dev);
+	if (ret) {
+		dev_err(dev->hw->dev, "failed to attach wed device\n");
+		__mtk_wed_detach(dev);
+	}
 unlock:
 	mutex_unlock(&hw_lock);
Signature · 1 line
2.38.1
Reviewed-by on v3 2/2
#4 lore raw

Re: [PATCH v3 net-next 2/2] net: ethernet: mtk_wed: fix possible deadlock if mtk_wed_wo_init fails

Quoted 15 lines · Lorenzo Bianconi

Introduce __mtk_wed_detach() in order to avoid a deadlock in mtk_wed_attach routine if mtk_wed_wo_init fails since both mtk_wed_attach and mtk_wed_detach run holding hw_lock mutex.

Fixes 4c5de09eb0d0 ("net: ethernet: mtk_wed: add configure wed wo support")
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/ethernet/mediatek/mtk_wed.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
drivers/net/ethernet/mediatek/mtk_wed.c +0 -0
index 4ef23eadd69e..a6271449617f 100644

drivers/net/ethernet/mediatek/mtk_wed.c mtk_wed_deinit(struct mtk_wed_device *dev) · line 578

Quoted 2 lines
drivers/net/ethernet/mediatek/mtk_wed.c +0 -0
@@ -577,12 +577,10 @@ mtk_wed_deinit(struct mtk_wed_device *dev)
 }

Thanks,

Reviewed-by Leon Romanovsky <[email protected]>
CI Applied
#5 lore raw
In reply to Lorenzo Bianconi
This series was applied to netdev/net-next.git (master) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (master) by Jakub Kicinski <[email protected]>:

Quoted 11 lines · On Wed, 7 Dec 2022 15:04:53 +0100 you wrote:

Fix a possible deadlock in mtk_wed_attach if mtk_wed_wo_init routine fails. Check wo pointer is properly allocated before running mtk_wed_wo_reset() and mtk_wed_wo_deinit().

Changes sice v2:
- add WARN_ON in mtk_wed_mcu_msg_update()
- split in two patches
Changes since v1:
- move wo pointer checks in __mtk_wed_detach()

[...]

Here is the summary with links:
- [v3,net-next,1/2] net: ethernet: mtk_wed: fix some possible NULL pointer dereferences

    https://git.kernel.org/netdev/net-next/c/c79e0af5ae5e
  - [v3,net-next,2/2] net: ethernet: mtk_wed: fix possible deadlock if mtk_wed_wo_init fails
    https://git.kernel.org/netdev/net-next/c/587585e1bbeb

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html