[libvirt] [PATCH v2 6/6] tools: make virt-host-validate check CPU vulnerabilities

[Martin Kletzander]

On Fri, Sep 27, 2019 at 01:52:25PM +0100, Daniel P. Berrangé wrote:
>Add a check reporting if any CPU vulnerabilities have not been mitigated
>by the kernel. It further reports whether it is safe to use Intel SMT
>for KVM guests or not, as several of the vulnerabilities are dangerous
>when combined with SMT and KVM, even if mitigations are in effect.
>
>eg on a host with mitigations, but unsafe SMT still enabled:
>
>  Checking CPU hardware vulnerability mitigation...PASS
>  Checking CPU hardware vulnerability SMT safety...FAIL
>
>Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
>---
> libvirt.spec.in                               |   1 +
> tools/Makefile.am                             |   1 +
> .../rules/linux-cpu-hardware-flaws.yaml       | 165 ++++++++++++++++++
> 3 files changed, 167 insertions(+)
> create mode 100644 tools/host-validate/rules/linux-cpu-hardware-flaws.yaml
>
>diff --git a/libvirt.spec.in b/libvirt.spec.in
>index f336296a08..8aa226798a 100644
>--- a/libvirt.spec.in
>+++ b/libvirt.spec.in
>@@ -1901,6 +1901,7 @@ exit 0
> %{_datadir}/libvirt/host-validate/linux-acpi.yaml
> %{_datadir}/libvirt/host-validate/linux-cgroups.yaml
> %{_datadir}/libvirt/host-validate/linux-cpu.yaml
>+%{_datadir}/libvirt/host-validate/linux-cpu-hardware-flaws.yaml
> %{_datadir}/libvirt/host-validate/linux-devices.yaml
> %{_datadir}/libvirt/host-validate/linux-iommu.yaml
> %{_datadir}/libvirt/host-validate/linux-namespaces.yaml
>diff --git a/tools/Makefile.am b/tools/Makefile.am
>index 728de475a2..907b0195c2 100644
>--- a/tools/Makefile.am
>+++ b/tools/Makefile.am
>@@ -173,6 +173,7 @@ virt_host_validate_rules_DATA = \
> 	$(srcdir)/host-validate/rules/linux-acpi.yaml \
> 	$(srcdir)/host-validate/rules/linux-cgroups.yaml \
> 	$(srcdir)/host-validate/rules/linux-cpu.yaml \
>+	$(srcdir)/host-validate/rules/linux-cpu-hardware-flaws.yaml \
> 	$(srcdir)/host-validate/rules/linux-devices.yaml \
> 	$(srcdir)/host-validate/rules/linux-iommu.yaml \
> 	$(srcdir)/host-validate/rules/linux-namespaces.yaml \
>diff --git a/tools/host-validate/rules/linux-cpu-hardware-flaws.yaml b/tools/host-validate/rules/linux-cpu-hardware-flaws.yaml
>new file mode 100644
>index 0000000000..6a243df96d
>--- /dev/null
>+++ b/tools/host-validate/rules/linux-cpu-hardware-flaws.yaml
>@@ -0,0 +1,165 @@
>+#
>+# Define facts related to CPU hardware vulnerabilities
>+#
>+
>+facts:
>+- name: cpu.vulnerability.meltdown
>+  filter:
>+    fact:
>+      name: os.kernel
>+      value: Linux
>+  value:
>+    file:
>+      path: /sys/devices/system/cpu/vulnerabilities/meltdown
>+      ignoreMissing: true
>+      parse:
>+        scalar:
>+          regex: (\w+)
>+          match: 1
>+- name: cpu.vulnerability.spectre_v1
>+  filter:
>+    fact:
>+      name: os.kernel
>+      value: Linux
>+  value:
>+    file:
>+      path: /sys/devices/system/cpu/vulnerabilities/spectre_v1
>+      ignoreMissing: true
>+      parse:
>+        scalar:
>+          regex: (\w+)
>+          match: 1
>+- name: cpu.vulnerability.spectre_v2
>+  filter:
>+    fact:
>+      name: os.kernel
>+      value: Linux
>+  value:
>+    file:
>+      path: /sys/devices/system/cpu/vulnerabilities/spectre_v2
>+      ignoreMissing: true
>+      parse:
>+        scalar:
>+          regex: (\w+)
>+          match: 1
>+- name: cpu.vulnerability.spec_store_bypass
>+  filter:
>+    fact:
>+      name: os.kernel
>+      value: Linux
>+  value:
>+    file:
>+      path: /sys/devices/system/cpu/vulnerabilities/spec_store_bypass
>+      ignoreMissing: true
>+      parse:
>+        scalar:
>+          regex: (\w+)
>+          match: 1
>+- name: cpu.vulnerability.mds
>+  filter:
>+    fact:
>+      name: os.kernel
>+      value: Linux
>+  value:
>+    file:
>+      path: /sys/devices/system/cpu/vulnerabilities/mds
>+      ignoreMissing: true
>+      parse:
>+        scalar:
>+          regex: (\w+)
>+          match: 1
>+- name: cpu.vulnerability.mds_smt
>+  filter:
>+    fact:
>+      name: os.kernel
>+      value: Linux
>+  value:
>+    file:
>+      path: /sys/devices/system/cpu/vulnerabilities/mds
>+      ignoreMissing: true
>+      parse:
>+        scalar:
>+          regex: SMT (\w+)
>+          match: 1
>+- name: cpu.vulnerability.l1tf
>+  filter:
>+    fact:
>+      name: os.kernel
>+      value: Linux
>+  value:
>+    file:
>+      path: /sys/devices/system/cpu/vulnerabilities/l1tf
>+      ignoreMissing: true
>+      parse:
>+        scalar:
>+          regex: (\w+)
>+          match: 1
>+- name: cpu.vulnerability.l1tf_smt
>+  filter:
>+    fact:
>+      name: os.kernel
>+      value: Linux
>+  value:
>+    file:
>+      path: /sys/devices/system/cpu/vulnerabilities/l1tf
>+      ignoreMissing: true
>+      parse:
>+        scalar:
>+          regex: SMT (\w+)
>+          match: 1

Given the fact that most of these could just be virFileReadValueUint() it does
not even make it easier to read or write the code.

Every time someone will want to add a new check or a fact they will need to find
a similar one, copy-paste it, change it and hope for the best.  This introduces
yet another "language" on top of the two you are adding already.  I really do
not see any benefit in this.

If I was to pick a new feature we could benefit from, I would much rather prefer
having an opt-in for report-home of HW features and usage for some very rough
anonymous statistics.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190930/db0d76b3/attachment-0001.sig>