[PATCH 0/6] media: Fix new smatch warnings

[PATCH 0/6] media: Fix new smatch warnings

[Ricardo Ribalda]

Current version of smatch triggers some warnings for the media tree.
Most of them are inoffensive, but we would like to have zero smatch
warnings.

drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
drivers/media/v4l2-core/v4l2-dev.c:1036 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
drivers/media/v4l2-core/v4l2-dev.c:1043 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
drivers/media/v4l2-core/v4l2-dev.c:1101 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
drivers/media/platform/chips-media/wave5/wave5-vpuapi.c:588 wave5_vpu_dec_get_output_info() error: buffer overflow 'inst->frame_buf' 64 <= 127
drivers/staging/media/ipu3/ipu3-css-params.c:1792 imgu_css_cfg_acc_stripe() warn: 'acc->stripe.bds_out_stripes[0]->width - 2 * f' 4294967168 can't fit into 65535 'acc->stripe.bds_out_stripes[1]->offset'
drivers/media/i2c/adv7604.c:3672 adv76xx_probe() error: buffer overflow 'state->pads' 7 <= 4294967294
drivers/media/i2c/adv7604.c:3673 adv76xx_probe() error: buffer overflow 'state->pads' 7 <= u32max
drivers/media/i2c/mt9p031.c:799 mt9p031_s_ctrl() warn: assigning (-1952) to unsigned variable 'data'

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Ricardo Ribalda (6):
      media: v4l2-dev: Add range check for vdev->minor
      media: i2c: mt9p031: Rewrite a bitwise mask
      media: i2c: adv7604: Add range checks for chip info
      media: chips-media: wave5: Add range checks for dec_output_info
      media: staging: ipu3-imgu: Add range check for imgu_css_cfg_acc_stripe
      media: amlogic-c3: Add validations for ae and awb config

 drivers/media/i2c/adv7604.c                             |  6 ++++++
 drivers/media/i2c/mt9p031.c                             |  2 +-
 drivers/media/platform/amlogic/c3/isp/c3-isp-params.c   |  4 ++++
 drivers/media/platform/chips-media/wave5/wave5-vpuapi.c | 11 +++++++++--
 drivers/media/v4l2-core/v4l2-dev.c                      |  6 ++++++
 drivers/staging/media/ipu3/ipu3-css-params.c            |  8 ++++++--
 6 files changed, 32 insertions(+), 5 deletions(-)
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260428-smatch-7-1-d969299dd3cf

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>

[PATCH 1/6] media: v4l2-dev: Add range check for vdev->minor

[Ricardo Ribalda]

If the fixed minor ranges are not properly set we could end up in a
situation where the calculated minor is invalid. Add a check for this in
the code.

This check also fixes the following smatch warning:

drivers/media/v4l2-core/v4l2-dev.c:1036 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
drivers/media/v4l2-core/v4l2-dev.c:1043 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
drivers/media/v4l2-core/v4l2-dev.c:1101 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/v4l2-core/v4l2-dev.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 6ce623a1245a..a731ffdb91ee 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -1032,6 +1032,12 @@ int __video_register_device(struct video_device *vdev,
 	vdev->minor = i + minor_offset;
 	vdev->num = nr;
 
+	if (WARN_ON(vdev->minor >= VIDEO_NUM_DEVICES)) {
+		mutex_unlock(&videodev_lock);
+		pr_err("invalid minor. Check ranges.\n");
+		return -EINVAL;
+	}
+
 	/* Should not happen since we thought this minor was free */
 	if (WARN_ON(video_devices[vdev->minor])) {
 		mutex_unlock(&videodev_lock);

-- 
2.54.0.545.g6539524ca2-goog

[PATCH 2/6] media: i2c: mt9p031: Rewrite a bitwise mask

[Ricardo Ribalda]

The current code makes smatch a bit uncomfortable:
drivers/media/i2c/mt9p031.c:799 mt9p031_s_ctrl() warn: assigning (-1952) to unsigned variable 'data'

Probably because smatch is not clever enough (yet). Do a simple rewrite
to make sure that smatch understands what we are doing here.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/i2c/mt9p031.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index ea5d43d925ff..5c9dff030b4d 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -795,7 +795,7 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
 			ctrl->val &= ~1;
 			data = (1 << 6) | (ctrl->val >> 1);
 		} else {
-			ctrl->val &= ~7;
+			ctrl->val -= ctrl->val % 8;
 			data = ((ctrl->val - 64) << 5) | (1 << 6) | 32;
 		}
 

-- 
2.54.0.545.g6539524ca2-goog

[PATCH 3/6] media: i2c: adv7604: Add range checks for chip info

[Ricardo Ribalda]

If the driver's chip information is invalid we can end up accessing an
invalid memory region.

This fixes the following smatch errors:
drivers/media/i2c/adv7604.c:3672 adv76xx_probe() error: buffer overflow 'state->pads' 7 <= 4294967294
drivers/media/i2c/adv7604.c:3673 adv76xx_probe() error: buffer overflow 'state->pads' 7 <= u32max

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/i2c/adv7604.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 67116a4ef134..ae75982fb514 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -3668,6 +3668,12 @@ static int adv76xx_probe(struct i2c_client *client)
 
 	state->source_pad = state->info->num_dv_ports
 			  + (state->info->has_afe ? 2 : 0);
+	if (WARN_ON(state->source_pad >= ADV76XX_PAD_MAX)) {
+		err = -EINVAL;
+		v4l2_err(sd, "invalid chip info\n");
+		goto err_i2c;
+	}
+
 	for (i = 0; i < state->source_pad; ++i)
 		state->pads[i].flags = MEDIA_PAD_FL_SINK;
 	state->pads[state->source_pad].flags = MEDIA_PAD_FL_SOURCE;

-- 
2.54.0.545.g6539524ca2-goog

[PATCH 4/6] media: chips-media: wave5: Add range checks for dec_output_info

[Ricardo Ribalda]

If the driver's dec_output_info contains invalid data the driver can
write in invalid memory. Add a range check for that.

This fixes this smatch error:
drivers/media/platform/chips-media/wave5/wave5-vpuapi.c:588 wave5_vpu_dec_get_output_info() error: buffer overflow 'inst->frame_buf' 64 <= 127

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/platform/chips-media/wave5/wave5-vpuapi.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
index d26ffc942219..f77abd5e122a 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
@@ -584,8 +584,15 @@ int wave5_vpu_dec_get_output_info(struct vpu_instance *inst, struct dec_output_i
 		p_dec_info->num_of_decoding_fbs : p_dec_info->num_of_display_fbs;
 
 	if (info->index_frame_display >= 0 &&
-	    info->index_frame_display < (int)max_dec_index)
-		info->disp_frame = inst->frame_buf[val + info->index_frame_display];
+	    info->index_frame_display < (int)max_dec_index) {
+		u32 idx = val + info->index_frame_display;
+
+		if (WARN_ON(idx >= MAX_REG_FRAME)) {
+			ret = -EINVAL;
+			goto err_out;
+		}
+		info->disp_frame = inst->frame_buf[idx];
+	}
 
 	info->rd_ptr = p_dec_info->stream_rd_ptr;
 	info->wr_ptr = p_dec_info->stream_wr_ptr;

-- 
2.54.0.545.g6539524ca2-goog

[PATCH 5/6] media: staging: ipu3-imgu: Add range check for imgu_css_cfg_acc_stripe

[Ricardo Ribalda]

If the driver's stripe information is invalid it can result in an integer
overflow. Add a range check with a WARN_ON to expose this kind of
error.

This patch fixes the following smatch error:
drivers/staging/media/ipu3/ipu3-css-params.c:1792 imgu_css_cfg_acc_stripe() warn: 'acc->stripe.bds_out_stripes[0]->width - 2 * f' 4294967168 can't fit into 65535 'acc->stripe.bds_out_stripes[1]->offset'

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/staging/media/ipu3/ipu3-css-params.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c
index 2c48d57a3180..6ed23c7a0c3f 100644
--- a/drivers/staging/media/ipu3/ipu3-css-params.c
+++ b/drivers/staging/media/ipu3/ipu3-css-params.c
@@ -1770,6 +1770,8 @@ static int imgu_css_cfg_acc_stripe(struct imgu_css *css, unsigned int pipe,
 		acc->stripe.bds_out_stripes[0].width =
 			ALIGN(css_pipe->rect[IPU3_CSS_RECT_BDS].width, f);
 	} else {
+		u32 offset;
+
 		/* Image processing is divided into two stripes */
 		acc->stripe.bds_out_stripes[0].width =
 			acc->stripe.bds_out_stripes[1].width =
@@ -1788,8 +1790,10 @@ static int imgu_css_cfg_acc_stripe(struct imgu_css *css, unsigned int pipe,
 			acc->stripe.bds_out_stripes[1].width += f;
 		}
 		/* Overlap between stripes is IPU3_UAPI_ISP_VEC_ELEMS * 4 */
-		acc->stripe.bds_out_stripes[1].offset =
-			acc->stripe.bds_out_stripes[0].width - 2 * f;
+		offset = acc->stripe.bds_out_stripes[0].width - 2 * f;
+		if (WARN_ON(offset > 65535))
+			return -EINVAL;
+		acc->stripe.bds_out_stripes[1].offset = offset;
 	}
 
 	acc->stripe.effective_stripes[0].height =

-- 
2.54.0.545.g6539524ca2-goog

[PATCH 6/6] media: amlogic-c3: Add validations for ae and awb config

[Ricardo Ribalda]

Avoid invalid memory access if the zones_num is bigger than
zone_weight.

This patch fixes the following smatch errors:
drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
index 6f9ca7a7dd88..42d780f684d1 100644
--- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
+++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
@@ -104,6 +104,8 @@ static void c3_isp_params_awb_wt(struct c3_isp_device *isp,
 	c3_isp_write(isp, ISP_AWB_BLK_WT_ADDR, 0);
 
 	zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
+	if (WARN_ON(zones_num > C3_ISP_AWB_MAX_ZONES))
+		zones_num = C3_ISP_AWB_MAX_ZONES;
 
 	/* Need to write 8 weights at once */
 	for (i = 0; i < zones_num / 8; i++) {
@@ -220,6 +222,8 @@ static void c3_isp_params_ae_wt(struct c3_isp_device *isp,
 	c3_isp_write(isp, ISP_AE_BLK_WT_ADDR, 0);
 
 	zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
+	if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
+		zones_num = C3_ISP_AE_MAX_ZONES;
 
 	/* Need to write 8 weights at once */
 	for (i = 0; i < zones_num / 8; i++) {

-- 
2.54.0.545.g6539524ca2-goog

Re: [PATCH 6/6] media: amlogic-c3: Add validations for ae and awb config

[Laurent Pinchart]

Hi Ricardo,

Thank you for the patch.

CC'ing Jacopo.

On Tue, Apr 28, 2026 at 12:41:12PM +0000, Ricardo Ribalda wrote:
> Avoid invalid memory access if the zones_num is bigger than
> zone_weight.
> 
> This patch fixes the following smatch errors:
> drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> index 6f9ca7a7dd88..42d780f684d1 100644
> --- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> +++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> @@ -104,6 +104,8 @@ static void c3_isp_params_awb_wt(struct c3_isp_device *isp,
>  	c3_isp_write(isp, ISP_AWB_BLK_WT_ADDR, 0);
>  
>  	zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> +	if (WARN_ON(zones_num > C3_ISP_AWB_MAX_ZONES))

This is triggerable by userspace, it shouldn't result in a WARN_ON().
Ideally the horiz_zones_num and vert_zones_num should be validated at
buf prepare time, and an error should be returned to userspace. That
will likely not fix your smatch issue though, I don't think it will be
able to understand that the values have been validated.

Jacopo, do we need to add a validate function pointer to
v4l2_isp_params_block_type_info ?

> +		zones_num = C3_ISP_AWB_MAX_ZONES;
>  
>  	/* Need to write 8 weights at once */
>  	for (i = 0; i < zones_num / 8; i++) {
> @@ -220,6 +222,8 @@ static void c3_isp_params_ae_wt(struct c3_isp_device *isp,
>  	c3_isp_write(isp, ISP_AE_BLK_WT_ADDR, 0);
>  
>  	zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> +	if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
> +		zones_num = C3_ISP_AE_MAX_ZONES;
>  
>  	/* Need to write 8 weights at once */
>  	for (i = 0; i < zones_num / 8; i++) {
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/6] media: staging: ipu3-imgu: Add range check for imgu_css_cfg_acc_stripe

[Laurent Pinchart]

On Tue, Apr 28, 2026 at 12:41:11PM +0000, Ricardo Ribalda wrote:
> If the driver's stripe information is invalid it can result in an integer
> overflow. Add a range check with a WARN_ON to expose this kind of
> error.
> 
> This patch fixes the following smatch error:
> drivers/staging/media/ipu3/ipu3-css-params.c:1792 imgu_css_cfg_acc_stripe() warn: 'acc->stripe.bds_out_stripes[0]->width - 2 * f' 4294967168 can't fit into 65535 'acc->stripe.bds_out_stripes[1]->offset'
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/staging/media/ipu3/ipu3-css-params.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c
> index 2c48d57a3180..6ed23c7a0c3f 100644
> --- a/drivers/staging/media/ipu3/ipu3-css-params.c
> +++ b/drivers/staging/media/ipu3/ipu3-css-params.c
> @@ -1770,6 +1770,8 @@ static int imgu_css_cfg_acc_stripe(struct imgu_css *css, unsigned int pipe,
>  		acc->stripe.bds_out_stripes[0].width =
>  			ALIGN(css_pipe->rect[IPU3_CSS_RECT_BDS].width, f);
>  	} else {
> +		u32 offset;
> +
>  		/* Image processing is divided into two stripes */
>  		acc->stripe.bds_out_stripes[0].width =
>  			acc->stripe.bds_out_stripes[1].width =
> @@ -1788,8 +1790,10 @@ static int imgu_css_cfg_acc_stripe(struct imgu_css *css, unsigned int pipe,
>  			acc->stripe.bds_out_stripes[1].width += f;
>  		}
>  		/* Overlap between stripes is IPU3_UAPI_ISP_VEC_ELEMS * 4 */
> -		acc->stripe.bds_out_stripes[1].offset =
> -			acc->stripe.bds_out_stripes[0].width - 2 * f;
> +		offset = acc->stripe.bds_out_stripes[0].width - 2 * f;
> +		if (WARN_ON(offset > 65535))

If this can be triggered by userspace it shouldn't WARN_ON().

> +			return -EINVAL;
> +		acc->stripe.bds_out_stripes[1].offset = offset;
>  	}
>  
>  	acc->stripe.effective_stripes[0].height =

-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/6] media: amlogic-c3: Add validations for ae and awb config

[Ricardo Ribalda]

Hi Laurent

On Tue, 28 Apr 2026 at 15:10, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> CC'ing Jacopo.
>
> On Tue, Apr 28, 2026 at 12:41:12PM +0000, Ricardo Ribalda wrote:
> > Avoid invalid memory access if the zones_num is bigger than
> > zone_weight.
> >
> > This patch fixes the following smatch errors:
> > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > index 6f9ca7a7dd88..42d780f684d1 100644
> > --- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > +++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > @@ -104,6 +104,8 @@ static void c3_isp_params_awb_wt(struct c3_isp_device *isp,
> >       c3_isp_write(isp, ISP_AWB_BLK_WT_ADDR, 0);
> >
> >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > +     if (WARN_ON(zones_num > C3_ISP_AWB_MAX_ZONES))
>
> This is triggerable by userspace, it shouldn't result in a WARN_ON().
> Ideally the horiz_zones_num and vert_zones_num should be validated at
> buf prepare time, and an error should be returned to userspace. That
> will likely not fix your smatch issue though, I don't think it will be
> able to understand that the values have been validated.

Based on the warnings from the other drivers I also suspect that if
you have validated the data somewhere else smatch will understand it.

Even if you add a validate function I would suggest to keep the
WARN_ON(), ideally it should never trigger, and if it triggers it will
get a lot more attention to get it fixed.

>
> Jacopo, do we need to add a validate function pointer to
> v4l2_isp_params_block_type_info ?
>
> > +             zones_num = C3_ISP_AWB_MAX_ZONES;
> >
> >       /* Need to write 8 weights at once */
> >       for (i = 0; i < zones_num / 8; i++) {
> > @@ -220,6 +222,8 @@ static void c3_isp_params_ae_wt(struct c3_isp_device *isp,
> >       c3_isp_write(isp, ISP_AE_BLK_WT_ADDR, 0);
> >
> >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > +     if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
> > +             zones_num = C3_ISP_AE_MAX_ZONES;
> >
> >       /* Need to write 8 weights at once */
> >       for (i = 0; i < zones_num / 8; i++) {
> >
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

Re: [PATCH 6/6] media: amlogic-c3: Add validations for ae and awb config

[Ricardo Ribalda]

On Tue, 28 Apr 2026 at 15:14, Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> Hi Laurent
>
> On Tue, 28 Apr 2026 at 15:10, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi Ricardo,
> >
> > Thank you for the patch.
> >
> > CC'ing Jacopo.
> >
> > On Tue, Apr 28, 2026 at 12:41:12PM +0000, Ricardo Ribalda wrote:
> > > Avoid invalid memory access if the zones_num is bigger than
> > > zone_weight.
> > >
> > > This patch fixes the following smatch errors:
> > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > >
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > >  drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > index 6f9ca7a7dd88..42d780f684d1 100644
> > > --- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > +++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > @@ -104,6 +104,8 @@ static void c3_isp_params_awb_wt(struct c3_isp_device *isp,
> > >       c3_isp_write(isp, ISP_AWB_BLK_WT_ADDR, 0);
> > >
> > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > +     if (WARN_ON(zones_num > C3_ISP_AWB_MAX_ZONES))
> >
> > This is triggerable by userspace, it shouldn't result in a WARN_ON().
> > Ideally the horiz_zones_num and vert_zones_num should be validated at
> > buf prepare time, and an error should be returned to userspace. That
> > will likely not fix your smatch issue though, I don't think it will be
> > able to understand that the values have been validated.
>
> Based on the warnings from the other drivers I also suspect that if
> you have validated the data somewhere else smatch will understand it.
I mean that

will *NOT* understand it.

Sorry

>
> Even if you add a validate function I would suggest to keep the
> WARN_ON(), ideally it should never trigger, and if it triggers it will
> get a lot more attention to get it fixed.
>
> >
> > Jacopo, do we need to add a validate function pointer to
> > v4l2_isp_params_block_type_info ?
> >
> > > +             zones_num = C3_ISP_AWB_MAX_ZONES;
> > >
> > >       /* Need to write 8 weights at once */
> > >       for (i = 0; i < zones_num / 8; i++) {
> > > @@ -220,6 +222,8 @@ static void c3_isp_params_ae_wt(struct c3_isp_device *isp,
> > >       c3_isp_write(isp, ISP_AE_BLK_WT_ADDR, 0);
> > >
> > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > +     if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
> > > +             zones_num = C3_ISP_AE_MAX_ZONES;
> > >
> > >       /* Need to write 8 weights at once */
> > >       for (i = 0; i < zones_num / 8; i++) {
> > >
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
>
>
>
> --
> Ricardo Ribalda



-- 
Ricardo Ribalda

Re: [PATCH 5/6] media: staging: ipu3-imgu: Add range check for imgu_css_cfg_acc_stripe

[Ricardo Ribalda]

HI Laurent

On Tue, 28 Apr 2026 at 15:13, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Tue, Apr 28, 2026 at 12:41:11PM +0000, Ricardo Ribalda wrote:
> > If the driver's stripe information is invalid it can result in an integer
> > overflow. Add a range check with a WARN_ON to expose this kind of
> > error.
> >
> > This patch fixes the following smatch error:
> > drivers/staging/media/ipu3/ipu3-css-params.c:1792 imgu_css_cfg_acc_stripe() warn: 'acc->stripe.bds_out_stripes[0]->width - 2 * f' 4294967168 can't fit into 65535 'acc->stripe.bds_out_stripes[1]->offset'
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  drivers/staging/media/ipu3/ipu3-css-params.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c
> > index 2c48d57a3180..6ed23c7a0c3f 100644
> > --- a/drivers/staging/media/ipu3/ipu3-css-params.c
> > +++ b/drivers/staging/media/ipu3/ipu3-css-params.c
> > @@ -1770,6 +1770,8 @@ static int imgu_css_cfg_acc_stripe(struct imgu_css *css, unsigned int pipe,
> >               acc->stripe.bds_out_stripes[0].width =
> >                       ALIGN(css_pipe->rect[IPU3_CSS_RECT_BDS].width, f);
> >       } else {
> > +             u32 offset;
> > +
> >               /* Image processing is divided into two stripes */
> >               acc->stripe.bds_out_stripes[0].width =
> >                       acc->stripe.bds_out_stripes[1].width =
> > @@ -1788,8 +1790,10 @@ static int imgu_css_cfg_acc_stripe(struct imgu_css *css, unsigned int pipe,
> >                       acc->stripe.bds_out_stripes[1].width += f;
> >               }
> >               /* Overlap between stripes is IPU3_UAPI_ISP_VEC_ELEMS * 4 */
> > -             acc->stripe.bds_out_stripes[1].offset =
> > -                     acc->stripe.bds_out_stripes[0].width - 2 * f;
> > +             offset = acc->stripe.bds_out_stripes[0].width - 2 * f;
> > +             if (WARN_ON(offset > 65535))
>
> If this can be triggered by userspace it shouldn't WARN_ON().

v2 will not have the WARN_ON.

Thanks!

>
> > +                     return -EINVAL;
> > +             acc->stripe.bds_out_stripes[1].offset = offset;
> >       }
> >
> >       acc->stripe.effective_stripes[0].height =
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

Re: [PATCH 2/6] media: i2c: mt9p031: Rewrite a bitwise mask

[Laurent Pinchart]

On Tue, Apr 28, 2026 at 12:41:08PM +0000, Ricardo Ribalda wrote:
> The current code makes smatch a bit uncomfortable:
> drivers/media/i2c/mt9p031.c:799 mt9p031_s_ctrl() warn: assigning (-1952) to unsigned variable 'data'
> 
> Probably because smatch is not clever enough (yet). Do a simple rewrite
> to make sure that smatch understands what we are doing here.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/i2c/mt9p031.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index ea5d43d925ff..5c9dff030b4d 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -795,7 +795,7 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
>  			ctrl->val &= ~1;
>  			data = (1 << 6) | (ctrl->val >> 1);
>  		} else {
> -			ctrl->val &= ~7;
> +			ctrl->val -= ctrl->val % 8;

This is less readable than it was :-/ Is there a way to avoid the
warning while keeping the ~7 ? Maybe by assigning

			data = (ctrl->val - 64) >> 3;
 			data = (data << 8) | (1 << 6) | 32;

>  			data = ((ctrl->val - 64) << 5) | (1 << 6) | 32;
>  		}
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/6] media: amlogic-c3: Add validations for ae and awb config

[Laurent Pinchart]

On Tue, Apr 28, 2026 at 03:14:21PM +0200, Ricardo Ribalda wrote:
> On Tue, 28 Apr 2026 at 15:10, Laurent Pinchart wrote:
> > On Tue, Apr 28, 2026 at 12:41:12PM +0000, Ricardo Ribalda wrote:
> > > Avoid invalid memory access if the zones_num is bigger than
> > > zone_weight.
> > >
> > > This patch fixes the following smatch errors:
> > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > >
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > >  drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > index 6f9ca7a7dd88..42d780f684d1 100644
> > > --- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > +++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > @@ -104,6 +104,8 @@ static void c3_isp_params_awb_wt(struct c3_isp_device *isp,
> > >       c3_isp_write(isp, ISP_AWB_BLK_WT_ADDR, 0);
> > >
> > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > +     if (WARN_ON(zones_num > C3_ISP_AWB_MAX_ZONES))
> >
> > This is triggerable by userspace, it shouldn't result in a WARN_ON().
> > Ideally the horiz_zones_num and vert_zones_num should be validated at
> > buf prepare time, and an error should be returned to userspace. That
> > will likely not fix your smatch issue though, I don't think it will be
> > able to understand that the values have been validated.
> 
> Based on the warnings from the other drivers I also suspect that if
> you have validated the data somewhere else smatch will understand it.
> 
> Even if you add a validate function I would suggest to keep the
> WARN_ON(), ideally it should never trigger, and if it triggers it will
> get a lot more attention to get it fixed.

We could keep the WARN_ON() if we first validate the data, but the
driver doesn't currently :-/ I expect there could be more similar
issues.

> > Jacopo, do we need to add a validate function pointer to
> > v4l2_isp_params_block_type_info ?
> >
> > > +             zones_num = C3_ISP_AWB_MAX_ZONES;
> > >
> > >       /* Need to write 8 weights at once */
> > >       for (i = 0; i < zones_num / 8; i++) {
> > > @@ -220,6 +222,8 @@ static void c3_isp_params_ae_wt(struct c3_isp_device *isp,
> > >       c3_isp_write(isp, ISP_AE_BLK_WT_ADDR, 0);
> > >
> > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > +     if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
> > > +             zones_num = C3_ISP_AE_MAX_ZONES;
> > >
> > >       /* Need to write 8 weights at once */
> > >       for (i = 0; i < zones_num / 8; i++) {

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/6] media: i2c: mt9p031: Rewrite a bitwise mask

[Ricardo Ribalda]

Hi Laurent

On Tue, 28 Apr 2026 at 15:25, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Tue, Apr 28, 2026 at 12:41:08PM +0000, Ricardo Ribalda wrote:
> > The current code makes smatch a bit uncomfortable:
> > drivers/media/i2c/mt9p031.c:799 mt9p031_s_ctrl() warn: assigning (-1952) to unsigned variable 'data'
> >
> > Probably because smatch is not clever enough (yet). Do a simple rewrite
> > to make sure that smatch understands what we are doing here.
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  drivers/media/i2c/mt9p031.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> > index ea5d43d925ff..5c9dff030b4d 100644
> > --- a/drivers/media/i2c/mt9p031.c
> > +++ b/drivers/media/i2c/mt9p031.c
> > @@ -795,7 +795,7 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
> >                       ctrl->val &= ~1;
> >                       data = (1 << 6) | (ctrl->val >> 1);
> >               } else {
> > -                     ctrl->val &= ~7;
> > +                     ctrl->val -= ctrl->val % 8;
>
> This is less readable than it was :-/ Is there a way to avoid the
> warning while keeping the ~7 ? Maybe by assigning

I guess it is a matter of taste. I find my version more readable.
Also yours do not seem to convince smatch :P


drivers/media/i2c/mt9p031.c:800 mt9p031_s_ctrl() warn: '(data << 8) |
(1 << 6) | 32' 16775264 can't fit into 65535 'data'

ribalda@ribalda:~/work/linux$ git diff
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 5c9dff030b4d..f9f51d5060f6 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -795,8 +795,9 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
                        ctrl->val &= ~1;
                        data = (1 << 6) | (ctrl->val >> 1);
                } else {
-                       ctrl->val -= ctrl->val % 8;
-                       data = ((ctrl->val - 64) << 5) | (1 << 6) | 32;
+                       ctrl->val &= ~7;
+                       data = (ctrl->val - 64) >> 3;
+                       data = (data << 8) | (1 << 6) | 32;

>
>                         data = (ctrl->val - 64) >> 3;
>                         data = (data << 8) | (1 << 6) | 32;
>
> >                       data = ((ctrl->val - 64) << 5) | (1 << 6) | 32;
> >               }
> >
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

Re: [PATCH 6/6] media: amlogic-c3: Add validations for ae and awb config

[Ricardo Ribalda]

On Tue, 28 Apr 2026 at 15:26, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Tue, Apr 28, 2026 at 03:14:21PM +0200, Ricardo Ribalda wrote:
> > On Tue, 28 Apr 2026 at 15:10, Laurent Pinchart wrote:
> > > On Tue, Apr 28, 2026 at 12:41:12PM +0000, Ricardo Ribalda wrote:
> > > > Avoid invalid memory access if the zones_num is bigger than
> > > > zone_weight.
> > > >
> > > > This patch fixes the following smatch errors:
> > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > >
> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > ---
> > > >  drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > index 6f9ca7a7dd88..42d780f684d1 100644
> > > > --- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > +++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > @@ -104,6 +104,8 @@ static void c3_isp_params_awb_wt(struct c3_isp_device *isp,
> > > >       c3_isp_write(isp, ISP_AWB_BLK_WT_ADDR, 0);
> > > >
> > > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > > +     if (WARN_ON(zones_num > C3_ISP_AWB_MAX_ZONES))
> > >
> > > This is triggerable by userspace, it shouldn't result in a WARN_ON().
> > > Ideally the horiz_zones_num and vert_zones_num should be validated at
> > > buf prepare time, and an error should be returned to userspace. That
> > > will likely not fix your smatch issue though, I don't think it will be
> > > able to understand that the values have been validated.
> >
> > Based on the warnings from the other drivers I also suspect that if
> > you have validated the data somewhere else smatch will understand it.
> >
> > Even if you add a validate function I would suggest to keep the
> > WARN_ON(), ideally it should never trigger, and if it triggers it will
> > get a lot more attention to get it fixed.
>
> We could keep the WARN_ON() if we first validate the data, but the
> driver doesn't currently :-/ I expect there could be more similar
> issues.

Yep, I got that. I will let you or Jacopo figure out the best way to
implement the validation in buf_prepare. If you do not have time to
implement it now I will just remove the WARN_ON in the interim... but
from my experience we only fix stuff if we get an oops.

Regards!

>
> > > Jacopo, do we need to add a validate function pointer to
> > > v4l2_isp_params_block_type_info ?
> > >
> > > > +             zones_num = C3_ISP_AWB_MAX_ZONES;
> > > >
> > > >       /* Need to write 8 weights at once */
> > > >       for (i = 0; i < zones_num / 8; i++) {
> > > > @@ -220,6 +222,8 @@ static void c3_isp_params_ae_wt(struct c3_isp_device *isp,
> > > >       c3_isp_write(isp, ISP_AE_BLK_WT_ADDR, 0);
> > > >
> > > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > > +     if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
> > > > +             zones_num = C3_ISP_AE_MAX_ZONES;
> > > >
> > > >       /* Need to write 8 weights at once */
> > > >       for (i = 0; i < zones_num / 8; i++) {
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

Re: [PATCH 0/6] media: Fix new smatch warnings

[Dan Carpenter]

On Tue, Apr 28, 2026 at 12:41:06PM +0000, Ricardo Ribalda wrote:
> Current version of smatch triggers some warnings for the media tree.
> Most of them are inoffensive, but we would like to have zero smatch
> warnings.
> 
> drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> drivers/media/v4l2-core/v4l2-dev.c:1036 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> drivers/media/v4l2-core/v4l2-dev.c:1043 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> drivers/media/v4l2-core/v4l2-dev.c:1101 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> drivers/media/platform/chips-media/wave5/wave5-vpuapi.c:588 wave5_vpu_dec_get_output_info() error: buffer overflow 'inst->frame_buf' 64 <= 127
> drivers/staging/media/ipu3/ipu3-css-params.c:1792 imgu_css_cfg_acc_stripe() warn: 'acc->stripe.bds_out_stripes[0]->width - 2 * f' 4294967168 can't fit into 65535 'acc->stripe.bds_out_stripes[1]->offset'
> drivers/media/i2c/adv7604.c:3672 adv76xx_probe() error: buffer overflow 'state->pads' 7 <= 4294967294
> drivers/media/i2c/adv7604.c:3673 adv76xx_probe() error: buffer overflow 'state->pads' 7 <= u32max
> drivers/media/i2c/mt9p031.c:799 mt9p031_s_ctrl() warn: assigning (-1952) to unsigned variable 'data'
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

I'm re-writing a bunch of core stuff right now...  Feel free to
complain about false positives.  I'm going to re-write the buffer
overflow warning in the next couple weeks.

regards,
dan carpenter

Re: [PATCH 0/6] media: Fix new smatch warnings

[Ricardo Ribalda]

Hi Dan

On Tue, 28 Apr 2026 at 15:52, Dan Carpenter <error27@gmail.com> wrote:
>
> On Tue, Apr 28, 2026 at 12:41:06PM +0000, Ricardo Ribalda wrote:
> > Current version of smatch triggers some warnings for the media tree.
> > Most of them are inoffensive, but we would like to have zero smatch
> > warnings.
> >
> > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > drivers/media/v4l2-core/v4l2-dev.c:1036 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> > drivers/media/v4l2-core/v4l2-dev.c:1043 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> > drivers/media/v4l2-core/v4l2-dev.c:1101 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> > drivers/media/platform/chips-media/wave5/wave5-vpuapi.c:588 wave5_vpu_dec_get_output_info() error: buffer overflow 'inst->frame_buf' 64 <= 127
> > drivers/staging/media/ipu3/ipu3-css-params.c:1792 imgu_css_cfg_acc_stripe() warn: 'acc->stripe.bds_out_stripes[0]->width - 2 * f' 4294967168 can't fit into 65535 'acc->stripe.bds_out_stripes[1]->offset'
> > drivers/media/i2c/adv7604.c:3672 adv76xx_probe() error: buffer overflow 'state->pads' 7 <= 4294967294
> > drivers/media/i2c/adv7604.c:3673 adv76xx_probe() error: buffer overflow 'state->pads' 7 <= u32max
> > drivers/media/i2c/mt9p031.c:799 mt9p031_s_ctrl() warn: assigning (-1952) to unsigned variable 'data'
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>
> I'm re-writing a bunch of core stuff right now...  Feel free to
> complain about false positives.  I'm going to re-write the buffer
> overflow warning in the next couple weeks.

The only one that deserves a complain is this one:
https://lore.kernel.org/linux-media/CANiDSCtm4Nh4Ub4rbEBvpjV8GXT9VQ5eFXZTHn=Wy=0RpR=3JA@mail.gmail.com/T/#m650723c33ec0318d8f32f1a6cc74c74a952ae11a

There are other false positives like this one:
https://lore.kernel.org/linux-media/CANiDSCtm4Nh4Ub4rbEBvpjV8GXT9VQ5eFXZTHn=Wy=0RpR=3JA@mail.gmail.com/T/#md58851baa54c511f57b05a4dcf3aecf0ffb1b1fa
But I think the extra check makes the code more robust.

Thanks for your tool :)

>
> regards,
> dan carpenter



-- 
Ricardo Ribalda

Re: [PATCH 6/6] media: amlogic-c3: Add validations for ae and awb config

[Jacopo Mondi]

Hello

   thank you Ricardo for the fix

On Tue, Apr 28, 2026 at 03:49:49PM +0200, Ricardo Ribalda wrote:
> On Tue, 28 Apr 2026 at 15:26, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > On Tue, Apr 28, 2026 at 03:14:21PM +0200, Ricardo Ribalda wrote:
> > > On Tue, 28 Apr 2026 at 15:10, Laurent Pinchart wrote:
> > > > On Tue, Apr 28, 2026 at 12:41:12PM +0000, Ricardo Ribalda wrote:
> > > > > Avoid invalid memory access if the zones_num is bigger than
> > > > > zone_weight.
> > > > >
> > > > > This patch fixes the following smatch errors:
> > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > > >
> > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > ---
> > > > >  drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > index 6f9ca7a7dd88..42d780f684d1 100644
> > > > > --- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > +++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > @@ -104,6 +104,8 @@ static void c3_isp_params_awb_wt(struct c3_isp_device *isp,
> > > > >       c3_isp_write(isp, ISP_AWB_BLK_WT_ADDR, 0);
> > > > >
> > > > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > > > +     if (WARN_ON(zones_num > C3_ISP_AWB_MAX_ZONES))
> > > >
> > > > This is triggerable by userspace, it shouldn't result in a WARN_ON().
> > > > Ideally the horiz_zones_num and vert_zones_num should be validated at
> > > > buf prepare time, and an error should be returned to userspace. That
> > > > will likely not fix your smatch issue though, I don't think it will be
> > > > able to understand that the values have been validated.
> > >
> > > Based on the warnings from the other drivers I also suspect that if
> > > you have validated the data somewhere else smatch will understand it.
> > >
> > > Even if you add a validate function I would suggest to keep the
> > > WARN_ON(), ideally it should never trigger, and if it triggers it will
> > > get a lot more attention to get it fixed.
> >
> > We could keep the WARN_ON() if we first validate the data, but the
> > driver doesn't currently :-/ I expect there could be more similar
> > issues.
>
> Yep, I got that. I will let you or Jacopo figure out the best way to
> implement the validation in buf_prepare. If you do not have time to
> implement it now I will just remove the WARN_ON in the interim... but
> from my experience we only fix stuff if we get an oops.
>
> Regards!
>
> >
> > > > Jacopo, do we need to add a validate function pointer to
> > > > v4l2_isp_params_block_type_info ?

To allow drivers to provide an additional per-block validation
function ? I think it could be nice indeed.

Ricardo, could you spare this patch for the moment ? I think we can
WARN_ON() to please smatch but we should pre-validate the buffer (without
spamming the system log in case of errors) to make sure we actually
never hit the WARN_ON() :)

I have some patches in the pipe for v4l2-isp to add support for
extensible stats, I could pile up a few more to give drivers a space
where to implement additional per-block validations

> > > >
> > > > > +             zones_num = C3_ISP_AWB_MAX_ZONES;
> > > > >
> > > > >       /* Need to write 8 weights at once */
> > > > >       for (i = 0; i < zones_num / 8; i++) {
> > > > > @@ -220,6 +222,8 @@ static void c3_isp_params_ae_wt(struct c3_isp_device *isp,
> > > > >       c3_isp_write(isp, ISP_AE_BLK_WT_ADDR, 0);
> > > > >
> > > > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > > > +     if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
> > > > > +             zones_num = C3_ISP_AE_MAX_ZONES;
> > > > >
> > > > >       /* Need to write 8 weights at once */
> > > > >       for (i = 0; i < zones_num / 8; i++) {
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
>
>
>
> --
> Ricardo Ribalda

Re: [PATCH 6/6] media: amlogic-c3: Add validations for ae and awb config

[Ricardo Ribalda]

Hi Jacopo

On Wed, 29 Apr 2026 at 08:15, Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hello
>
>    thank you Ricardo for the fix
>
> On Tue, Apr 28, 2026 at 03:49:49PM +0200, Ricardo Ribalda wrote:
> > On Tue, 28 Apr 2026 at 15:26, Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> > >
> > > On Tue, Apr 28, 2026 at 03:14:21PM +0200, Ricardo Ribalda wrote:
> > > > On Tue, 28 Apr 2026 at 15:10, Laurent Pinchart wrote:
> > > > > On Tue, Apr 28, 2026 at 12:41:12PM +0000, Ricardo Ribalda wrote:
> > > > > > Avoid invalid memory access if the zones_num is bigger than
> > > > > > zone_weight.
> > > > > >
> > > > > > This patch fixes the following smatch errors:
> > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > > > >
> > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > > ---
> > > > > >  drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 4 ++++
> > > > > >  1 file changed, 4 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > > index 6f9ca7a7dd88..42d780f684d1 100644
> > > > > > --- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > > +++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > > @@ -104,6 +104,8 @@ static void c3_isp_params_awb_wt(struct c3_isp_device *isp,
> > > > > >       c3_isp_write(isp, ISP_AWB_BLK_WT_ADDR, 0);
> > > > > >
> > > > > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > > > > +     if (WARN_ON(zones_num > C3_ISP_AWB_MAX_ZONES))
> > > > >
> > > > > This is triggerable by userspace, it shouldn't result in a WARN_ON().
> > > > > Ideally the horiz_zones_num and vert_zones_num should be validated at
> > > > > buf prepare time, and an error should be returned to userspace. That
> > > > > will likely not fix your smatch issue though, I don't think it will be
> > > > > able to understand that the values have been validated.
> > > >
> > > > Based on the warnings from the other drivers I also suspect that if
> > > > you have validated the data somewhere else smatch will understand it.
> > > >
> > > > Even if you add a validate function I would suggest to keep the
> > > > WARN_ON(), ideally it should never trigger, and if it triggers it will
> > > > get a lot more attention to get it fixed.
> > >
> > > We could keep the WARN_ON() if we first validate the data, but the
> > > driver doesn't currently :-/ I expect there could be more similar
> > > issues.
> >
> > Yep, I got that. I will let you or Jacopo figure out the best way to
> > implement the validation in buf_prepare. If you do not have time to
> > implement it now I will just remove the WARN_ON in the interim... but
> > from my experience we only fix stuff if we get an oops.
> >
> > Regards!
> >
> > >
> > > > > Jacopo, do we need to add a validate function pointer to
> > > > > v4l2_isp_params_block_type_info ?
>
> To allow drivers to provide an additional per-block validation
> function ? I think it could be nice indeed.
>
> Ricardo, could you spare this patch for the moment ? I think we can
> WARN_ON() to please smatch but we should pre-validate the buffer (without
> spamming the system log in case of errors) to make sure we actually
> never hit the WARN_ON() :)
>
> I have some patches in the pipe for v4l2-isp to add support for
> extensible stats, I could pile up a few more to give drivers a space
> where to implement additional per-block validations

Do you have any idea of the timeline for this?

I would really like to land this in this cycle. If it is going to take
long maybe i can just

if (zones_num > C3_ISP_AE_MAX_ZONES)

and then when you add your checks you can promote it to:

if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))

?

Also it would be much easier to backport this change than a change in v4l2-isp.

Regards!

>
> > > > >
> > > > > > +             zones_num = C3_ISP_AWB_MAX_ZONES;
> > > > > >
> > > > > >       /* Need to write 8 weights at once */
> > > > > >       for (i = 0; i < zones_num / 8; i++) {
> > > > > > @@ -220,6 +222,8 @@ static void c3_isp_params_ae_wt(struct c3_isp_device *isp,
> > > > > >       c3_isp_write(isp, ISP_AE_BLK_WT_ADDR, 0);
> > > > > >
> > > > > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > > > > +     if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
> > > > > > +             zones_num = C3_ISP_AE_MAX_ZONES;
> > > > > >
> > > > > >       /* Need to write 8 weights at once */
> > > > > >       for (i = 0; i < zones_num / 8; i++) {
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart
> >
> >
> >
> > --
> > Ricardo Ribalda



-- 
Ricardo Ribalda

Re: [PATCH 6/6] media: amlogic-c3: Add validations for ae and awb config

[Jacopo Mondi]

Hi Ricardo

On Wed, Apr 29, 2026 at 08:44:11AM +0200, Ricardo Ribalda wrote:
> Hi Jacopo
>
> On Wed, 29 Apr 2026 at 08:15, Jacopo Mondi
> <jacopo.mondi@ideasonboard.com> wrote:
> >
> > Hello
> >
> >    thank you Ricardo for the fix
> >
> > On Tue, Apr 28, 2026 at 03:49:49PM +0200, Ricardo Ribalda wrote:
> > > On Tue, 28 Apr 2026 at 15:26, Laurent Pinchart
> > > <laurent.pinchart@ideasonboard.com> wrote:
> > > >
> > > > On Tue, Apr 28, 2026 at 03:14:21PM +0200, Ricardo Ribalda wrote:
> > > > > On Tue, 28 Apr 2026 at 15:10, Laurent Pinchart wrote:
> > > > > > On Tue, Apr 28, 2026 at 12:41:12PM +0000, Ricardo Ribalda wrote:
> > > > > > > Avoid invalid memory access if the zones_num is bigger than
> > > > > > > zone_weight.
> > > > > > >
> > > > > > > This patch fixes the following smatch errors:
> > > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > > > > >
> > > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > > > ---
> > > > > > >  drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 4 ++++
> > > > > > >  1 file changed, 4 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > > > index 6f9ca7a7dd88..42d780f684d1 100644
> > > > > > > --- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > > > +++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > > > @@ -104,6 +104,8 @@ static void c3_isp_params_awb_wt(struct c3_isp_device *isp,
> > > > > > >       c3_isp_write(isp, ISP_AWB_BLK_WT_ADDR, 0);
> > > > > > >
> > > > > > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > > > > > +     if (WARN_ON(zones_num > C3_ISP_AWB_MAX_ZONES))
> > > > > >
> > > > > > This is triggerable by userspace, it shouldn't result in a WARN_ON().
> > > > > > Ideally the horiz_zones_num and vert_zones_num should be validated at
> > > > > > buf prepare time, and an error should be returned to userspace. That
> > > > > > will likely not fix your smatch issue though, I don't think it will be
> > > > > > able to understand that the values have been validated.
> > > > >
> > > > > Based on the warnings from the other drivers I also suspect that if
> > > > > you have validated the data somewhere else smatch will understand it.
> > > > >
> > > > > Even if you add a validate function I would suggest to keep the
> > > > > WARN_ON(), ideally it should never trigger, and if it triggers it will
> > > > > get a lot more attention to get it fixed.
> > > >
> > > > We could keep the WARN_ON() if we first validate the data, but the
> > > > driver doesn't currently :-/ I expect there could be more similar
> > > > issues.
> > >
> > > Yep, I got that. I will let you or Jacopo figure out the best way to
> > > implement the validation in buf_prepare. If you do not have time to
> > > implement it now I will just remove the WARN_ON in the interim... but
> > > from my experience we only fix stuff if we get an oops.
> > >
> > > Regards!
> > >
> > > >
> > > > > > Jacopo, do we need to add a validate function pointer to
> > > > > > v4l2_isp_params_block_type_info ?
> >
> > To allow drivers to provide an additional per-block validation
> > function ? I think it could be nice indeed.
> >
> > Ricardo, could you spare this patch for the moment ? I think we can
> > WARN_ON() to please smatch but we should pre-validate the buffer (without
> > spamming the system log in case of errors) to make sure we actually
> > never hit the WARN_ON() :)
> >
> > I have some patches in the pipe for v4l2-isp to add support for
> > extensible stats, I could pile up a few more to give drivers a space
> > where to implement additional per-block validations
>
> Do you have any idea of the timeline for this?
>

Give the change will likely come on top of extensible stats it might
slip this cycle

> I would really like to land this in this cycle. If it is going to take
> long maybe i can just
>
> if (zones_num > C3_ISP_AE_MAX_ZONES)

Fine by me

>
> and then when you add your checks you can promote it to:
>
> if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))

To be honest, if we pre-validate and silence the smatch warning with
the above

        if (zones_num > C3_ISP_AE_MAX_ZONES)

then there shouldn't be any need to WARN_ON() ?

>
> ?
>
> Also it would be much easier to backport this change than a change in v4l2-isp.
>
> Regards!
>
> >
> > > > > >
> > > > > > > +             zones_num = C3_ISP_AWB_MAX_ZONES;
> > > > > > >
> > > > > > >       /* Need to write 8 weights at once */
> > > > > > >       for (i = 0; i < zones_num / 8; i++) {
> > > > > > > @@ -220,6 +222,8 @@ static void c3_isp_params_ae_wt(struct c3_isp_device *isp,
> > > > > > >       c3_isp_write(isp, ISP_AE_BLK_WT_ADDR, 0);
> > > > > > >
> > > > > > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > > > > > +     if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
> > > > > > > +             zones_num = C3_ISP_AE_MAX_ZONES;
> > > > > > >
> > > > > > >       /* Need to write 8 weights at once */
> > > > > > >       for (i = 0; i < zones_num / 8; i++) {
> > > >
> > > > --
> > > > Regards,
> > > >
> > > > Laurent Pinchart
> > >
> > >
> > >
> > > --
> > > Ricardo Ribalda
>
>
>
> --
> Ricardo Ribalda

Re: [PATCH 6/6] media: amlogic-c3: Add validations for ae and awb config

[Ricardo Ribalda]

Hi Jacopo

On Wed, 29 Apr 2026 at 08:55, Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Ricardo
>
> On Wed, Apr 29, 2026 at 08:44:11AM +0200, Ricardo Ribalda wrote:
> > Hi Jacopo
> >
> > On Wed, 29 Apr 2026 at 08:15, Jacopo Mondi
> > <jacopo.mondi@ideasonboard.com> wrote:
> > >
> > > Hello
> > >
> > >    thank you Ricardo for the fix
> > >
> > > On Tue, Apr 28, 2026 at 03:49:49PM +0200, Ricardo Ribalda wrote:
> > > > On Tue, 28 Apr 2026 at 15:26, Laurent Pinchart
> > > > <laurent.pinchart@ideasonboard.com> wrote:
> > > > >
> > > > > On Tue, Apr 28, 2026 at 03:14:21PM +0200, Ricardo Ribalda wrote:
> > > > > > On Tue, 28 Apr 2026 at 15:10, Laurent Pinchart wrote:
> > > > > > > On Tue, Apr 28, 2026 at 12:41:12PM +0000, Ricardo Ribalda wrote:
> > > > > > > > Avoid invalid memory access if the zones_num is bigger than
> > > > > > > > zone_weight.
> > > > > > > >
> > > > > > > > This patch fixes the following smatch errors:
> > > > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > > > > > >
> > > > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > > > > ---
> > > > > > > >  drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 4 ++++
> > > > > > > >  1 file changed, 4 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > > > > index 6f9ca7a7dd88..42d780f684d1 100644
> > > > > > > > --- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > > > > +++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > > > > @@ -104,6 +104,8 @@ static void c3_isp_params_awb_wt(struct c3_isp_device *isp,
> > > > > > > >       c3_isp_write(isp, ISP_AWB_BLK_WT_ADDR, 0);
> > > > > > > >
> > > > > > > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > > > > > > +     if (WARN_ON(zones_num > C3_ISP_AWB_MAX_ZONES))
> > > > > > >
> > > > > > > This is triggerable by userspace, it shouldn't result in a WARN_ON().
> > > > > > > Ideally the horiz_zones_num and vert_zones_num should be validated at
> > > > > > > buf prepare time, and an error should be returned to userspace. That
> > > > > > > will likely not fix your smatch issue though, I don't think it will be
> > > > > > > able to understand that the values have been validated.
> > > > > >
> > > > > > Based on the warnings from the other drivers I also suspect that if
> > > > > > you have validated the data somewhere else smatch will understand it.
> > > > > >
> > > > > > Even if you add a validate function I would suggest to keep the
> > > > > > WARN_ON(), ideally it should never trigger, and if it triggers it will
> > > > > > get a lot more attention to get it fixed.
> > > > >
> > > > > We could keep the WARN_ON() if we first validate the data, but the
> > > > > driver doesn't currently :-/ I expect there could be more similar
> > > > > issues.
> > > >
> > > > Yep, I got that. I will let you or Jacopo figure out the best way to
> > > > implement the validation in buf_prepare. If you do not have time to
> > > > implement it now I will just remove the WARN_ON in the interim... but
> > > > from my experience we only fix stuff if we get an oops.
> > > >
> > > > Regards!
> > > >
> > > > >
> > > > > > > Jacopo, do we need to add a validate function pointer to
> > > > > > > v4l2_isp_params_block_type_info ?
> > >
> > > To allow drivers to provide an additional per-block validation
> > > function ? I think it could be nice indeed.
> > >
> > > Ricardo, could you spare this patch for the moment ? I think we can
> > > WARN_ON() to please smatch but we should pre-validate the buffer (without
> > > spamming the system log in case of errors) to make sure we actually
> > > never hit the WARN_ON() :)
> > >
> > > I have some patches in the pipe for v4l2-isp to add support for
> > > extensible stats, I could pile up a few more to give drivers a space
> > > where to implement additional per-block validations
> >
> > Do you have any idea of the timeline for this?
> >
>
> Give the change will likely come on top of extensible stats it might
> slip this cycle
>
> > I would really like to land this in this cycle. If it is going to take
> > long maybe i can just
> >
> > if (zones_num > C3_ISP_AE_MAX_ZONES)
>
> Fine by me
>
> >
> > and then when you add your checks you can promote it to:
> >
> > if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
>
> To be honest, if we pre-validate and silence the smatch warning with
> the above
>
>         if (zones_num > C3_ISP_AE_MAX_ZONES)
>
> then there shouldn't be any need to WARN_ON() ?

I like WARN_ON in "sanity checks" because when they fail they give you
a pretty verbose warning that will lead to resolution.
But if you do not need/like it, it is also fine by me.


>
> >
> > ?
> >
> > Also it would be much easier to backport this change than a change in v4l2-isp.
> >
> > Regards!
> >
> > >
> > > > > > >
> > > > > > > > +             zones_num = C3_ISP_AWB_MAX_ZONES;
> > > > > > > >
> > > > > > > >       /* Need to write 8 weights at once */
> > > > > > > >       for (i = 0; i < zones_num / 8; i++) {
> > > > > > > > @@ -220,6 +222,8 @@ static void c3_isp_params_ae_wt(struct c3_isp_device *isp,
> > > > > > > >       c3_isp_write(isp, ISP_AE_BLK_WT_ADDR, 0);
> > > > > > > >
> > > > > > > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > > > > > > +     if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
> > > > > > > > +             zones_num = C3_ISP_AE_MAX_ZONES;
> > > > > > > >
> > > > > > > >       /* Need to write 8 weights at once */
> > > > > > > >       for (i = 0; i < zones_num / 8; i++) {
> > > > >
> > > > > --
> > > > > Regards,
> > > > >
> > > > > Laurent Pinchart
> > > >
> > > >
> > > >
> > > > --
> > > > Ricardo Ribalda
> >
> >
> >
> > --
> > Ricardo Ribalda



-- 
Ricardo Ribalda

Re: [PATCH 0/6] media: Fix new smatch warnings

[Laurent Pinchart]

On Tue, Apr 28, 2026 at 03:58:08PM +0200, Ricardo Ribalda wrote:
> On Tue, 28 Apr 2026 at 15:52, Dan Carpenter wrote:
> > On Tue, Apr 28, 2026 at 12:41:06PM +0000, Ricardo Ribalda wrote:
> > > Current version of smatch triggers some warnings for the media tree.
> > > Most of them are inoffensive, but we would like to have zero smatch
> > > warnings.
> > >
> > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > drivers/media/v4l2-core/v4l2-dev.c:1036 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> > > drivers/media/v4l2-core/v4l2-dev.c:1043 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> > > drivers/media/v4l2-core/v4l2-dev.c:1101 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> > > drivers/media/platform/chips-media/wave5/wave5-vpuapi.c:588 wave5_vpu_dec_get_output_info() error: buffer overflow 'inst->frame_buf' 64 <= 127
> > > drivers/staging/media/ipu3/ipu3-css-params.c:1792 imgu_css_cfg_acc_stripe() warn: 'acc->stripe.bds_out_stripes[0]->width - 2 * f' 4294967168 can't fit into 65535 'acc->stripe.bds_out_stripes[1]->offset'
> > > drivers/media/i2c/adv7604.c:3672 adv76xx_probe() error: buffer overflow 'state->pads' 7 <= 4294967294
> > > drivers/media/i2c/adv7604.c:3673 adv76xx_probe() error: buffer overflow 'state->pads' 7 <= u32max
> > > drivers/media/i2c/mt9p031.c:799 mt9p031_s_ctrl() warn: assigning (-1952) to unsigned variable 'data'
> > >
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> >
> > I'm re-writing a bunch of core stuff right now...  Feel free to
> > complain about false positives.  I'm going to re-write the buffer
> > overflow warning in the next couple weeks.
> 
> The only one that deserves a complain is this one:
> https://lore.kernel.org/linux-media/CANiDSCtm4Nh4Ub4rbEBvpjV8GXT9VQ5eFXZTHn=Wy=0RpR=3JA@mail.gmail.com/T/#m650723c33ec0318d8f32f1a6cc74c74a952ae11a
> 
> There are other false positives like this one:
> https://lore.kernel.org/linux-media/CANiDSCtm4Nh4Ub4rbEBvpjV8GXT9VQ5eFXZTHn=Wy=0RpR=3JA@mail.gmail.com/T/#md58851baa54c511f57b05a4dcf3aecf0ffb1b1fa
> But I think the extra check makes the code more robust.

I think there's also a more general question. How can we tell smatch
(and other static analysis tools) that a value has been checked
elsewhere and is guaranteed to be within certain bounds, without
performing runtime bounds checking at the site where the value is used ?

> Thanks for your tool :)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/6] media: amlogic-c3: Add validations for ae and awb config

[Jacopo Mondi]

Hi Ricardo

On Wed, Apr 29, 2026 at 09:15:59AM +0200, Ricardo Ribalda wrote:
> Hi Jacopo
>
> On Wed, 29 Apr 2026 at 08:55, Jacopo Mondi
> <jacopo.mondi@ideasonboard.com> wrote:
> >
> > Hi Ricardo
> >
> > On Wed, Apr 29, 2026 at 08:44:11AM +0200, Ricardo Ribalda wrote:
> > > Hi Jacopo
> > >
> > > On Wed, 29 Apr 2026 at 08:15, Jacopo Mondi
> > > <jacopo.mondi@ideasonboard.com> wrote:
> > > >
> > > > Hello
> > > >
> > > >    thank you Ricardo for the fix
> > > >
> > > > On Tue, Apr 28, 2026 at 03:49:49PM +0200, Ricardo Ribalda wrote:
> > > > > On Tue, 28 Apr 2026 at 15:26, Laurent Pinchart
> > > > > <laurent.pinchart@ideasonboard.com> wrote:
> > > > > >
> > > > > > On Tue, Apr 28, 2026 at 03:14:21PM +0200, Ricardo Ribalda wrote:
> > > > > > > On Tue, 28 Apr 2026 at 15:10, Laurent Pinchart wrote:
> > > > > > > > On Tue, Apr 28, 2026 at 12:41:12PM +0000, Ricardo Ribalda wrote:
> > > > > > > > > Avoid invalid memory access if the zones_num is bigger than
> > > > > > > > > zone_weight.
> > > > > > > > >
> > > > > > > > > This patch fixes the following smatch errors:
> > > > > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > > > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:111 c3_isp_params_awb_wt() error: buffer overflow 'cfg->zone_weight' 768 <= u32max
> > > > > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > > > > > > > drivers/media/platform/amlogic/c3/isp/c3-isp-params.c:227 c3_isp_params_ae_wt() error: buffer overflow 'cfg->zone_weight' 255 <= u32max
> > > > > > > > >
> > > > > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > > > > > ---
> > > > > > > > >  drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 4 ++++
> > > > > > > > >  1 file changed, 4 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > > > > > index 6f9ca7a7dd88..42d780f684d1 100644
> > > > > > > > > --- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > > > > > +++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> > > > > > > > > @@ -104,6 +104,8 @@ static void c3_isp_params_awb_wt(struct c3_isp_device *isp,
> > > > > > > > >       c3_isp_write(isp, ISP_AWB_BLK_WT_ADDR, 0);
> > > > > > > > >
> > > > > > > > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > > > > > > > +     if (WARN_ON(zones_num > C3_ISP_AWB_MAX_ZONES))
> > > > > > > >
> > > > > > > > This is triggerable by userspace, it shouldn't result in a WARN_ON().
> > > > > > > > Ideally the horiz_zones_num and vert_zones_num should be validated at
> > > > > > > > buf prepare time, and an error should be returned to userspace. That
> > > > > > > > will likely not fix your smatch issue though, I don't think it will be
> > > > > > > > able to understand that the values have been validated.
> > > > > > >
> > > > > > > Based on the warnings from the other drivers I also suspect that if
> > > > > > > you have validated the data somewhere else smatch will understand it.
> > > > > > >
> > > > > > > Even if you add a validate function I would suggest to keep the
> > > > > > > WARN_ON(), ideally it should never trigger, and if it triggers it will
> > > > > > > get a lot more attention to get it fixed.
> > > > > >
> > > > > > We could keep the WARN_ON() if we first validate the data, but the
> > > > > > driver doesn't currently :-/ I expect there could be more similar
> > > > > > issues.
> > > > >
> > > > > Yep, I got that. I will let you or Jacopo figure out the best way to
> > > > > implement the validation in buf_prepare. If you do not have time to
> > > > > implement it now I will just remove the WARN_ON in the interim... but
> > > > > from my experience we only fix stuff if we get an oops.
> > > > >
> > > > > Regards!
> > > > >
> > > > > >
> > > > > > > > Jacopo, do we need to add a validate function pointer to
> > > > > > > > v4l2_isp_params_block_type_info ?
> > > >
> > > > To allow drivers to provide an additional per-block validation
> > > > function ? I think it could be nice indeed.
> > > >
> > > > Ricardo, could you spare this patch for the moment ? I think we can
> > > > WARN_ON() to please smatch but we should pre-validate the buffer (without
> > > > spamming the system log in case of errors) to make sure we actually
> > > > never hit the WARN_ON() :)
> > > >
> > > > I have some patches in the pipe for v4l2-isp to add support for
> > > > extensible stats, I could pile up a few more to give drivers a space
> > > > where to implement additional per-block validations
> > >
> > > Do you have any idea of the timeline for this?
> > >
> >
> > Give the change will likely come on top of extensible stats it might
> > slip this cycle
> >
> > > I would really like to land this in this cycle. If it is going to take
> > > long maybe i can just
> > >
> > > if (zones_num > C3_ISP_AE_MAX_ZONES)
> >
> > Fine by me
> >
> > >
> > > and then when you add your checks you can promote it to:
> > >
> > > if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
> >
> > To be honest, if we pre-validate and silence the smatch warning with
> > the above
> >
> >         if (zones_num > C3_ISP_AE_MAX_ZONES)
> >
> > then there shouldn't be any need to WARN_ON() ?
>
> I like WARN_ON in "sanity checks" because when they fail they give you
> a pretty verbose warning that will lead to resolution.
> But if you do not need/like it, it is also fine by me.

once we add validation, it should never be triggered, so WARN_ON is
fine with me.

In general, whenever an error is triggerable by userspace imho it
shouldn't get to the system's log by default. If someone hits an error
while developing a userspace component it should enable debug on the module
to make a more verbose error description visibile ?

>
>
> >
> > >
> > > ?
> > >
> > > Also it would be much easier to backport this change than a change in v4l2-isp.
> > >
> > > Regards!
> > >
> > > >
> > > > > > > >
> > > > > > > > > +             zones_num = C3_ISP_AWB_MAX_ZONES;
> > > > > > > > >
> > > > > > > > >       /* Need to write 8 weights at once */
> > > > > > > > >       for (i = 0; i < zones_num / 8; i++) {
> > > > > > > > > @@ -220,6 +222,8 @@ static void c3_isp_params_ae_wt(struct c3_isp_device *isp,
> > > > > > > > >       c3_isp_write(isp, ISP_AE_BLK_WT_ADDR, 0);
> > > > > > > > >
> > > > > > > > >       zones_num = cfg->horiz_zones_num * cfg->vert_zones_num;
> > > > > > > > > +     if (WARN_ON(zones_num > C3_ISP_AE_MAX_ZONES))
> > > > > > > > > +             zones_num = C3_ISP_AE_MAX_ZONES;
> > > > > > > > >
> > > > > > > > >       /* Need to write 8 weights at once */
> > > > > > > > >       for (i = 0; i < zones_num / 8; i++) {
> > > > > >
> > > > > > --
> > > > > > Regards,
> > > > > >
> > > > > > Laurent Pinchart
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Ricardo Ribalda
> > >
> > >
> > >
> > > --
> > > Ricardo Ribalda
>
>
>
> --
> Ricardo Ribalda

Re: [PATCH 1/6] media: v4l2-dev: Add range check for vdev->minor

[Hans Verkuil]

On 28/04/2026 14:41, Ricardo Ribalda wrote:
> If the fixed minor ranges are not properly set we could end up in a
> situation where the calculated minor is invalid. Add a check for this in
> the code.

the code -> the code to make it more robust.

> 
> This check also fixes the following smatch warning:

I'd say:

This check also fixes the following false positive smatch warnings:

> 
> drivers/media/v4l2-core/v4l2-dev.c:1036 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> drivers/media/v4l2-core/v4l2-dev.c:1043 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> drivers/media/v4l2-core/v4l2-dev.c:1101 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

The code in __video_register_device is quite complex, so adding this check is
worthwhile. So even though the warning is a false positive, it's a good change.

Regards,

	Hans

> ---
>  drivers/media/v4l2-core/v4l2-dev.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 6ce623a1245a..a731ffdb91ee 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -1032,6 +1032,12 @@ int __video_register_device(struct video_device *vdev,
>  	vdev->minor = i + minor_offset;
>  	vdev->num = nr;
>  
> +	if (WARN_ON(vdev->minor >= VIDEO_NUM_DEVICES)) {
> +		mutex_unlock(&videodev_lock);
> +		pr_err("invalid minor. Check ranges.\n");
> +		return -EINVAL;
> +	}
> +
>  	/* Should not happen since we thought this minor was free */
>  	if (WARN_ON(video_devices[vdev->minor])) {
>  		mutex_unlock(&videodev_lock);
> 

Re: [PATCH 3/6] media: i2c: adv7604: Add range checks for chip info

[Hans Verkuil]

On 28/04/2026 14:41, Ricardo Ribalda wrote:
> If the driver's chip information is invalid we can end up accessing an
> invalid memory region.
> 
> This fixes the following smatch errors:

"This fixes the following false positive smatch errors:"

> drivers/media/i2c/adv7604.c:3672 adv76xx_probe() error: buffer overflow 'state->pads' 7 <= 4294967294
> drivers/media/i2c/adv7604.c:3673 adv76xx_probe() error: buffer overflow 'state->pads' 7 <= u32max
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Reviewed-by: Hans Verkuil <hverkuil+cisco@kernel.org>

> ---
>  drivers/media/i2c/adv7604.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 67116a4ef134..ae75982fb514 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -3668,6 +3668,12 @@ static int adv76xx_probe(struct i2c_client *client)
>  
>  	state->source_pad = state->info->num_dv_ports
>  			  + (state->info->has_afe ? 2 : 0);
> +	if (WARN_ON(state->source_pad >= ADV76XX_PAD_MAX)) {
> +		err = -EINVAL;
> +		v4l2_err(sd, "invalid chip info\n");
> +		goto err_i2c;
> +	}
> +
>  	for (i = 0; i < state->source_pad; ++i)
>  		state->pads[i].flags = MEDIA_PAD_FL_SINK;
>  	state->pads[state->source_pad].flags = MEDIA_PAD_FL_SOURCE;
> 

Re: [PATCH 1/6] media: v4l2-dev: Add range check for vdev->minor

[Sakari Ailus]

Hi Ricardo,

On Tue, Apr 28, 2026 at 12:41:07PM +0000, Ricardo Ribalda wrote:
> If the fixed minor ranges are not properly set we could end up in a
> situation where the calculated minor is invalid. Add a check for this in
> the code.
> 
> This check also fixes the following smatch warning:
> 
> drivers/media/v4l2-core/v4l2-dev.c:1036 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> drivers/media/v4l2-core/v4l2-dev.c:1043 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> drivers/media/v4l2-core/v4l2-dev.c:1101 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/v4l2-core/v4l2-dev.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 6ce623a1245a..a731ffdb91ee 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -1032,6 +1032,12 @@ int __video_register_device(struct video_device *vdev,
>  	vdev->minor = i + minor_offset;
>  	vdev->num = nr;
>  
> +	if (WARN_ON(vdev->minor >= VIDEO_NUM_DEVICES)) {

Could this be combined with the should-not-happen case below? The error
handling is the same (releasing the mutex) and the error code could be as
well. I think the message can be just as well removed as we have a
WARN_ON() here anyway.

I wonder what Hans thinks.

> +		mutex_unlock(&videodev_lock);
> +		pr_err("invalid minor. Check ranges.\n");
> +		return -EINVAL;
> +	}
> +
>  	/* Should not happen since we thought this minor was free */
>  	if (WARN_ON(video_devices[vdev->minor])) {
>  		mutex_unlock(&videodev_lock);
> 

-- 
Kind regards,

Sakari Ailus

Re: [PATCH 1/6] media: v4l2-dev: Add range check for vdev->minor

[Hans Verkuil]

On 29/04/2026 09:43, Sakari Ailus wrote:
> Hi Ricardo,
> 
> On Tue, Apr 28, 2026 at 12:41:07PM +0000, Ricardo Ribalda wrote:
>> If the fixed minor ranges are not properly set we could end up in a
>> situation where the calculated minor is invalid. Add a check for this in
>> the code.
>>
>> This check also fixes the following smatch warning:
>>
>> drivers/media/v4l2-core/v4l2-dev.c:1036 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
>> drivers/media/v4l2-core/v4l2-dev.c:1043 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
>> drivers/media/v4l2-core/v4l2-dev.c:1101 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
>>
>> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>> ---
>>  drivers/media/v4l2-core/v4l2-dev.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
>> index 6ce623a1245a..a731ffdb91ee 100644
>> --- a/drivers/media/v4l2-core/v4l2-dev.c
>> +++ b/drivers/media/v4l2-core/v4l2-dev.c
>> @@ -1032,6 +1032,12 @@ int __video_register_device(struct video_device *vdev,
>>  	vdev->minor = i + minor_offset;
>>  	vdev->num = nr;
>>  
>> +	if (WARN_ON(vdev->minor >= VIDEO_NUM_DEVICES)) {
> 
> Could this be combined with the should-not-happen case below? The error
> handling is the same (releasing the mutex) and the error code could be as
> well. I think the message can be just as well removed as we have a
> WARN_ON() here anyway.
> 
> I wonder what Hans thinks.

I actually prefer to keep it separate. If you combine it, then it is hard
to see which of the two possibilities is actually wrong (out of range or
minor in use). And this function sits at the core of V4L2, so it's OK to
be a bit more verbose.

I do agree that a pr_err after a WARN_ON is not needed.

Regards,

	Hans

> 
>> +		mutex_unlock(&videodev_lock);
>> +		pr_err("invalid minor. Check ranges.\n");
>> +		return -EINVAL;
>> +	}
>> +
>>  	/* Should not happen since we thought this minor was free */
>>  	if (WARN_ON(video_devices[vdev->minor])) {
>>  		mutex_unlock(&videodev_lock);
>>
> 

Re: [PATCH 1/6] media: v4l2-dev: Add range check for vdev->minor

[Sakari Ailus]

Hi Hans,

On Wed, Apr 29, 2026 at 09:53:41AM +0200, Hans Verkuil wrote:
> On 29/04/2026 09:43, Sakari Ailus wrote:
> > Hi Ricardo,
> > 
> > On Tue, Apr 28, 2026 at 12:41:07PM +0000, Ricardo Ribalda wrote:
> >> If the fixed minor ranges are not properly set we could end up in a
> >> situation where the calculated minor is invalid. Add a check for this in
> >> the code.
> >>
> >> This check also fixes the following smatch warning:
> >>
> >> drivers/media/v4l2-core/v4l2-dev.c:1036 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> >> drivers/media/v4l2-core/v4l2-dev.c:1043 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> >> drivers/media/v4l2-core/v4l2-dev.c:1101 __video_register_device() error: buffer overflow 'video_devices' 256 <= 288
> >>
> >> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> >> ---
> >>  drivers/media/v4l2-core/v4l2-dev.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> >> index 6ce623a1245a..a731ffdb91ee 100644
> >> --- a/drivers/media/v4l2-core/v4l2-dev.c
> >> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> >> @@ -1032,6 +1032,12 @@ int __video_register_device(struct video_device *vdev,
> >>  	vdev->minor = i + minor_offset;
> >>  	vdev->num = nr;
> >>  
> >> +	if (WARN_ON(vdev->minor >= VIDEO_NUM_DEVICES)) {
> > 
> > Could this be combined with the should-not-happen case below? The error
> > handling is the same (releasing the mutex) and the error code could be as
> > well. I think the message can be just as well removed as we have a
> > WARN_ON() here anyway.
> > 
> > I wonder what Hans thinks.
> 
> I actually prefer to keep it separate. If you combine it, then it is hard
> to see which of the two possibilities is actually wrong (out of range or
> minor in use). And this function sits at the core of V4L2, so it's OK to

Not if they're on different lines as you get the line number with
WARN_ON(). Keeping such code small has benefits.

That being said, I certainly have no strong opinion about this.

> be a bit more verbose.
> 
> I do agree that a pr_err after a WARN_ON is not needed.

-- 
Regards,

Sakari Ailus

Re: [PATCH 0/6] media: Fix new smatch warnings

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 469 bytes --]

On Tue, Apr 28, 2026 at 03:58:08PM +0200, Ricardo Ribalda wrote:
> 
> The only one that deserves a complain is this one:
> https://lore.kernel.org/linux-media/CANiDSCtm4Nh4Ub4rbEBvpjV8GXT9VQ5eFXZTHn=Wy=0RpR=3JA@mail.gmail.com/T/#m650723c33ec0318d8f32f1a6cc74c74a952ae11a

Thanks.  I've written a fix for this.  Let me test it tonight
and I'll push later.

I've attached the validation/ test so you can look at the new
ouput. ./smatch sm_mask1.c

regards,
dan carpenter

[-- Attachment #2: sm_mask1.c --]
[-- Type: text/x-csrc, Size: 1183 bytes --]

#include "check_debug.h"

void func(int a, int b, int c, int d, int e)
{
	if (a < 65)
		return;
	if (b < 0 || b > 7)
		return;
	if (c < 7 || c > 17)
		return;
	if (d < 0)
		return;

	e &= 0xf0;

	__smatch_implied(a);
	__smatch_implied(a & ~7);
	__smatch_implied(~7 & a);
	__smatch_implied(b & ~7);
	__smatch_implied(c & ~7);
	__smatch_implied(d & 0xff);
	__smatch_implied(d & 0xf0);
	__smatch_implied(d & e);
	__smatch_implied(d & (unsigned char)a);
	__smatch_implied(b & (unsigned char)a);
	__smatch_implied(c & (unsigned char)a);
}

/*
 * check-name: smatch: mask #1
 * check-command: ./smatch -I.. sm_mask1.c
 *
 * check-output-start
sm_mask1.c:16 func() implied: a = '65-s32max'
sm_mask1.c:17 func() implied: a & ~7 = '64-s32max'
sm_mask1.c:18 func() implied: ~7 & a = '64-s32max'
sm_mask1.c:19 func() implied: b & ~7 = '0'
sm_mask1.c:20 func() implied: c & ~7 = '0,8-16'
sm_mask1.c:21 func() implied: d & 255 = '0-255'
sm_mask1.c:22 func() implied: d & 240 = '0,16-240'
sm_mask1.c:23 func() implied: d & e = '0,16-240'
sm_mask1.c:24 func() implied: d & a = '0-255'
sm_mask1.c:25 func() implied: b & a = '0-7'
sm_mask1.c:26 func() implied: c & a = '0-17'
 * check-output-end
 */