Since QEMU v4.0.0, the host's model is no longer automatically exposed
to the guest via /proc/device-tree/host-model. There is now a machine
option 'host-model' that allows an arbitrary string to be used as the
host model [1].

This patch adds support for exposing the real host model string from
/proc/cpuinfo (also found on /proc/device-tree/model) to the guest via
-machine pseries,host-model=<model> by setting in the domain XML file:

<features>
    <host_model_passthrough state='on'/>
<features/>

The functionality is disabled by default to avoid leaking host
information to the guest.

1 - https://git.qemu.org/?p=qemu.git;a=commitdiff;h=0a794529bd11

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 docs/formatdomain.html.in                     |  9 ++++++
 docs/schemas/domaincommon.rng                 |  5 ++++
 src/conf/domain_conf.c                        |  4 +++
 src/conf/domain_conf.h                        |  1 +
 src/qemu/qemu_command.c                       | 25 ++++++++++++++--
 src/qemu/qemu_domain.c                        |  1 +
 tests/qemuxml2argvdata/pseries-features.args  |  3 +-
 tests/qemuxml2argvdata/pseries-features.xml   |  1 +
 tests/qemuxml2argvtest.c                      | 30 ++++++++++++++++++-
 tests/qemuxml2xmloutdata/pseries-features.xml |  1 +
 10 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 647f96f651..b613f61f23 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2357,6 +2357,15 @@
           will not be ignored.
           <span class="since">Since 5.1.0</span> (bhyve only)
       </dd>
+      <dt><code>host-model-passthrough</code></dt>
+      <dd>Configure the passthrough of the host's model string to
+          pSeries guests. The host model will be present in the guest at
+          /proc/device-tree/host-model.
+          Possible values for the <code>state</code> attribute are
+          <code>on</code> and <code>off</code>. If the attribute is
+          not defined, the default value (<code>off</code>) is assumed.
+          <span class="since">Since 5.8.0</span> (QEMU/KVM only)
+      </dd>
     </dl>
 
     <h3><a id="elementsTime">Time keeping</a></h3>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 40eb4a2d75..0d505bb38a 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5117,6 +5117,11 @@
               <ref name="featurestate"/>
             </element>
           </optional>
+          <optional>
+            <element name="host-model-passthrough">
+              <ref name="featurestate"/>
+            </element>
+          </optional>
           <optional>
             <ref name="msrs"/>
           </optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a53cd6a725..9803f18389 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -171,6 +171,7 @@ VIR_ENUM_IMPL(virDomainFeature,
               "htm",
               "nested-hv",
               "msrs",
+              "host-model-passthrough",
 );
 
 VIR_ENUM_IMPL(virDomainCapabilitiesPolicy,
@@ -20396,6 +20397,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 
         case VIR_DOMAIN_FEATURE_HTM:
         case VIR_DOMAIN_FEATURE_NESTED_HV:
+        case VIR_DOMAIN_FEATURE_HOST_MODEL_PASSTHROUGH:
             if (!(tmp = virXMLPropString(nodes[i], "state"))) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                _("missing state attribute '%s' of feature '%s'"),
@@ -22621,6 +22623,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
         case VIR_DOMAIN_FEATURE_VMCOREINFO:
         case VIR_DOMAIN_FEATURE_HTM:
         case VIR_DOMAIN_FEATURE_NESTED_HV:
+        case VIR_DOMAIN_FEATURE_HOST_MODEL_PASSTHROUGH:
             if (src->features[i] != dst->features[i]) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                _("State of feature '%s' differs: "
@@ -28187,6 +28190,7 @@ virDomainDefFormatFeatures(virBufferPtr buf,
         case VIR_DOMAIN_FEATURE_VMPORT:
         case VIR_DOMAIN_FEATURE_HTM:
         case VIR_DOMAIN_FEATURE_NESTED_HV:
+        case VIR_DOMAIN_FEATURE_HOST_MODEL_PASSTHROUGH:
             switch ((virTristateSwitch) def->features[i]) {
             case VIR_TRISTATE_SWITCH_LAST:
             case VIR_TRISTATE_SWITCH_ABSENT:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 2884af49d8..ba32116e90 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1755,6 +1755,7 @@ typedef enum {
     VIR_DOMAIN_FEATURE_HTM,
     VIR_DOMAIN_FEATURE_NESTED_HV,
     VIR_DOMAIN_FEATURE_MSRS,
+    VIR_DOMAIN_FEATURE_HOST_MODEL_PASSTHROUGH,
 
     VIR_DOMAIN_FEATURE_LAST
 } virDomainFeature;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cbf25d5f07..bdb4461282 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7110,7 +7110,6 @@ qemuAppendLoadparmMachineParm(virBuffer *buf,
     }
 }
 
-
 static int
 qemuBuildNameCommandLine(virCommandPtr cmd,
                          virQEMUDriverConfigPtr cfg,
@@ -7140,14 +7139,16 @@ qemuBuildNameCommandLine(virCommandPtr cmd,
 
 static int
 qemuBuildMachineCommandLine(virCommandPtr cmd,
-                            virQEMUDriverConfigPtr cfg,
+                            virQEMUDriverPtr driver,
                             const virDomainDef *def,
                             virQEMUCapsPtr qemuCaps)
 {
     virTristateSwitch vmport = def->features[VIR_DOMAIN_FEATURE_VMPORT];
     virTristateSwitch smm = def->features[VIR_DOMAIN_FEATURE_SMM];
     virCPUDefPtr cpu = def->cpu;
+    virSysinfoDefPtr hostinfo = driver->hostsysinfo;
     VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+    VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
     size_t i;
 
     /* This should *never* be NULL, since we always provide
@@ -7409,6 +7410,24 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
         virBufferAsprintf(&buf, ",cap-nested-hv=%s", str);
     }
 
+    if (def->features[VIR_DOMAIN_FEATURE_HOST_MODEL_PASSTHROUGH] == VIR_TRISTATE_SWITCH_ON) {
+        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_HOST_MODEL)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("Host model passthrough is not supported by "
+                             "this QEMU binary"));
+            return -1;
+        }
+
+        if (!hostinfo || !hostinfo->system) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("Host model information is not available"));
+            return -1;
+        }
+
+        virBufferAddLit(&buf, ",host-model=");
+        virQEMUBuildBufferEscapeComma(&buf, hostinfo->system->serial);
+    }
+
     if (cpu && cpu->model &&
         cpu->mode == VIR_CPU_MODE_HOST_MODEL &&
         qemuDomainIsPSeries(def) &&
@@ -10309,7 +10328,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
     if (enableFips)
         virCommandAddArg(cmd, "-enable-fips");
 
-    if (qemuBuildMachineCommandLine(cmd, cfg, def, qemuCaps) < 0)
+    if (qemuBuildMachineCommandLine(cmd, driver, def, qemuCaps) < 0)
         return NULL;
 
     qemuBuildTSEGCommandLine(cmd, def);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b4175a846e..a310ab828e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4730,6 +4730,7 @@ qemuDomainDefValidateFeatures(const virDomainDef *def,
         case VIR_DOMAIN_FEATURE_HPT:
         case VIR_DOMAIN_FEATURE_HTM:
         case VIR_DOMAIN_FEATURE_NESTED_HV:
+        case VIR_DOMAIN_FEATURE_HOST_MODEL_PASSTHROUGH:
             if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT &&
                 !qemuDomainIsPSeries(def)) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
diff --git a/tests/qemuxml2argvdata/pseries-features.args b/tests/qemuxml2argvdata/pseries-features.args
index 9fde54b37a..931645cba6 100644
--- a/tests/qemuxml2argvdata/pseries-features.args
+++ b/tests/qemuxml2argvdata/pseries-features.args
@@ -11,7 +11,8 @@ QEMU_AUDIO_DRV=none \
 -name guest \
 -S \
 -machine pseries,accel=tcg,usb=off,dump-guest-core=off,resize-hpt=required,\
-cap-hpt-max-page-size=1048576k,cap-htm=on,cap-nested-hv=off \
+cap-hpt-max-page-size=1048576k,cap-htm=on,cap-nested-hv=off,\
+host-model=fake_model \
 -m 512 \
 -realtime mlock=off \
 -smp 1,sockets=1,cores=1,threads=1 \
diff --git a/tests/qemuxml2argvdata/pseries-features.xml b/tests/qemuxml2argvdata/pseries-features.xml
index 6f7d32b065..8dd222c562 100644
--- a/tests/qemuxml2argvdata/pseries-features.xml
+++ b/tests/qemuxml2argvdata/pseries-features.xml
@@ -12,6 +12,7 @@
     </hpt>
     <htm state='on'/>
     <nested-hv state='off'/>
+    <host-model-passthrough state='on'/>
   </features>
   <devices>
     <emulator>/usr/bin/qemu-system-ppc64</emulator>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index db79301f0e..1d00068d2b 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -95,6 +95,30 @@ static virSecretDriver fakeSecretDriver = {
     .secretUndefine = NULL,
 };
 
+static void
+testSetHostSysinfo(virSysinfoDefPtr sysinfo)
+{
+    if (!sysinfo) {
+        virSysinfoDefFree(driver.hostsysinfo);
+        return;
+    }
+
+    driver.hostsysinfo = sysinfo;
+}
+
+static virSysinfoDefPtr
+fakeVirSysinfoReadPPC(void)
+{
+    virSysinfoDefPtr sysinfo = NULL;
+
+    if (VIR_ALLOC(sysinfo) < 0 || VIR_ALLOC(sysinfo->system) < 0) {
+        virSysinfoDefFree(sysinfo);
+        return NULL;
+    }
+
+    ignore_value(VIR_STRDUP(sysinfo->system->serial, "fake_model"));
+    return sysinfo;
+}
 
 # define STORAGE_POOL_XML_PATH "storagepoolxml2xmlout/"
 static const unsigned char fakeUUID[VIR_UUID_BUFLEN] = "fakeuuid";
@@ -1955,12 +1979,16 @@ mymain(void)
             QEMU_CAPS_VIRTIO_SCSI,
             QEMU_CAPS_DEVICE_VFIO_PCI);
 
+    testSetHostSysinfo(fakeVirSysinfoReadPPC());
     DO_TEST("pseries-features",
             QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE,
             QEMU_CAPS_MACHINE_PSERIES_CAP_HTM,
             QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV,
-            QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
+            QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT,
+            QEMU_CAPS_MACHINE_PSERIES_HOST_MODEL);
+    testSetHostSysinfo(NULL);
+
     DO_TEST_FAILURE("pseries-features",
                     QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
     DO_TEST_PARSE_ERROR("pseries-features-invalid-machine", NONE);
diff --git a/tests/qemuxml2xmloutdata/pseries-features.xml b/tests/qemuxml2xmloutdata/pseries-features.xml
index 7e12bc9c03..0d0277280d 100644
--- a/tests/qemuxml2xmloutdata/pseries-features.xml
+++ b/tests/qemuxml2xmloutdata/pseries-features.xml
@@ -14,6 +14,7 @@
     </hpt>
     <htm state='on'/>
     <nested-hv state='off'/>
+    <host-model-passthrough state='on'/>
   </features>
   <clock offset='utc'/>
   <on_poweroff>destroy</on_poweroff>
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Oct 08, 2019 at 01:46:57PM -0300, Fabiano Rosas wrote:
> Since QEMU v4.0.0, the host's model is no longer automatically exposed
> to the guest via /proc/device-tree/host-model. There is now a machine
> option 'host-model' that allows an arbitrary string to be used as the
> host model [1].
> 
> This patch adds support for exposing the real host model string from
> /proc/cpuinfo (also found on /proc/device-tree/model) to the guest via
> -machine pseries,host-model=<model> by setting in the domain XML file:
> 
> <features>
>     <host_model_passthrough state='on'/>
> <features/>

I definitely don't want to see that. A mgmt app which used this
would be re-exposing the security risk that the QEMU change was
intended to fix.  That might be ok in some cases, but in the
general case we need the ability for the mgmt app to supply the
actual data to expose. This is how we deal with SMBIOS - we allow
<smbios mode="emulate|host|sysinfo"/>, where 'sysinfo' option lets
the mgmt app have full control over what's exposed.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Oct 08, 2019 at 01:46:57PM -0300, Fabiano Rosas wrote:
>> Since QEMU v4.0.0, the host's model is no longer automatically exposed
>> to the guest via /proc/device-tree/host-model. There is now a machine
>> option 'host-model' that allows an arbitrary string to be used as the
>> host model [1].
>> 
>> This patch adds support for exposing the real host model string from
>> /proc/cpuinfo (also found on /proc/device-tree/model) to the guest via
>> -machine pseries,host-model=<model> by setting in the domain XML file:
>> 
>> <features>
>>     <host_model_passthrough state='on'/>
>> <features/>
>
> I definitely don't want to see that. A mgmt app which used this
> would be re-exposing the security risk that the QEMU change was
> intended to fix.  That might be ok in some cases, but in the
> general case we need the ability for the mgmt app to supply the
> actual data to expose. This is how we deal with SMBIOS - we allow
> <smbios mode="emulate|host|sysinfo"/>, where 'sysinfo' option lets
> the mgmt app have full control over what's exposed.

Ok, fair enough

My ideia of only allowing either the vulnerable behavior or none at
all (the default) was to encourage users to find another solution,
instead of having them alter the management applications and continue
relying on (some sort of) host information exposure.

So do you suggest something like <host-model mode="none|sysinfo"/> ?
Because having "host" as well (like smbios has) would have the same
effect as this patch.


Thanks

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list