[PATCH v2 0/3] media: mediatek: unregister VPU watchdog handlers

[PATCH v2 0/3] media: mediatek: unregister VPU watchdog handlers

[Guangshuo Li]

The Mediatek vcodec and MDP drivers register watchdog reset handlers
with the VPU driver and pass driver-owned objects as private data.

These handlers can remain registered after an initialization failure or
device removal. A later VPU watchdog reset can therefore invoke a
handler with stale private data and potentially dereference freed
memory.

This series adds a common helper for unregistering VPU watchdog
handlers, then uses it in the vcodec and MDP cleanup paths before their
VPU device references are released.

The series is organized as follows:

1. Add vpu_wdt_unreg_handler() to the VPU driver.
2. Unregister the vcodec watchdog handler during initialization
   failure and firmware release.
3. Unregister the MDP watchdog handler during probe failure and
   device removal.

v2:

  - Split the changes into three patches as requested by Nicolas.
  - Fix the extra blank line reported by checkpatch.
  - Carry Nicolas' Reviewed-by tag on the MDP patch.

Guangshuo Li (3):
media: mediatek: vpu: add watchdog handler unregister helper
media: mediatek: vcodec: unregister VPU watchdog handler
media: mtk-mdp: unregister VPU watchdog handler

drivers/media/platform/mediatek/mdp/mtk_mdp_core.c |  2 ++
.../mediatek/vcodec/common/mtk_vcodec_fw_vpu.c     |  6 ++++++
drivers/media/platform/mediatek/vpu/mtk_vpu.c      | 14 ++++++++++++++
drivers/media/platform/mediatek/vpu/mtk_vpu.h      |  2 ++
4 files changed, 24 insertions(+)

--
2.43.0

[PATCH v2 1/3] media: mediatek: vpu: add watchdog handler unregister helper

[Guangshuo Li]

VPU clients register watchdog reset handlers with private data owned by
the client device. The VPU driver currently provides no way to clear a
handler when client initialization fails or when the client device is
removed.

Add vpu_wdt_unreg_handler() to clear the reset callback and its private
data while holding the VPU mutex. This allows clients to remove stale
watchdog entries before releasing their device references.

Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/media/platform/mediatek/vpu/mtk_vpu.c | 14 ++++++++++++++
 drivers/media/platform/mediatek/vpu/mtk_vpu.h |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.c b/drivers/media/platform/mediatek/vpu/mtk_vpu.c
index 8d8319f0cd22..a2b937095248 100644
--- a/drivers/media/platform/mediatek/vpu/mtk_vpu.c
+++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.c
@@ -437,6 +437,20 @@ int vpu_wdt_reg_handler(struct platform_device *pdev,
 }
 EXPORT_SYMBOL_GPL(vpu_wdt_reg_handler);
 
+void vpu_wdt_unreg_handler(struct platform_device *pdev, enum rst_id id)
+{
+	struct mtk_vpu *vpu = platform_get_drvdata(pdev);
+
+	if (!vpu || id >= VPU_RST_MAX)
+		return;
+
+	mutex_lock(&vpu->vpu_mutex);
+	vpu->wdt.handler[id].reset_func = NULL;
+	vpu->wdt.handler[id].priv = NULL;
+	mutex_unlock(&vpu->vpu_mutex);
+}
+EXPORT_SYMBOL_GPL(vpu_wdt_unreg_handler);
+
 unsigned int vpu_get_vdec_hw_capa(struct platform_device *pdev)
 {
 	struct mtk_vpu *vpu = platform_get_drvdata(pdev);
diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.h b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
index 3951547e9ec5..2ccb481a04cf 100644
--- a/drivers/media/platform/mediatek/vpu/mtk_vpu.h
+++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
@@ -141,6 +141,8 @@ int vpu_wdt_reg_handler(struct platform_device *pdev,
 			void vpu_wdt_reset_func(void *priv),
 			void *priv, enum rst_id id);
 
+void vpu_wdt_unreg_handler(struct platform_device *pdev, enum rst_id id);
+
 /**
  * vpu_get_vdec_hw_capa - get video decoder hardware capability
  *
-- 
2.43.0

[PATCH v2 2/3] media: mediatek: vcodec: unregister VPU watchdog handler

[Guangshuo Li]

mtk_vcodec_fw_vpu_init() registers a VPU watchdog reset handler and
passes the vcodec device as the private data.

The handler is stored in the VPU device and can outlive the vcodec
device. If firmware initialization fails after the handler has been
registered, or if the firmware object is later released, the watchdog
table can retain a pointer to a vcodec device that is about to be
released. A later watchdog reset can then call the reset handler with a
stale pointer and dereference freed memory while walking the vcodec
context list.

Unregister the watchdog handler in the firmware initialization failure
path and in the firmware release path before dropping the VPU device
reference.

Fixes: 01abf5fbb081 ("media: mediatek: vcodec: separate struct 'mtk_vcodec_ctx'")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 .../platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c     | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
index 3632037f78f5..ee50a794077f 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
@@ -40,6 +40,11 @@ static int mtk_vcodec_vpu_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
 
 static void mtk_vcodec_vpu_release(struct mtk_vcodec_fw *fw)
 {
+	enum rst_id rst_id;
+
+	rst_id = fw->fw_use == ENCODER ? VPU_RST_ENC : VPU_RST_DEC;
+	vpu_wdt_unreg_handler(fw->pdev, rst_id);
+
 	put_device(&fw->pdev->dev);
 }
 
@@ -120,6 +125,7 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_vpu_init(void *priv, enum mtk_vcodec_fw_use
 
 	fw = devm_kzalloc(&plat_dev->dev, sizeof(*fw), GFP_KERNEL);
 	if (!fw) {
+		vpu_wdt_unreg_handler(fw_pdev, rst_id);
 		put_device(&fw_pdev->dev);
 		return ERR_PTR(-ENOMEM);
 	}
-- 
2.43.0

[PATCH v2 3/3] media: mtk-mdp: unregister VPU watchdog handler

[Guangshuo Li]

mtk_mdp_probe() registers the MDP reset handler with the VPU watchdog
code and passes the MDP device as the private data.

The handler is stored in the VPU device and can outlive the MDP probe or
remove path. If probe fails after the handler has been registered, or if
the MDP device is removed, the watchdog table can retain a pointer to an
MDP object that is about to be released. A later VPU watchdog reset can
then call mtk_mdp_reset_handler() with a stale pointer and dereference
freed memory while queuing the MDP watchdog work.

Unregister the watchdog handler in the probe error path and in the
remove path before dropping the VPU device reference.

Fixes: ee18fc7b0b95 ("media: mtk-mdp: handle vpu_wdt_reg_handler() errors during probe")
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/media/platform/mediatek/mdp/mtk_mdp_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/mediatek/mdp/mtk_mdp_core.c b/drivers/media/platform/mediatek/mdp/mtk_mdp_core.c
index 8432833814f3..5ccb7aa925d9 100644
--- a/drivers/media/platform/mediatek/mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mediatek/mdp/mtk_mdp_core.c
@@ -221,6 +221,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
 	return 0;
 
 err_reg_handler:
+	vpu_wdt_unreg_handler(mdp->vpu_dev, VPU_RST_MDP);
 	platform_device_put(mdp->vpu_dev);
 
 err_vpu_get_dev:
@@ -254,6 +255,7 @@ static void mtk_mdp_remove(struct platform_device *pdev)
 
 	pm_runtime_disable(&pdev->dev);
 	vb2_dma_contig_clear_max_seg_size(&pdev->dev);
+	vpu_wdt_unreg_handler(mdp->vpu_dev, VPU_RST_MDP);
 	platform_device_put(mdp->vpu_dev);
 	mtk_mdp_unregister_m2m_device(mdp);
 	v4l2_device_unregister(&mdp->v4l2_dev);
-- 
2.43.0