On Fri, Dec 20, 2013 at 04:51:45PM +0800, Wenliang Fan wrote: > Thanks for your advice. > But the variable 'psFlash2xReadWrite->offset' in ' > *drivers/staging/bcm/nvm.c*:validateFlash2xReadWrite()' is also comes from > user space, which would cause an integer overflow in the following line: > > if ((uiSectStartOffset + psFlash2xReadWrite->offset + uiNumOfBytes) <= > uiSectEndOffset) > > in '*drivers/staging/bcm/**nvm.c*: validateFlash2xReadWrite()', > and another integer overflow in the following line: > > > ReadOffset = ReadOffset + ReadBytes; (or WriteOffset = WriteOffset + > WriteBytes;) > > in '*drivers/staging/bcm/**Bcmchar.c*: bcm_char_ioctl()'. > Alright, fine. But the new check is messy. Do it like this: /* these are user controlled and can lead to integer overflows */ if (psFlash2xReadWrite->offset > uiSectEndOffset) return false; if (uiNumOfBytes > uiSectEndOffset) return false; if (uiSectStartOffset + psFlash2xReadWrite->offset + uiNumOfBytes > uiSectEndOffset) return false; return true; That way each step is simpler to understand. People are too fond of compound conditions... *grumble*. regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
