On 12/08/15 19:39, Dan Carpenter wrote:
On Wed, Aug 12, 2015 at 10:30:47AM -0700, H Hartley Sweeten wrote:@@ -1061,6 +1061,14 @@ static int do_chaninfo_ioctl(struct comedi_device *dev, if (it.maxdata_list) { if (s->maxdata || !s->maxdata_list) return -EINVAL; + /* + * s->n_chan is usually <= 32 but _some_ comedi drivers + * do have more. Do a simple sanity check to make sure + * copy_to_user() does not overflow. In reality this + * should never fail... + */ + if (s->n_chan > (0xffffffff / sizeof(unsigned int))) + return -EINVAL; if (copy_to_user(it.maxdata_list, s->maxdata_list, s->n_chan * sizeof(unsigned int)))This change doesn't make sense because an integer overflow here would be basically harmless. I don't like silencing false positives in this way. It's better to ignore the warning. The bounds err in ni6501_port_command() was a false positive too, but the fix for that made to code more readable so it was a good thing. regards, dan carpenter
Would a sanity check in __comedi_device_postconfig() ("drivers/staging/comedi/drivers.c") to prevent s->n_chan being set to a silly value in the first place suppress the coverity issue?
-- -=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=- -=( Web: http://www.mev.co.uk/ )=- _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
