iris_vdec_inst_init() allocates the source and destination v4l2_format
structures and then immediately writes fields through inst->fmt_src and
inst->fmt_dst. Either allocation can fail, leading to a NULL pointer
dereference during instance initialization.

Check both allocations before initializing the formats. Free any partial
allocation, clear the instance pointers so later cleanup does not see
dangling values, and return -ENOMEM so the open path can unwind the
instance.

Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/media/platform/qcom/iris/iris_vdec.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
index 99d544e2af4f9..dd18079a9ea5f 100644
--- a/drivers/media/platform/qcom/iris/iris_vdec.c
+++ b/drivers/media/platform/qcom/iris/iris_vdec.c
@@ -23,6 +23,13 @@ int iris_vdec_inst_init(struct iris_inst *inst)
 
 	inst->fmt_src = kzalloc_obj(*inst->fmt_src);
 	inst->fmt_dst = kzalloc_obj(*inst->fmt_dst);
+	if (!inst->fmt_src || !inst->fmt_dst) {
+		kfree(inst->fmt_src);
+		kfree(inst->fmt_dst);
+		inst->fmt_src = NULL;
+		inst->fmt_dst = NULL;
+		return -ENOMEM;
+	}
 
 	inst->fw_min_count = MIN_BUFFERS;
 
-- 
2.34.1

On Sat, Jun 06, 2026 at 12:07:36PM +0800, Ruoyu Wang wrote:
> iris_vdec_inst_init() allocates the source and destination v4l2_format
> structures and then immediately writes fields through inst->fmt_src and
> inst->fmt_dst. Either allocation can fail, leading to a NULL pointer
> dereference during instance initialization.
> 
> Check both allocations before initializing the formats. Free any partial
> allocation, clear the instance pointers so later cleanup does not see
> dangling values, and return -ENOMEM so the open path can unwind the
> instance.
> 
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
>  drivers/media/platform/qcom/iris/iris_vdec.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
> index 99d544e2af4f9..dd18079a9ea5f 100644
> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
> @@ -23,6 +23,13 @@ int iris_vdec_inst_init(struct iris_inst *inst)
>  
>  	inst->fmt_src = kzalloc_obj(*inst->fmt_src);
>  	inst->fmt_dst = kzalloc_obj(*inst->fmt_dst);
> +	if (!inst->fmt_src || !inst->fmt_dst) {
> +		kfree(inst->fmt_src);
> +		kfree(inst->fmt_dst);
> +		inst->fmt_src = NULL;
> +		inst->fmt_dst = NULL;
> +		return -ENOMEM;
> +	}

I'd rather see the check for the allocated objects before they are
assigned to the fields in the instance.

>  
>  	inst->fw_min_count = MIN_BUFFERS;
>  
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

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@gmail.com>
---
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;
+	}
+
+	inst->fmt_src = fmt_src;
+	inst->fmt_dst = fmt_dst;
 
 	inst->fw_min_count = MIN_BUFFERS;
 
-- 
2.51.0

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@gmail.com>
> ---
> 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