On Monday, February 23, 2015 3:56 AM, Ian Abbott wrote: > On 20/02/15 19:52, H Hartley Sweeten wrote: >> usb_blk_msg() will return the passed 'len' (64) as the 'actual_len' (cnt) of > > That's usb_bulk_msg(). > >> the transfer. The addition of the '\0' to the end of the returned string will >> overrun the 'rx' array. Increase the array size by 1 to fix the out-of-bounds >> write. >> >> Reported-by: coverity (CID 711413) >> Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx> >> Cc: Ian Abbott <abbotti@xxxxxxxxx> >> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> >> --- >> drivers/staging/comedi/drivers/vmk80xx.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/staging/comedi/drivers/vmk80xx.c b/drivers/staging/comedi/drivers/vmk80xx.c >> index e371183..e0656d1 100644 >> --- a/drivers/staging/comedi/drivers/vmk80xx.c >> +++ b/drivers/staging/comedi/drivers/vmk80xx.c >> @@ -195,7 +195,7 @@ static void vmk80xx_read_eeprom(struct comedi_device *dev, int flag) >> unsigned int tx_pipe; >> unsigned int rx_pipe; >> unsigned char tx[1]; >> - unsigned char rx[64]; >> + unsigned char rx[65]; >> int cnt; >> >> tx_pipe = usb_sndbulkpipe(usb, 0x01); >> > > cnt could be a junk value if the call to usb_bulk_msg() fails. > Initializing cnt to 0 should fix that. > > You could just check cnt is in range before setting rx[cnt] to '\0'. > Better yet, since the subsequent strncpy() copies up to 24 chars > starting at either rx + 1 or rx + 25, the tail end of rx[] could be > zero-filled after returning from usb_bulk_msg(). Something like: > > if (cnt < 64) > memset(rx + cnt, 0, 64 - cnt); > > (Ideally, this driver should check the returns from usb_bulk_msg() for > errors, but that would be a separate set of patches. In this case, the > information extracted is only used in a kernel log message.) Ian, Actually, since the information is just used in kernel log messages can we just remove it? It looks like the vmk80xx_read_eeprom() and vmk80xx_check_data_link() functions could go away along with the struct firmware_version in the private data. Regards, Hartley _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
