Re: [Qemu-devel] Re: [PATCH 5/5] Port apic to new VMState design

[Reimar Döffinger]

On Tue, Aug 18, 2009 at 05:38:57PM +0200, Juan Quintela wrote:
> Are your changes on upstream hw/eepro100.c?  I can't see anything there
> that can't be done in a table approach.

No, so far noone got around to taking my patches apart (and that is
actually one I have not yet properly submitted, it is mangled into that
patch: http://article.gmane.org/gmane.comp.emulators.qemu/49853

> >> It is already that way.  This design don't change anything.  And I am
> >> not sure how to fix it.  We don't have a "is this value safe for this
> >> field", around yet.  It is possible to add some support for it, but I
> >> would like to 1st have an use case.
> >
> > Well, I meant nowadays it is just possible to add a check in load_vm and
> > fix any values that are off. While it is quite a bit of work there is
> > nothing in the API stopping you from doing it, you even can return
> > -EINVAL and hopefully the core will print some somewhat useful message.
> 
> I guess we are going to have an optional callback to be called
> before/after loading the state.  You should be able to put your verify
> there.

Maybe I'm silly, but what would the callback for before loading state be
good for?

> > That is completely different from what I meant.
> > Changing the RAM compromises the VM and only the VM, an exploit in a
> > device emulation might allow to compromise the _host_. Is it now clearer
> > what I meant?
> 
> yes, I see where you are meaning now.  But I guess that one is needed to
> be solved, not only for migration.  Not sure what to do about this.

I think it is mostly leg-work of finding the assumptions the emulations
do. That really should be left to maintainers where available IMO.
I'm just suggesting that it's better to design the API in a way that
doesn't further discourage fixing this :-).
If the patch is close to being accepted maybe I can help out by writing
such verification code for vmware_vga, there e.g. depth, bypp, wred,
wgreen and wblue must fit together as well as
width/height/new_width/new_height and fb_size (I think)
and width/height/bypp must be limit to ensure no integer overflows...