Re: [PATCH] media: v4l2-ioctl: prevent underflow in v4l_enumoutput()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 17/05/18 11:05, Dan Carpenter wrote:
> My Smatch allmodconfig build only detects one function implementing
> vpbe_device_ops->enum_outputs and that's vpbe_enum_outputs().  The
> problem really happens in that function when we do:
> 
> 	int temp_index = output->index;
> 
> 	if (temp_index >= cfg->num_outputs)
> 		return -EINVAL;
> 
> Unfortunately, both temp_index and cfg->num_outputs are type int so we
> have a potential read before the start of the array if "temp_index" is
> negative.

Why not fix it in this driver? Make num_outputs unsigned, as it should be.

I really don't like having a random index check in the core. If we ever
want to do such things in the core, then it needs to be implemented
consistently for all ioctls that do something similar.

Regards,

	Hans

> 
> I could have fixed the bug in that function but it's more secure and
> future proof to block that bug earlier in a central place.  There is no
> one who need p->index to be more than INT_MAX.
> 
> Fixes: 66715cdc3224 ("[media] davinci vpbe: VPBE display driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index a40dbec271f1..115757ab8bc0 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1099,6 +1099,9 @@ static int v4l_enumoutput(const struct v4l2_ioctl_ops *ops,
>  	if (is_valid_ioctl(vfd, VIDIOC_S_STD))
>  		p->capabilities |= V4L2_OUT_CAP_STD;
>  
> +	if (p->index > INT_MAX)
> +		return -EINVAL;
> +
>  	return ops->vidioc_enum_output(file, fh, p);
>  }
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux