On 09/15/2011 05:58 AM, David Gibson wrote:
> Well.. I'm pretty sure that particular case can no longer be > triggered. But there may be other integer overflow bugs in this code, > though I didn't spot them at a glance.Actually, there definitely are. addrrange_end() should probably be redefined as the inclusive end, rather than the exclusive end, and computed as: (start + size - 1)< start ? INT64_MAX : start + size - 1 Hrm, except that that doesn't handle zero size ranges. And gets a bit weird for INT64_MAX sized regions. Hrm, yeah, these really should be changed to use unsigneds.
Unsigneds don't work because we sometimes subtract values and get into the negative area, due to aliases.
We're using values that are close to the end of the 63 or 64 bit range and playing with them. Overflow is always close and we'll always be able to find a way to defeat the code (even if it doesn't occur in practice). I'm tempted to declare that the memory API supports only 60 bits of address space (and s/INT64_MAX/MEMORY_REGION_SIZE_MAX/), that gives us a few levels of wild offsetting before we get confused.
-- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
