On Thursday, August 13, 2015 7:29 AM, Ian Abbott wrote:
> 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.
>
> 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,
I'm not sure if coverity would handle the static analysis enough to detect
that s->n_chan had been bounds checked in drivers.c when if detects the
possible overflow in comedi_fops.c.
Regardless, I think a bounds check in __comedi_device_postconfig() is a good
idea. Maybe it should be a check against a define (COMEDI_NCHANS_MAX)?
That way something reasonable (256 or 1024?) can be used and if someone
_should_ make a board that has more it's easy to update.
Currently the largest s->n_chan appears to be 256 (in the addi_apci_3501 and
cb_pcidas drivers). I think these drivers only support that many channels by
using some external add-on boards.
Regards,
Hartley
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
