diff mbox series

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

Message ID d4fffbaa-f01d-4e2e-9b1b-45d996236642@moroto.mountain (mailing list archive)
State New, archived
Headers show
Series [1/4] media: mediatek: vcodec: fix potential double free | expand

Commit Message

Dan Carpenter June 14, 2023, 1:07 p.m. UTC
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")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 .../media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Comments

Nicolas Dufresne July 20, 2023, 7:38 p.m. UTC | #1
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@collabora.com>

> Signed-off-by: Dan Carpenter <dan.carpenter@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);
>
diff mbox series

Patch

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);