Linux Media Controller development
 help / color / mirror / Atom feed
* [PATCH] staging: media: atomisp: bound DVS 6-axis config copy size against allocated grid
@ 2026-05-12  1:45 Shayaun Nejad
  2026-05-12  7:46 ` Dan Carpenter
  0 siblings, 1 reply; 2+ messages in thread
From: Shayaun Nejad @ 2026-05-12  1:45 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans de Goede
  Cc: Sakari Ailus, Greg Kroah-Hartman, linux-media, linux-staging,
	linux-kernel, stable, Shayaun Nejad

atomisp_cp_dvs_6axis_config() copies user-provided coordinate arrays into
a 6-axis grid allocated from ISP dimensions.

The copy sizes are computed from the user width and height fields, so
mismatched or overflowing dimensions can copy past the allocated buffers.

Reject dimensions that do not match the allocated config and compute the
copy sizes with array3_size() before copying.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Cc: stable@vger.kernel.org
Signed-off-by: Shayaun Nejad <snejad123@gmail.com>
---
 .../staging/media/atomisp/pci/atomisp_cmd.c   | 84 ++++++++++++-------
 1 file changed, 52 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index fec369575d..677037f1da 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/kfifo.h>
 #include <linux/pm_runtime.h>
+#include <linux/overflow.h>
 #include <linux/timer.h>
 
 #include <asm/iosf_mbi.h>
@@ -2570,6 +2571,29 @@ int atomisp_css_cp_dvs2_coefs(struct atomisp_sub_device *asd,
 	return 0;
 }
 
+static int atomisp_dvs_6axis_size(struct ia_css_dvs_6axis_config *config,
+				  u32 width_y, u32 height_y,
+				  u32 width_uv, u32 height_uv,
+				  size_t *y_size, size_t *uv_size)
+{
+	if (config->width_y != width_y ||
+	    config->height_y != height_y ||
+	    config->width_uv != width_uv ||
+	    config->height_uv != height_uv)
+		return -EINVAL;
+
+	*y_size = array3_size(width_y, height_y, sizeof(*config->xcoords_y));
+	if (*y_size == SIZE_MAX)
+		return -EINVAL;
+
+	*uv_size = array3_size(width_uv, height_uv,
+			       sizeof(*config->xcoords_uv));
+	if (*uv_size == SIZE_MAX)
+		return -EINVAL;
+
+	return 0;
+}
+
 int atomisp_cp_dvs_6axis_config(struct atomisp_sub_device *asd,
 				struct atomisp_dvs_6axis_config *source_6axis_config,
 				struct atomisp_css_params *css_param,
@@ -2582,6 +2606,8 @@ int atomisp_cp_dvs_6axis_config(struct atomisp_sub_device *asd,
 	struct ia_css_dvs_grid_info *dvs_grid_info =
 	    atomisp_css_get_dvs_grid_info(&asd->params.curr_grid_info);
 	int ret = -EFAULT;
+	size_t y_size;
+	size_t uv_size;
 
 	if (!stream) {
 		dev_err(asd->isp->dev, "%s: internal error!", __func__);
@@ -2628,35 +2654,32 @@ int atomisp_cp_dvs_6axis_config(struct atomisp_sub_device *asd,
 				return -ENOMEM;
 		}
 
+		ret = atomisp_dvs_6axis_size(dvs_6axis_config,
+					     t_6axis_config.width_y,
+					     t_6axis_config.height_y,
+					     t_6axis_config.width_uv,
+					     t_6axis_config.height_uv,
+					     &y_size, &uv_size);
+		if (ret)
+			goto error;
+
 		dvs_6axis_config->exp_id = t_6axis_config.exp_id;
 
 		if (copy_from_compatible(dvs_6axis_config->xcoords_y,
 					t_6axis_config.xcoords_y,
-					t_6axis_config.width_y *
-					t_6axis_config.height_y *
-					sizeof(*dvs_6axis_config->xcoords_y),
-					from_user))
+					y_size, from_user))
 			goto error;
 		if (copy_from_compatible(dvs_6axis_config->ycoords_y,
 					t_6axis_config.ycoords_y,
-					t_6axis_config.width_y *
-					t_6axis_config.height_y *
-					sizeof(*dvs_6axis_config->ycoords_y),
-					from_user))
+					y_size, from_user))
 			goto error;
 		if (copy_from_compatible(dvs_6axis_config->xcoords_uv,
 					t_6axis_config.xcoords_uv,
-					t_6axis_config.width_uv *
-					t_6axis_config.height_uv *
-					sizeof(*dvs_6axis_config->xcoords_uv),
-					from_user))
+					uv_size, from_user))
 			goto error;
 		if (copy_from_compatible(dvs_6axis_config->ycoords_uv,
 					t_6axis_config.ycoords_uv,
-					t_6axis_config.width_uv *
-					t_6axis_config.height_uv *
-					sizeof(*dvs_6axis_config->ycoords_uv),
-					from_user))
+					uv_size, from_user))
 			goto error;
 	} else {
 		if (old_6axis_config &&
@@ -2680,35 +2703,32 @@ int atomisp_cp_dvs_6axis_config(struct atomisp_sub_device *asd,
 			}
 		}
 
+		ret = atomisp_dvs_6axis_size(dvs_6axis_config,
+					     source_6axis_config->width_y,
+					     source_6axis_config->height_y,
+					     source_6axis_config->width_uv,
+					     source_6axis_config->height_uv,
+					     &y_size, &uv_size);
+		if (ret)
+			goto error;
+
 		dvs_6axis_config->exp_id = source_6axis_config->exp_id;
 
 		if (copy_from_compatible(dvs_6axis_config->xcoords_y,
 					source_6axis_config->xcoords_y,
-					source_6axis_config->width_y *
-					source_6axis_config->height_y *
-					sizeof(*source_6axis_config->xcoords_y),
-					from_user))
+					y_size, from_user))
 			goto error;
 		if (copy_from_compatible(dvs_6axis_config->ycoords_y,
 					source_6axis_config->ycoords_y,
-					source_6axis_config->width_y *
-					source_6axis_config->height_y *
-					sizeof(*source_6axis_config->ycoords_y),
-					from_user))
+					y_size, from_user))
 			goto error;
 		if (copy_from_compatible(dvs_6axis_config->xcoords_uv,
 					source_6axis_config->xcoords_uv,
-					source_6axis_config->width_uv *
-					source_6axis_config->height_uv *
-					sizeof(*source_6axis_config->xcoords_uv),
-					from_user))
+					uv_size, from_user))
 			goto error;
 		if (copy_from_compatible(dvs_6axis_config->ycoords_uv,
 					source_6axis_config->ycoords_uv,
-					source_6axis_config->width_uv *
-					source_6axis_config->height_uv *
-					sizeof(*source_6axis_config->ycoords_uv),
-					from_user))
+					uv_size, from_user))
 			goto error;
 	}
 	css_param->dvs_6axis = dvs_6axis_config;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] staging: media: atomisp: bound DVS 6-axis config copy size against allocated grid
  2026-05-12  1:45 [PATCH] staging: media: atomisp: bound DVS 6-axis config copy size against allocated grid Shayaun Nejad
@ 2026-05-12  7:46 ` Dan Carpenter
  0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2026-05-12  7:46 UTC (permalink / raw)
  To: Shayaun Nejad
  Cc: Mauro Carvalho Chehab, Hans de Goede, Sakari Ailus,
	Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel,
	stable

On Mon, May 11, 2026 at 06:45:14PM -0700, Shayaun Nejad wrote:
> atomisp_cp_dvs_6axis_config() copies user-provided coordinate arrays into
> a 6-axis grid allocated from ISP dimensions.
> 
> The copy sizes are computed from the user width and height fields, so
> mismatched or overflowing dimensions can copy past the allocated buffers.
> 
> Reject dimensions that do not match the allocated config and compute the
> copy sizes with array3_size() before copying.
> 
> Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shayaun Nejad <snejad123@gmail.com>
> ---
>  .../staging/media/atomisp/pci/atomisp_cmd.c   | 84 ++++++++++++-------
>  1 file changed, 52 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
> index fec369575d..677037f1da 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
> @@ -14,6 +14,7 @@
>  #include <linux/kernel.h>
>  #include <linux/kfifo.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/overflow.h>
>  #include <linux/timer.h>
>  
>  #include <asm/iosf_mbi.h>
> @@ -2570,6 +2571,29 @@ int atomisp_css_cp_dvs2_coefs(struct atomisp_sub_device *asd,
>  	return 0;
>  }
>  
> +static int atomisp_dvs_6axis_size(struct ia_css_dvs_6axis_config *config,
> +				  u32 width_y, u32 height_y,
> +				  u32 width_uv, u32 height_uv,
> +				  size_t *y_size, size_t *uv_size)
> +{
> +	if (config->width_y != width_y ||
> +	    config->height_y != height_y ||
> +	    config->width_uv != width_uv ||
> +	    config->height_uv != height_uv)
> +		return -EINVAL;
> +
> +	*y_size = array3_size(width_y, height_y, sizeof(*config->xcoords_y));
> +	if (*y_size == SIZE_MAX)
> +		return -EINVAL;
> +
> +	*uv_size = array3_size(width_uv, height_uv,
> +			       sizeof(*config->xcoords_uv));
> +	if (*uv_size == SIZE_MAX)
> +		return -EINVAL;
> +
> +	return 0;
> +}

This commit doesn't make sense.  Any time people end up checking
size_mul() type calculations for SIZE_MAX it's probably a sign things
have gone wrong.  You're supposed to just pass it along and let
regular bounds checking handle it.  It's not like ULONG_MAX is a special
sort of "extra bad" invalid number.

So we have some math here and if it equals >= ULONG_MAX then it's
invalid.

> +
>  int atomisp_cp_dvs_6axis_config(struct atomisp_sub_device *asd,
>  				struct atomisp_dvs_6axis_config *source_6axis_config,
>  				struct atomisp_css_params *css_param,
> @@ -2582,6 +2606,8 @@ int atomisp_cp_dvs_6axis_config(struct atomisp_sub_device *asd,
>  	struct ia_css_dvs_grid_info *dvs_grid_info =
>  	    atomisp_css_get_dvs_grid_info(&asd->params.curr_grid_info);
>  	int ret = -EFAULT;
> +	size_t y_size;
> +	size_t uv_size;
>  
>  	if (!stream) {
>  		dev_err(asd->isp->dev, "%s: internal error!", __func__);
> @@ -2628,35 +2654,32 @@ int atomisp_cp_dvs_6axis_config(struct atomisp_sub_device *asd,
>  				return -ENOMEM;
>  		}
>  
> +		ret = atomisp_dvs_6axis_size(dvs_6axis_config,
> +					     t_6axis_config.width_y,
> +					     t_6axis_config.height_y,
> +					     t_6axis_config.width_uv,
> +					     t_6axis_config.height_uv,
> +					     &y_size, &uv_size);
> +		if (ret)
> +			goto error;
> +
>  		dvs_6axis_config->exp_id = t_6axis_config.exp_id;
>  
>  		if (copy_from_compatible(dvs_6axis_config->xcoords_y,
>  					t_6axis_config.xcoords_y,
> -					t_6axis_config.width_y *
> -					t_6axis_config.height_y *
> -					sizeof(*dvs_6axis_config->xcoords_y),
> -					from_user))
> +					y_size, from_user))

But it the result stored in y_size is ULONG_MAX - 1 then we copy that
number of bytes from the user.

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-05-12  7:46 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12  1:45 [PATCH] staging: media: atomisp: bound DVS 6-axis config copy size against allocated grid Shayaun Nejad
2026-05-12  7:46 ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox