| Message ID | 20211019141101.327397-1-liyu.yukiteru@bytedance.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86/mmu: Warn on nx_huge_pages for possible problems | expand |
On Tue, Oct 19, 2021, Li Yu wrote: > Add warning when `nx_huge_pages` is enabled by `auto` for hint that > huge pages may be splited by kernel. > > Add warning when CVE-2018-12207 may arise but `nx_huge_pages` is > disabled for hint that malicious guest may cause a CPU lookup. For the shortlog and changelog, "warning" is misleading. A "warn" usually means a WARN with a backtrace. This is really just an information message that happens to be displayed at level=warn, and that's an implementation detail. > Signed-off-by: Li Yu <liyu.yukiteru@bytedance.com> > --- > arch/x86/kvm/mmu/mmu.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 1a64ba5b9437..32026592e566 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -6056,19 +6056,26 @@ static void __set_nx_huge_pages(bool val) > nx_huge_pages = itlb_multihit_kvm_mitigation = val; > } > > +#define ITLB_MULTIHIT_NX_ON "iTLB multi-hit CPU bug present and cpu mitigations enabled, huge pages may be splited by kernel for security. See CVE-2018-12207 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/multihit.html for details.\n" ^ ^^^^^^^ |- guest split > +#define ITLB_MULTIHIT_NX_OFF "iTLB multi-hit CPU bug present and cpu mitigations enabled, malicious guest may cause a CPU lookup. See CVE-2018-12207 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/multihit.html for details.\n" ^^^^^^^ disabled This is almost entirely redundant with the information that is displayed in /sys/devices/system/cpu/vulnerabilities/itlb_multihit and /sys/module/kvm/parameters/nx_huge_pages. It also makes the below helper more than a bit messy. I kind of agree with Paolo's feedback that KVM should log a message if the bug is present but the mitigation is off. But if the admin explicitly turns off the mitigation, logging the message every time KVM is loaded is obnoxious. IMO, if we want to display a message then we should do something similar to l1tf_mitigation and give the admin the option to suppress any logging. Personally, I don't see much value in a message of any kind. Anyone that is running untrusted guests should darn well do a lot of performance testing and risk analysis before touching the knob. And any use case that cares enough about performance to explicitly turn off the mitigation should already know exactly how KVM' params affect performance. The L1D flushing messages are justified because a data leak is theoretically possible when SMT is enabled even when the kernel's default mitigation is enabled. That's not the case here. > static int set_nx_huge_pages(const char *val, const struct kernel_param *kp) > { > bool old_val = nx_huge_pages; > bool new_val; > > /* In "auto" mode deploy workaround only if CPU has the bug. */ > - if (sysfs_streq(val, "off")) > + if (sysfs_streq(val, "off")) { > new_val = 0; > - else if (sysfs_streq(val, "force")) > + if (get_nx_auto_mode() && new_val != old_val) > + pr_warn(ITLB_MULTIHIT_NX_OFF); > + } else if (sysfs_streq(val, "force")) > new_val = 1; > - else if (sysfs_streq(val, "auto")) > + else if (sysfs_streq(val, "auto")) { > new_val = get_nx_auto_mode(); > - else if (strtobool(val, &new_val) < 0) > + if (new_val && new_val != old_val) > + pr_warn(ITLB_MULTIHIT_NX_ON); > + } else if (strtobool(val, &new_val) < 0) All branches need braces if any branch has braces. > return -EINVAL; > > __set_nx_huge_pages(new_val); > @@ -6095,8 +6102,11 @@ int kvm_mmu_module_init(void) > { > int ret = -ENOMEM; > > - if (nx_huge_pages == -1) > + if (nx_huge_pages == -1) { > __set_nx_huge_pages(get_nx_auto_mode()); > + if (is_nx_huge_page_enabled()) > + pr_warn_once(ITLB_MULTIHIT_NX_ON); > + } > > /* > * MMU roles use union aliasing which is, generally speaking, an > -- > 2.11.0 >
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 1a64ba5b9437..32026592e566 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -6056,19 +6056,26 @@ static void __set_nx_huge_pages(bool val) nx_huge_pages = itlb_multihit_kvm_mitigation = val; } +#define ITLB_MULTIHIT_NX_ON "iTLB multi-hit CPU bug present and cpu mitigations enabled, huge pages may be splited by kernel for security. See CVE-2018-12207 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/multihit.html for details.\n" +#define ITLB_MULTIHIT_NX_OFF "iTLB multi-hit CPU bug present and cpu mitigations enabled, malicious guest may cause a CPU lookup. See CVE-2018-12207 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/multihit.html for details.\n" + static int set_nx_huge_pages(const char *val, const struct kernel_param *kp) { bool old_val = nx_huge_pages; bool new_val; /* In "auto" mode deploy workaround only if CPU has the bug. */ - if (sysfs_streq(val, "off")) + if (sysfs_streq(val, "off")) { new_val = 0; - else if (sysfs_streq(val, "force")) + if (get_nx_auto_mode() && new_val != old_val) + pr_warn(ITLB_MULTIHIT_NX_OFF); + } else if (sysfs_streq(val, "force")) new_val = 1; - else if (sysfs_streq(val, "auto")) + else if (sysfs_streq(val, "auto")) { new_val = get_nx_auto_mode(); - else if (strtobool(val, &new_val) < 0) + if (new_val && new_val != old_val) + pr_warn(ITLB_MULTIHIT_NX_ON); + } else if (strtobool(val, &new_val) < 0) return -EINVAL; __set_nx_huge_pages(new_val); @@ -6095,8 +6102,11 @@ int kvm_mmu_module_init(void) { int ret = -ENOMEM; - if (nx_huge_pages == -1) + if (nx_huge_pages == -1) { __set_nx_huge_pages(get_nx_auto_mode()); + if (is_nx_huge_page_enabled()) + pr_warn_once(ITLB_MULTIHIT_NX_ON); + } /* * MMU roles use union aliasing which is, generally speaking, an
Add warning when `nx_huge_pages` is enabled by `auto` for hint that huge pages may be splited by kernel. Add warning when CVE-2018-12207 may arise but `nx_huge_pages` is disabled for hint that malicious guest may cause a CPU lookup. Signed-off-by: Li Yu <liyu.yukiteru@bytedance.com> --- arch/x86/kvm/mmu/mmu.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)