Re: [PATCH] staging: media: atomisp: bound DVS 6-axis config copy size against allocated grid

[Dan Carpenter]

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@xxxxxxxxxxxxxxx
> Signed-off-by: Shayaun Nejad <snejad123@xxxxxxxxx>
> ---
>  .../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