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
regards,
dan carpenter
Attachment:
signature.asc
Description: Digital signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
