Re: [Qemu-devel] qmp: dump-guest-memory: -p option has issues, fix it or drop it?

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed, 19 Sep 2012 11:26:51 +0900 (JST)
HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com> wrote:

> From: Wen Congyang <wency@cn.fujitsu.com>
> Subject: Re: [Qemu-devel] qmp: dump-guest-memory: -p option has issues, fix it or drop it?
> Date: Wed, 19 Sep 2012 10:07:04 +0800
> 
> > At 09/19/2012 08:18 AM, Luiz Capitulino Wrote:
> >> On Tue, 18 Sep 2012 16:13:30 -0500
> >> Anthony Liguori <anthony@codemonkey.ws> wrote:
> >> 
> >>> Markus Armbruster <armbru@redhat.com> writes:
> >>>
> >>>> Jan Kiszka <jan.kiszka@siemens.com> writes:
> >>>>
> >>>>>>>>>  * The issues discussed in this email plus the fact that the guest
> >>>>>>>>>    memory may be corrupted, and the guest may be in real-mode even
> >>>>>>>>>    when paging is enabled
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> Yes, there are some limitations with this option. Jan said that he
> >>>>>>>> always use gdb to deal with vmcore, so he needs such information.
> >>>>>>>
> >>>>>>> The point is to overcome the focus on Linux-only dump processing tools.
> >>>>>>
> >>>>>> While I don't care for supporting alternate dump processing tools
> >>>>>> myself, I certainly don't mind supporting them, as long as the code
> >>>>>> satisfies basic safety and reliability requirements.
> >>>>>>
> >>>>>> This code doesn't, as far as I can tell.
> >>>>>
> >>>>> It works, thought not under all circumstances.
> >>>>
> >>>> I don't doubt it works often enough to be useful to somebody.  But basic
> >>>> safety and reliability requirements are a bit more than that.  They
> >>>> include "don't explode in ways a reasonable user can't be expected to
> >>>> foresee".  I don't think a reasonable user can be expected to see that
> >>>> -p is safe only for trusted guests.
> >>>
> >>> We shipped the API, we're not removing it.  Our compatibility isn't
> >>> "whatever libvirt is currently using".
> >>>
> >>> It's perfectly reasonable to ask to document the behavior of the
> >>> method.  It's also a trivial patch to qapi-schema.json.
> >> 
> >> I feel that documenting it is not enough. It would be fine to do that
> >> if the worst case was a bad dump file, but the worst case as the
> >> code stands right now will affect the host.
> >> 
> >>> It's unreasonable to ask for an interface to be removed just because it
> >>> could be misused when it has a legimitate use-case.
> >> 
> >> The point is not that it can be misused. The issue we're concerned about
> >> is that a malicious guest could cause qemu to allocate dozens of
> >> gigabytes of RAM.
> > 
> > Hmm, I guess the malicious guest's page table provides wrong information.
> > We allocate memory to store memory mapping. Each memory mapping needs
> > less than 40 bytes memory. The num of memory mapping is less than
> > (2^48) / (2^12) = 2^36. And 2^36 * 40 = 64G * 40, too many memory....
> > 
> > What about this:
> > 1. if the num of memory mapping > 100000, we only store 100000 memory
> >    mappings.
> > 
> > 2. The memory mapping which has smaller virtual address will be dropped?
> > 
> > In this case, the memory we need is less than 10MB. So we will not allocate
> > too many memory.

The problem of artificially limiting it is that we can reach the limit
in "legal" usage (ie. a very large machine).

> How about dropping making a whole list of memory maps at the same
> time, and how about rewriting the code so that it always has at most
> one memory mapping by merging virtually consequtive chunks? If
> possible, only 40 bytes is needed.

It already merges contiguous addresses and addresses that fall in
the same range. It can also skip addresses due to a few reasons (I/O
page, not present page, etc), which makes the problem very unlikely
in practice.

Our concern is with guests intentionally trying to make qemu allocate
more memory.