coda: improve safety in coda_register_device()
Subsystems (2)
PATCH 1/1 coda: improve safety in coda_register_device()
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.
drivers/media/platform/coda/coda-common.c +3 -2
@@ -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 ; -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,
Am 08.01.2015 11:07, schrieb Dan Carpenter:
Quoted 15 lines
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 <[email protected]>drivers/media/platform/coda/coda-common.c +0
-0
drivers/media/platform/coda/coda-common.c static int coda_register_device(struct coda_dev *dev, int i) · line 1848
Quoted 6 lines
drivers/media/platform/coda/coda-common.c +1
-1@@ -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
Quoted 11 lines
drivers/media/platform/coda/coda-common.c +2
-2-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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 11 lines · walter harms
drivers/media/platform/coda/coda-common.c static int coda_register_device(struct coda_dev *dev, int i) · line 1848
Quoted 7 lines
drivers/media/platform/coda/coda-common.c +1
-1@@ -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:
Quoted 5 lines
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"
The "access" is just taking the address, not dereferencing so it's ok. This kind of code is fairly common and CodingStyle doesn't have an opinion here so I left it how the original author wrote it.
regards,
dan carpenter