Re: [PATCH] media: mediatek: vcodec: unregister VPU watchdog handler

[Nicolas Dufresne]

Hi,

Le mercredi 08 juillet 2026 à 20:42 +0800, Guangshuo Li a écrit :
> 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 vcodec device is later removed, the VPU watchdog
> table can keep a pointer to a vcodec device that is about to be released.
> A later VPU watchdog reset can then call the reset handler with a stale
> pointer and dereference freed memory while walking the vcodec context
> list.
> 
> Add a VPU watchdog unregister helper and clear the vcodec watchdog

The helper is already added in previous patch.

> handler from both the firmware initialization failure path and the VPU
> firmware release path before dropping the VPU device reference.

I'm changing my mind. Please resubmit with proper commit message, and make this
a series of 3 patches.

- 1/3 Add vpu_wdt_unreg_handler to vpu driver
- 2/3 Fix WPU watchdog
- 3/3 Fix MDP watchdog

Nicolas

> 
> Fixes: 01abf5fbb081 ("media: mediatek: vcodec: separate struct 'mtk_vcodec_ctx'")
> Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> ---
>  .../mediatek/vcodec/common/mtk_vcodec_fw_vpu.c    |  4 ++++
>  	    | 15 +++++++++++++++
>  drivers/media/platform/mediatek/vpu/mtk_vpu.h     |  2 ++
>  3 files changed, 21 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..c79a78c371ea 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,9 @@ 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)
>  {
> +	vpu_wdt_unreg_handler(fw->pdev,
> +			      fw->fw_use == ENCODER ? VPU_RST_ENC :
> +				       VPU_RST_DEC);
>  	put_device(&fw->pdev->dev);
>  }
>  
> @@ -120,6 +123,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);
>  	}
> diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.c b/drivers/media/platform/mediatek/vpu/mtk_vpu.c
> index 8d8319f0cd22..f59f45fecc75 100644
> --- a/drivers/media/platform/mediatek/vpu/mtk_vpu.c
> +++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.c
> @@ -437,6 +437,21 @@ 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
>   *

Attachment: signature.asc
Description: This is a digitally signed message part