[PATCH 2/2] Staging: bcm: Fix an integer overflow in IOCTL_BCM_NVM_READ/WRITE

[Kevin McKinney]

On Fri, Dec 16, 2011 at 4:47 AM, Dan Carpenter <dan.carpenter at oracle.com> 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