Hi,
Just a reminder to Yunfei that this change was originally addressed to him, and
a review was to be expected. Over a month is a bit too long for fixes.
Le mercredi 14 juin 2023 à 16:05 +0300, Dan Carpenter a écrit :
> The "lat_buf->private_data" needs to be set to NULL to prevent a
> double free. How this would happen is if vdec_msg_queue_init() failed
> twice in a row and on the second time it failed earlier than on the
> first time.
>
> The vdec_msg_queue_init() function has a loop which does:
> for (i = 0; i < NUM_BUFFER_COUNT; i++) {
>
> Each iteration initializes one element in the msg_queue->lat_buf[] array
> and then the clean up function vdec_msg_queue_deinit() frees each
> element of the msg_queue->lat_buf[] array. This clean up code relies
> on the assumption that every element is either initialized or zeroed.
> Leaving a freed pointer which is non-zero breaks the assumption.
>
> Fixes: b199fe46f35c ("media: mtk-vcodec: Add msg queue feature for lat and core architecture")
Unbalanced calls to deinit/init would be unfortunate, but I like keeping it safe
and aligned with the fact everything is clears to 0/null otherwise.
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> drivers/media/platform/mediatek/vcodec/vdec_msg_queue.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.c b/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.c
> index f555341ae708..92ac82eb444e 100644
> --- a/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.c
> +++ b/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.c
> @@ -231,6 +231,7 @@ void vdec_msg_queue_deinit(struct vdec_msg_queue *msg_queue,
> mtk_vcodec_mem_free(ctx, mem);
>
> kfree(lat_buf->private_data);
> + lat_buf->private_data = NULL;
> }
>
> cancel_work_sync(&msg_queue->core_work);
