Re: [patch] [media] coda: improve safety in coda_register_device()

[walter harms <wharms@bfs.de>]

Am 08.01.2015 11:07, schrieb Dan Carpenter:
> The "i" variable is used as an offset into both the dev->vfd[] and the
> dev->devtype->vdevs[] arrays.  The second array is smaller so we should
> use that as a limit instead of ARRAY_SIZE(dev->vfd).  Also the original
> check was off by one.
> 
> We should use a format string as well in case the ->name has any funny
> characters and also to stop static checkers from complaining.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
> index 39330a7..5dd6cae 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -1844,10 +1844,11 @@ static int coda_register_device(struct coda_dev *dev, int i)
>  {
>  	struct video_device *vfd = &dev->vfd[i];
>  
> -	if (i > ARRAY_SIZE(dev->vfd))
> +	if (i >= dev->devtype->num_vdevs)
>  		return -EINVAL;

hi,
 just a minor question. if i can not be trusted, i feel you should move the
 array access:
   struct video_device *vfd = &dev->vfd[i];
 after the check
   i >= dev->devtype->num_vdevs
at least that would improve the readability by not trigger my internal alarm
"check after access"

re,
 wh


> -	snprintf(vfd->name, sizeof(vfd->name), dev->devtype->vdevs[i]->name);
> +	snprintf(vfd->name, sizeof(vfd->name), "%s",
> +		 dev->devtype->vdevs[i]->name);
>  	vfd->fops	= &coda_fops;
>  	vfd->ioctl_ops	= &coda_ioctl_ops;
>  	vfd->release	= video_device_release_empty,
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>