On Fri, Dec 16, 2011 at 4:47 AM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
