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

[David Gibson]

On Wed, Sep 14, 2011 at 11:23:25AM +0300, Avi Kivity wrote:
> On 09/14/2011 10:02 AM, David Gibson wrote:
> >It is quite common to have a MemoryRegion with size of INT64_MAX.
> >When processing alias regions in render_memory_region() it's quite
> >easy to find a case where it will construct a temporary AddrRange with
> >a non-zero start, and size still of INT64_MAX.  When means attempting
> >to compute the end of such a range as start + size will result in
> >signed integer overflow.
> >
> >This integer overflow means that addrrange_intersects() can
> >incorrectly report regions as not intersecting when they do.  For
> >example consider the case of address ranges {0x10000000000,
> >0x7fffffffffffffff} and {0x10010000000, 0x10000000} where the second
> >is in fact included completely in the first.
> 
> Good catch, thanks for digging this out.
> 
> >This patch rearranges addrrange_intersects() to avoid the integer
> >overflow, correcting this behaviour.
> 
> I expect that the bad behaviour can still be triggered, for example
> by pointing aliases towards the end of very large regions.  Not that
> I expect this to occur in practice.

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.

> I think we should move towards using __int128 internally.  Is there
> any relevant host which does not support __int128?

Hrm, using a barely supported integer type I'd never previously heard
of seems sub-optimal.  With appropriate care it ought to be possible
to do all the necessary calculations correctly in 64 bits, even if we
need a few explicit "if (a + b) < a" tests in some places.


> Meanwhile, applied to memory/core, and will request a pull shortly.
> 

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