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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Mon, May 04, 2026 at 06:54:04AM +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 to make it more robust.

If it was just for that, we could define the ranges in a way that could
not lead to future programmatic errors.

> This check also fixes the following false positive 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 | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 6ce623a1245a..5516b2bbb08f 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -1032,6 +1032,11 @@ 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);

I may get tempted to convert code to using scoped guards at some point.

> +		return -EINVAL;
> +	}
> +

I'm annoyed by the proliferation of workarounds for smatch false
positives that generate useless code :-/ This is in particular is not a
big deal though, so

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

but I don't want to continue in this direction with every new kernel
release. We need a way to tell smatch that this is safe with incurring
an runtime cost.

>  	/* Should not happen since we thought this minor was free */
>  	if (WARN_ON(video_devices[vdev->minor])) {
>  		mutex_unlock(&videodev_lock);
> 

-- 
Regards,

Laurent Pinchart