Re: [Qemu-devel] [PATCH] Fix subtle integer overflow bug in memory API

[Avi Kivity <avi@redhat.com>]

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.