Re: [PATCH v3 2/4] media: mali-c55: Implement CCM block validation

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Sat, Jun 27, 2026 at 04:29:14PM +0200, Jacopo Mondi wrote:
> From: Jacopo Mondi <jacopo.mondi+renesas@xxxxxxxxxxxxxxxx>
> 
> Implement validation of CCM block parameters.
> 
> CCM coefficients are expressed as 13 bits signed Q4.8 format and their
> raw value cannot be higher than 8191 (BIT(13) - 1).
> 
> CCM gains are expressed as unsigned 12 bits Q4.8 format and their raw
> value cannot be higher than 4095 (BIT(12) - 1).
> 
> CCM offsets are 12 bits unsigned integers and their value cannot be
> higher than 4095 (BIT(12) - 1).
> 
> Validate the parameters provided by userspace using the .block_validate
> callback of struct v4l2_isp_params_block_type_info.

I don't think this is needed.

We need to validate parameters that can cause the ISP to malfunction in
ways that requires a system reset, or in ways that cause malfunction of
other system components (e.g. buffer overflows, memory bus lock ups,
...). The rest doesn't need to be validated.

If you want to be cautious, you can just mask the value when writing to
registers, which I think you're doing in patch 1/4.

> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@xxxxxxxxxxxxxxxx>
> 
> ---
> v3:
> - new patch
> ---
>  .../media/platform/arm/mali-c55/mali-c55-params.c  | 36 ++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> index 0da5215c52c3..333e66ee3923 100644
> --- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> @@ -477,6 +477,41 @@ static const mali_c55_params_handler mali_c55_params_handlers[] = {
>  	[MALI_C55_PARAM_BLOCK_CCM] = &mali_c55_params_ccm,
>  };
>  
> +static int mali_c55_ccm_validate(struct device *dev,
> +				 const struct v4l2_isp_block_header *block)
> +{
> +	const struct mali_c55_params_ccm *ccm =
> +		(const struct mali_c55_params_ccm *)(block);
> +
> +	for (unsigned int i = 0; i < 3; i++) {
> +
> +		for (unsigned int j = 0; j < 3; j++) {
> +			/* Coefficients are 13 bits signed Q4.8. */
> +			if (ccm->coeffs[i][j] > 8191) {
> +				dev_dbg(dev, "Invalid ccm coefficient %u\n",
> +					ccm->coeffs[i][j]);
> +				return -EINVAL;
> +			}
> +		}
> +
> +		/* Gains are 12 bits unsigned Q4.8. */
> +		if (ccm->gains[i] > 4095) {
> +			dev_dbg(dev, "Invalid ccm gain %u\n",
> +				ccm->gains[i]);
> +			return -EINVAL;
> +		}
> +
> +		/* Offsets are 12 bits unsigned integers. */
> +		if (ccm->offs[i] > 4095) {
> +			dev_dbg(dev, "Invalid ccm offset %u\n",
> +				ccm->offs[i]);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct v4l2_isp_params_block_type_info
>  mali_c55_params_block_types_info[] = {
>  	[MALI_C55_PARAM_BLOCK_SENSOR_OFFS] = {
> @@ -514,6 +549,7 @@ mali_c55_params_block_types_info[] = {
>  	},
>  	[MALI_C55_PARAM_BLOCK_CCM] = {
>  		.size = sizeof(struct mali_c55_params_ccm),
> +		.block_validate = mali_c55_ccm_validate,
>  	},
>  };
>  

-- 
Regards,

Laurent Pinchart