On Fri, Dec 16, 2011 at 4:47 AM, Dan Carpenter <[email protected]> wrote: > On Thu, Dec 15, 2011 at 11:14:54PM -0500, Kevin McKinney wrote: >> Variables stNVMReadWrite.uioffset and stNVMReadWrite.uiNumBytes >> are chosen from userspace and can be very high. The sum of >> these two digits would result in a small number. Therefore, >> this patch reorganizes the equation to remove the integer >> overflow. >> > > No no.. The original code is ok here. There *is* a potential > integer overflow some lines earlier though... > > 1291 if (copy_from_user(&stNVMReadWrite, > 1292 (IOCTL_BCM_NVM_READ == cmd) ? > IoBuffer.OutputBuffer : IoBuffer.InputBuffer, > 1293 sizeof(NVM_READWRITE))) > 1294 return -EFAULT; > 1295 > 1296 /* > 1297 * Deny the access if the offset crosses the cal area > limit. > 1298 */ > 1299 > 1300 if ((stNVMReadWrite.uiOffset + > stNVMReadWrite.uiNumBytes) > Adapter->uiNVMDSDSize) { > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > If you chose a high value for stNVMReadWrite.uiOffset it could > overflow and the test would say it was valid even though it wasn't. > > 1301 /* BCM_DEBUG_PRINT(Adapter,DBG_TYPE_PRINTK, 0, > 0,"Can't allow access beyond NVM Size: 0x%x 0x%x\n", stNVMReadWrite.uiOffset, > stNVMReadWrite.uiNumBytes); */ > 1302 return STATUS_FAILURE; > 1303 } > 1304 > > Perhaps you could fix that one instead. :P > My bad; I thought the error I fixed was an integer overflow. I will study this; and submit a patch to fix this error. Thanks for reviewing Dan!
Thanks, Kevin _______________________________________________ devel mailing list [email protected] http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
