qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] memory: info mtree check mr range overflow


From: Peter Xu
Subject: Re: [Qemu-devel] [PATCH] memory: info mtree check mr range overflow
Date: Wed, 15 Mar 2017 12:04:27 +0800
User-agent: Mutt/1.5.24 (2015-08-30)

On Wed, Mar 15, 2017 at 05:30:56AM +0200, Michael S. Tsirkin wrote:
> On Wed, Mar 15, 2017 at 11:15:50AM +0800, Peter Xu wrote:
> > On Wed, Mar 15, 2017 at 03:24:04AM +0200, Michael S. Tsirkin wrote:
> > > On Tue, Mar 14, 2017 at 08:56:27PM +0800, Peter Xu wrote:
> > > > The address of memory regions might overflow when something wrong
> > > > happened, like reported in:
> > > > 
> > > > https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg02043.html
> > > > 
> > > > For easier debugging, let's try to detect it.
> > > > 
> > > > Reported-by: Mark Cave-Ayland <address@hidden>
> > > > Signed-off-by: Peter Xu <address@hidden>
> > > 
> > > 
> > > After a chat with Paolo, I think the following is a more general fix
> > > 
> > > - fix info mtree to do 128 bit math and display more than
> > >   16 digits if necessary
> > 
> > Could you help elaborate in what case will we really need that 128 bit
> > address?
> 
> This is how memory API works. It uses 128 bit addresses (in reality
> it typically only needs 64 bit addresses but 128 means it can do
> math without worrying about it too much).

Yes. To be more specific, could I ask why do we need 128 bits here
when doing "info mtree"?

> Thus a region at offset 0xf << 60 in parent with address 0x1 << 60
> and size 0x1 << 20 is not "overflowing" it is simply at and address
> 0x1 << 64 which is outside the range of parent so not visible
> in the flat view.
> But same can be said for region at offset 0x1 << 60 in same parent
> and your patch does nothing to help detect it.

Not sure I fully understand the case mentioned above... I believe for
above example, current patch (either with, or without) will print:

    0x2000000000000000

And even with the patch "memory: use 128 bit in info mtree", it should
print the same. IIUC this is what we want, no? Did I miss anything?

> 
> > Btw, thanks for pointing out in the other thread that your patch
> > wasn't printing 128 bits but 64 bits, actually I didn't notice that
> > before... but even with that, I would still slightly prefer this one
> > though considering readability and simplicity.
> 
> Right but it's just trying to address the specific problem with
> the given device. Which is unlikely to trigger again exactly
> in the same way. The general issue is that the child region
> address is outside the range of the parent.

Hmm... frankly speaking I don't know whether current memory API would
allow this happen. I just see no danger if that happens, as long as we
will make sure those outranged regions will never be used during
rendering.

Anyway, IMHO that's another topic. This patch should be solely solving
the issue that was reported. Thanks,

-- peterx



reply via email to

[Prev in Thread] Current Thread [Next in Thread]