Re: [PATCH v2] media: iris: check decoder format allocations

[Dmitry Baryshkov]

On Sat, Jun 06, 2026 at 04:16:36PM +0800, Ruoyu Wang wrote:
> iris_vdec_inst_init() allocates source and destination v4l2_format
> structures before initializing their fields. Allocation failures would
> leave the function dereferencing NULL pointers during instance
> initialization.
> 
> Allocate the formats into local variables and check each allocation before
> assigning them to the instance. If the second allocation fails, free the
> first allocation and return -ENOMEM. Store the pointers in the instance
> only after both allocations have succeeded so the open path can unwind
> cleanly.
> 
> Signed-off-by: Ruoyu Wang <ruoyuw560@xxxxxxxxx>
> ---
> Changes in v2:
> - Allocate the formats into local variables and assign them to the
>   instance only after both allocations succeed, as requested in review.
> 
>  drivers/media/platform/qcom/iris/iris_vdec.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
> index 99d544e2af4f9..837f29f403bb7 100644
> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
> @@ -19,10 +19,21 @@
>  int iris_vdec_inst_init(struct iris_inst *inst)
>  {
>  	struct iris_core *core = inst->core;
> +	struct v4l2_format *fmt_src, *fmt_dst;
>  	struct v4l2_format *f;
>  
> -	inst->fmt_src = kzalloc_obj(*inst->fmt_src);
> -	inst->fmt_dst = kzalloc_obj(*inst->fmt_dst);
> +	fmt_src = kzalloc_obj(*fmt_src);
> +	if (!fmt_src)
> +		return -ENOMEM;
> +
> +	fmt_dst = kzalloc_obj(*fmt_dst);
> +	if (!fmt_dst) {
> +		kfree(fmt_src);
> +		return -ENOMEM;
> +	}

This is not the style of the rollback that is used in Linux kernel. Also
if iris_ctrls_init() fails, then the allocate memory will not be
unallocated. Further iris_open() will happily overwrite those
pointers, resulting in a memory leak.

Should we replace the pointers with the instances of v4l2_format
instead?


BTW: please don't send patch iterations as a reply to a previous thread.
Always start a new thread for the new iteration.

> +
> +	inst->fmt_src = fmt_src;
> +	inst->fmt_dst = fmt_dst;
>  
>  	inst->fw_min_count = MIN_BUFFERS;
>  
> -- 
> 2.51.0
> 

-- 
With best wishes
Dmitry