[PATCH 3/4] media: mediatek: vcodec: Fix potential crash in mtk_vcodec_dbgfs_remove()

[Nicolas Dufresne]

Le mercredi 14 juin 2023 à 16:07 +0300, Dan Carpenter a écrit :
> The list iterator "dbgfs_inst" is always non-NULL.  This means that the
> test for NULL inside the loop is unnecessary and it also means that the
> test for NULL outside the loop will not work.  If we do not find the item
> on the list with the correct the ctx_id then it will free invalid memory
> leading to a crash.
> 
> Fixes: cd403a6a0419 ("media: mediatek: vcodec: Add a debugfs file to get different useful information")

Clearly better.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>

> Signed-off-by: Dan Carpenter <dan.carpenter at linaro.org>
> ---
>  .../media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> index 2151c3967684..2ebf68d33d57 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> @@ -166,16 +166,13 @@ void mtk_vcodec_dbgfs_remove(struct mtk_vcodec_dev *vcodec_dev, int ctx_id)
>  	struct mtk_vcodec_dbgfs_inst *dbgfs_inst;
>  
>  	list_for_each_entry(dbgfs_inst, &vcodec_dev->dbgfs.dbgfs_head, node) {
> -		if (dbgfs_inst && dbgfs_inst->inst_id == ctx_id) {
> +		if (dbgfs_inst->inst_id == ctx_id) {
>  			vcodec_dev->dbgfs.inst_count--;
> -			break;
> +			list_del(&dbgfs_inst->node);
> +			kfree(dbgfs_inst);
> +			return;
>  		}
>  	}
> -
> -	if (dbgfs_inst) {
> -		list_del(&dbgfs_inst->node);
> -		kfree(dbgfs_inst);
> -	}
>  }
>  EXPORT_SYMBOL_GPL(mtk_vcodec_dbgfs_remove);
>