[PATCH v2] media: atomisp: fix scaler setup leaks

[Dawei Feng]

load_video_binaries() and load_primary_binaries() create a CAS scaler
descriptor before allocating and looking up the YUV scaler binaries. Error
paths after descriptor creation can return without destroying the
descriptor and without freeing the partially allocated scaler state.

Fix this by properly destroying the CAS scaler descriptor on failure and
using reverse-order cleanup labels to free the partially allocated scaler
arrays and state variables.

The bug was first flagged by an experimental analysis tool we are
developing for kernel memory-management bugs while analyzing
v6.13-rc1. The tool is still under development and is not yet publicly
available. Manual inspection confirms that the bug is still
present in v7.1.1.

An x86_64 allyesconfig build showed no new warnings. As we do not have
an Intel Atom ISP camera platform with matching sensor firmware and ACPI
camera graph to test with, no runtime testing was able to be performed.

Fixes: ad85094b293e ("Revert "media: staging: atomisp: Remove driver"")
Signed-off-by: Dawei Feng <dawei.feng@xxxxxxxxxx>
---
Changes in v2:
- Use reverse-order cleanup labels to free partially allocated scaler
  state on the scaler setup failure paths.

 drivers/staging/media/atomisp/pci/sh_css.c | 43 +++++++++++++++++-----
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 6cda5925fa45..b17e0b3879f4 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -4527,21 +4527,25 @@ static int load_video_binaries(struct ia_css_pipe *pipe)
 			  pipe_out_info,
 			  NULL,
 			  &cas_scaler_descr);
-		if (err)
+		if (err) {
+			ia_css_pipe_destroy_cas_scaler_desc(&cas_scaler_descr);
 			return err;
+		}
 		mycs->num_yuv_scaler = cas_scaler_descr.num_stage;
 		mycs->yuv_scaler_binary = kzalloc_objs(struct ia_css_binary,
 						       cas_scaler_descr.num_stage);
 		if (!mycs->yuv_scaler_binary) {
 			mycs->num_yuv_scaler = 0;
 			err = -ENOMEM;
+			ia_css_pipe_destroy_cas_scaler_desc(&cas_scaler_descr);
 			return err;
 		}
 		mycs->is_output_stage = kzalloc_objs(bool,
 						     cas_scaler_descr.num_stage);
 		if (!mycs->is_output_stage) {
 			err = -ENOMEM;
-			return err;
+			ia_css_pipe_destroy_cas_scaler_desc(&cas_scaler_descr);
+			goto free_yuv_scaler;
 		}
 		for (i = 0; i < cas_scaler_descr.num_stage; i++) {
 			struct ia_css_binary_descr yuv_scaler_descr;
@@ -4555,9 +4559,8 @@ static int load_video_binaries(struct ia_css_pipe *pipe)
 			err = ia_css_binary_find(&yuv_scaler_descr,
 						 &mycs->yuv_scaler_binary[i]);
 			if (err) {
-				kfree(mycs->is_output_stage);
-				mycs->is_output_stage = NULL;
-				return err;
+				ia_css_pipe_destroy_cas_scaler_desc(&cas_scaler_descr);
+				goto free_output_stage;
 			}
 		}
 		ia_css_pipe_destroy_cas_scaler_desc(&cas_scaler_descr);
@@ -4734,6 +4737,15 @@ static int load_video_binaries(struct ia_css_pipe *pipe)
 	}
 	IA_CSS_LEAVE_PRIVATE("");
 	return 0;
+
+free_output_stage:
+	kfree(mycs->is_output_stage);
+	mycs->is_output_stage = NULL;
+free_yuv_scaler:
+	kfree(mycs->yuv_scaler_binary);
+	mycs->yuv_scaler_binary = NULL;
+	mycs->num_yuv_scaler = 0;
+	return err;
 }
 
 static int
@@ -5105,6 +5117,7 @@ static int load_primary_binaries(
 			  NULL,
 			  &cas_scaler_descr);
 		if (err) {
+			ia_css_pipe_destroy_cas_scaler_desc(&cas_scaler_descr);
 			IA_CSS_LEAVE_ERR_PRIVATE(err);
 			return err;
 		}
@@ -5112,7 +5125,9 @@ static int load_primary_binaries(
 		mycs->yuv_scaler_binary = kzalloc_objs(struct ia_css_binary,
 						       cas_scaler_descr.num_stage);
 		if (!mycs->yuv_scaler_binary) {
+			mycs->num_yuv_scaler = 0;
 			err = -ENOMEM;
+			ia_css_pipe_destroy_cas_scaler_desc(&cas_scaler_descr);
 			IA_CSS_LEAVE_ERR_PRIVATE(err);
 			return err;
 		}
@@ -5120,8 +5135,8 @@ static int load_primary_binaries(
 						     cas_scaler_descr.num_stage);
 		if (!mycs->is_output_stage) {
 			err = -ENOMEM;
-			IA_CSS_LEAVE_ERR_PRIVATE(err);
-			return err;
+			ia_css_pipe_destroy_cas_scaler_desc(&cas_scaler_descr);
+			goto free_yuv_scaler;
 		}
 		for (i = 0; i < cas_scaler_descr.num_stage; i++) {
 			struct ia_css_binary_descr yuv_scaler_descr;
@@ -5135,8 +5150,8 @@ static int load_primary_binaries(
 			err = ia_css_binary_find(&yuv_scaler_descr,
 						 &mycs->yuv_scaler_binary[i]);
 			if (err) {
-				IA_CSS_LEAVE_ERR_PRIVATE(err);
-				return err;
+				ia_css_pipe_destroy_cas_scaler_desc(&cas_scaler_descr);
+				goto free_output_stage;
 			}
 		}
 		ia_css_pipe_destroy_cas_scaler_desc(&cas_scaler_descr);
@@ -5257,6 +5272,16 @@ static int load_primary_binaries(
 	}
 
 	return 0;
+
+free_output_stage:
+	kfree(mycs->is_output_stage);
+	mycs->is_output_stage = NULL;
+free_yuv_scaler:
+	kfree(mycs->yuv_scaler_binary);
+	mycs->yuv_scaler_binary = NULL;
+	mycs->num_yuv_scaler = 0;
+	IA_CSS_LEAVE_ERR_PRIVATE(err);
+	return err;
 }
 
 static int
-- 
2.34.1