On Fri, Mar 29, 2019 at 02:34:37PM +0100, Greg Kurz wrote: > On Fri, 29 Mar 2019 10:28:46 +1100 > David Gibson <da...@gibson.dropbear.id.au> wrote: > > > On Thu, Mar 28, 2019 at 01:56:48PM +0100, Greg Kurz wrote: > > > On Thu, 28 Mar 2019 15:40:25 +1100 > > > David Gibson <da...@gibson.dropbear.id.au> wrote: > > > > > > > 27461d69a0f "ppc: add host-serial and host-model machine attributes > > > > (CVE-2019-8934)" introduced 'host-serial' and 'host-model' machine > > > > properties for spapr to explicitly control the values advertised to the > > > > guest in device tree properties with the same names. > > > > > > > > The previous behaviour on KVM was to unconditionally populate the device > > > > tree with the real host serial number and model, which leaks possibly > > > > sensitive information about the host to the guest. > > > > > > > > To maintain compatibility for old machine types, we allowed those props > > > > to be set to "passthrough" to take the value from the host as before. > > > > Or > > > > they could be set to "none" to explicitly omit the device tree items. > > > > > > > > Special casing specific values on what's otherwise a user supplied > > > > string > > > > is very ugly. So, this patch simplifies things by implementing the > > > > backwards compatibility in a different way: we have a machine class flag > > > > set for the older machines, and we only load the host values into the > > > > device tree if A) they're not set by the user and B) we have that flag > > > > set. > > > > > > > > This does mean that the "passthrough" functionality is no longer > > > > available > > > > with the current machine type. That's ok though: if a user or > > > > management > > > > layer really wants the information passed through they can read it > > > > themselves (OpenStack Nova already does something similar for x86). > > > > > > > > It also means the user can't explicitly ask for the values to be omitted > > > > on the old machine types. I think that's an acceptable trade-off: if > > > > you > > > > care enough about not leaking the host information you can either move > > > > to > > > > the new machine type, or use a dummy value for the properties. > > > > > > > > This also removes an odd inconsistency between running on a POWER and > > > > non-POWER (or non-Linux) hosts: if the host information couldn't be read > > > > from where we expect (in the host's device tree as exposed by Linux), > > > > we'd > > > > fallback to omitting the guest device tree items. > > > > > > This is still the case, isn't it ? A pseries-3.1 machine on a POWER linux > > > host > > > has the DT items but the same machine on any other setup doesn't have > > > them... > > > or maybe^Wlikely I don't understand what you mean :) > > > > So, I was talking about the case of the new machine type there. Which > > admittedly probably wasn't terribly clear in context. > > > > Ok... I guess I got confused by the "this also removes", like if you were > saying "I've dropped the previous behavior for new machines and I've also > fixed an inconsistency", whereas you just give an example of how broken > the old behavior was. > > > > > While we're there, improve some poorly worded comments, and the help > > > > text > > > > for the properties. > > > > > > > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > > > > --- > > > > > > > > I've (tentatively) put this into my ppc-for-4.0 tree already, which I > > > > hope to push in the next few days. I realize it's very late to make > > > > such a cleanup in 4.0, however I'd like to clean up the interface > > > > before it goes into a released version which we have to support for > > > > ages. > > > > > > > > > > Sure. So, I've tested on POWER, non-POWER, with KVM, with TCG. It works as > > > expected. Just one remark: when running an old machine type under TCG on > > > a POWER host, the DT items are populated with the host data if QEMU was > > > built with KVM support, and missing if QEMU was built without KVM support. > > > This makes me wonder if kvm_enabled() should be added somewhere in the > > > picture... Anyway, this has always been here and could be addressed in > > > some other patch. > > > > I don't see that there's much point. Yes, the old behaviour is broken > > and inconsistent, and the new machine type behaviour fixes that. I > > don't see much profit in tweaking the exact areas of inconsistency in > > the old behaviour. > > > > Yes you're right. BTW, if it is settled for good that new machine > types won't look at the host DT, then maybe we should even get rid > of kvmppc_get_host_serial() and kvmppc_get_host_model() and opencode > them where they're being used ? So that nobody is tempted to use them > in new code.
Yeah, I've considered it and may well do that at some point.
--
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
signature.asc
Description: PGP signature
