From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:57086) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R4Oum-0003sA-LJ for qemu-devel@nongnu.org; Thu, 15 Sep 2011 23:16:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R4Oul-0002I8-ND for qemu-devel@nongnu.org; Thu, 15 Sep 2011 23:16:32 -0400 Received: from ozlabs.org ([203.10.76.45]:35099) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R4Oul-0002Hn-DN for qemu-devel@nongnu.org; Thu, 15 Sep 2011 23:16:31 -0400 Date: Fri, 16 Sep 2011 13:16:17 +1000 From: David Gibson Message-ID: <20110916031617.GA9025@yookeroo.fritz.box> References: <1315983769-8287-1-git-send-email-david@gibson.dropbear.id.au> <4E70647D.10408@redhat.com> <20110915023431.GO9025@yookeroo.fritz.box> <20110915025803.GP9025@yookeroo.fritz.box> <4E71A92A.3080701@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4E71A92A.3080701@redhat.com> Subject: Re: [Qemu-devel] [PATCH] Fix subtle integer overflow bug in memory API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: agraf@suse.de, qemu-devel@nongnu.org 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