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

[David Gibson <david@gibson.dropbear.id.au>]

On Thu, Sep 15, 2011 at 10:28:42AM +0300, Avi Kivity wrote:
> 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.

I realise that, but I'm fairly confident the order of calculations can
be adjusted to cope with those cases, just as we can cope with the
overflow cases with care.

Remember that because unsigned overflow is defined, and we can use
that in some cases.


> 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.

There is something to be said for that.  Although many systems have
theoretical 64-bit addressing, on powerpc at least, the biggest
actually implemented physical address bus I'm aware of is under 50
bits.

Of course, there could be some highly sparse intermediate bus
addressing schemes which screw that up.


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson