* [PATCH] KVM: x86/mmu: Warn on nx_huge_pages when initializing kvm @ 2021-10-19 12:18 Li Yu 2021-10-19 12:56 ` Paolo Bonzini 0 siblings, 1 reply; 6+ messages in thread From: Li Yu @ 2021-10-19 12:18 UTC (permalink / raw) To: pbonzini Cc: liyu.yukiteru, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, kvm, linux-kernel Add warning when `nx_huge_pages` is enabled by kvm mmu for hint that huge pages may be splited by kernel. Signed-off-by: Li Yu <liyu.yukiteru@bytedance.com> --- arch/x86/kvm/mmu/mmu.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 1a64ba5b9437..b75dbaf29f2d 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -6091,12 +6091,17 @@ static int set_nx_huge_pages(const char *val, const struct kernel_param *kp) return 0; } +#define ITLB_MULTIHIT_MSG "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" + 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_MSG); + } /* * MMU roles use union aliasing which is, generally speaking, an -- 2.11.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Warn on nx_huge_pages when initializing kvm 2021-10-19 12:18 [PATCH] KVM: x86/mmu: Warn on nx_huge_pages when initializing kvm Li Yu @ 2021-10-19 12:56 ` Paolo Bonzini 2021-10-19 14:11 ` [PATCH] KVM: x86/mmu: Warn on nx_huge_pages for possible problems Li Yu 0 siblings, 1 reply; 6+ messages in thread From: Paolo Bonzini @ 2021-10-19 12:56 UTC (permalink / raw) To: Li Yu Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, kvm, linux-kernel On 19/10/21 14:18, Li Yu wrote: > Add warning when `nx_huge_pages` is enabled by kvm mmu for hint that > huge pages may be splited by kernel. > > Signed-off-by: Li Yu <liyu.yukiteru@bytedance.com> > --- > arch/x86/kvm/mmu/mmu.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 1a64ba5b9437..b75dbaf29f2d 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -6091,12 +6091,17 @@ static int set_nx_huge_pages(const char *val, const struct kernel_param *kp) > return 0; > } > > +#define ITLB_MULTIHIT_MSG "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" Shouldn't it warn if mitigations are disabled but the bug is present? Paolo > 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_MSG); > + } > > /* > * MMU roles use union aliasing which is, generally speaking, an > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] KVM: x86/mmu: Warn on nx_huge_pages for possible problems 2021-10-19 12:56 ` Paolo Bonzini @ 2021-10-19 14:11 ` Li Yu 2021-10-19 16:23 ` Sean Christopherson 0 siblings, 1 reply; 6+ messages in thread From: Li Yu @ 2021-10-19 14:11 UTC (permalink / raw) To: pbonzini Cc: liyu.yukiteru, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, kvm, linux-kernel 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(-) 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 -- 2.11.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Warn on nx_huge_pages for possible problems 2021-10-19 14:11 ` [PATCH] KVM: x86/mmu: Warn on nx_huge_pages for possible problems Li Yu @ 2021-10-19 16:23 ` Sean Christopherson 2021-10-20 4:31 ` [PATCH v3] KVM: x86/mmu: Warn on iTLB multi-hit " Li Yu 0 siblings, 1 reply; 6+ messages in thread From: Sean Christopherson @ 2021-10-19 16:23 UTC (permalink / raw) To: Li Yu Cc: pbonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, kvm, linux-kernel 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3] KVM: x86/mmu: Warn on iTLB multi-hit for possible problems 2021-10-19 16:23 ` Sean Christopherson @ 2021-10-20 4:31 ` Li Yu 2021-12-08 18:40 ` Sean Christopherson 0 siblings, 1 reply; 6+ messages in thread From: Li Yu @ 2021-10-20 4:31 UTC (permalink / raw) To: pbonzini Cc: liyu.yukiteru, Jonathan Corbet, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, Paul E. McKenney, Andrew Morton, Peter Zijlstra, Lu Baolu, Maciej W. Rozycki, Muchun Song, Viresh Kumar, Robin Murphy, Randy Dunlap, linux-doc, linux-kernel, kvm Warn for guest huge pages split if iTLB multi-hit bug is present and CPU mitigations is enabled. Warn for possible CPU lockup if iTLB multi-hit bug is present but CPU mitigations is disabled. Signed-off-by: Li Yu <liyu.yukiteru@bytedance.com> --- Documentation/admin-guide/hw-vuln/multihit.rst | 8 +++-- Documentation/admin-guide/kernel-parameters.txt | 10 +++--- arch/x86/kvm/mmu/mmu.c | 48 +++++++++++++++++++++---- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/Documentation/admin-guide/hw-vuln/multihit.rst b/Documentation/admin-guide/hw-vuln/multihit.rst index 140e4cec38c3..7b2cd027d759 100644 --- a/Documentation/admin-guide/hw-vuln/multihit.rst +++ b/Documentation/admin-guide/hw-vuln/multihit.rst @@ -129,19 +129,21 @@ boot time with the option "kvm.nx_huge_pages=". The valid arguments for these options are: - ========== ================================================================ + ========== ================================================================= force Mitigation is enabled. In this case, the mitigation implements non-executable huge pages in Linux kernel KVM module. All huge pages in the EPT are marked as non-executable. If a guest attempts to execute in one of those pages, the page is broken down into 4K pages, which are then marked executable. - off Mitigation is disabled. + off Mitigation is disabled. + + off,nowarn Same as 'off', but hypervisors will not warn when KVM is loaded. auto Enable mitigation only if the platform is affected and the kernel was not booted with the "mitigations=off" command line parameter. This is the default option. - ========== ================================================================ + ========== ================================================================= Mitigation selection guide diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 43dc35fe5bc0..8f014cf462a3 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -2339,10 +2339,12 @@ kvm.nx_huge_pages= [KVM] Controls the software workaround for the X86_BUG_ITLB_MULTIHIT bug. - force : Always deploy workaround. - off : Never deploy workaround. - auto : Deploy workaround based on the presence of - X86_BUG_ITLB_MULTIHIT. + force : Always deploy workaround. + off : Never deploy workaround. + off,nowarn : Same as 'off', but hypervisors will not + warn when KVM is loaded. + auto : Deploy workaround based on the presence of + X86_BUG_ITLB_MULTIHIT and cpu mitigations. Default is 'auto'. diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 1a64ba5b9437..b9dc68e3dc2c 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -6056,20 +6056,41 @@ 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, guest huge pages may split 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 but cpu mitigations disabled, malicious guest may cause a CPU lockup. 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; + bool nowarn = false; /* 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, "off,nowarn")) { new_val = 0; - else if (sysfs_streq(val, "force")) + nowarn = true; + } else if (sysfs_streq(val, "force")) { + /* + * When `force` is set, admin should know that no matter whether + * CPU has the bug or not, guest pages may split anyway. So warn + * is not needed. + */ new_val = 1; - else if (sysfs_streq(val, "auto")) + nowarn = true; + } else if (sysfs_streq(val, "auto")) { new_val = get_nx_auto_mode(); - else if (strtobool(val, &new_val) < 0) + } else if (strtobool(val, &new_val) < 0) { return -EINVAL; + } + + if (!nowarn && boot_cpu_has_bug(X86_BUG_ITLB_MULTIHIT)) { + if (new_val) + pr_warn_once(ITLB_MULTIHIT_NX_ON); + else + pr_warn_once(ITLB_MULTIHIT_NX_OFF); + } __set_nx_huge_pages(new_val); @@ -6094,9 +6115,24 @@ static int set_nx_huge_pages(const char *val, const struct kernel_param *kp) int kvm_mmu_module_init(void) { int ret = -ENOMEM; + bool mode; - if (nx_huge_pages == -1) - __set_nx_huge_pages(get_nx_auto_mode()); + if (nx_huge_pages == -1) { + mode = get_nx_auto_mode(); + if (boot_cpu_has_bug(X86_BUG_ITLB_MULTIHIT)) { + /* + * Warn on the CPU multi-hit bug when `nx_huge_pages` is `auto` + * by default. If cpu mitigations was enabled, warn that guest + * huge pages may split, otherwise warn that the bug may cause + * a CPU lockup because of a malicious guest. + */ + if (mode) + pr_warn_once(ITLB_MULTIHIT_NX_ON); + else + pr_warn_once(ITLB_MULTIHIT_NX_OFF); + } + __set_nx_huge_pages(mode); + } /* * MMU roles use union aliasing which is, generally speaking, an -- 2.11.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] KVM: x86/mmu: Warn on iTLB multi-hit for possible problems 2021-10-20 4:31 ` [PATCH v3] KVM: x86/mmu: Warn on iTLB multi-hit " Li Yu @ 2021-12-08 18:40 ` Sean Christopherson 0 siblings, 0 replies; 6+ messages in thread From: Sean Christopherson @ 2021-12-08 18:40 UTC (permalink / raw) To: Li Yu Cc: pbonzini, Jonathan Corbet, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin, Paul E. McKenney, Andrew Morton, Peter Zijlstra, Lu Baolu, Maciej W. Rozycki, Muchun Song, Viresh Kumar, Robin Murphy, Randy Dunlap, linux-doc, linux-kernel, kvm On Wed, Oct 20, 2021, Li Yu wrote: > Warn for guest huge pages split if iTLB multi-hit bug is present > and CPU mitigations is enabled. > > Warn for possible CPU lockup if iTLB multi-hit bug is present but > CPU mitigations is disabled. > > Signed-off-by: Li Yu <liyu.yukiteru@bytedance.com> > --- Gah, my last reply in the previous version was offlist. I would like an answer to the below question before we complicate KVM just to log a message. On Wed, Oct 20, 2021 at 12:51 PM Sean Christopherson <seanjc@google.com> wrote: > > On Wed, Oct 20, 2021, Li Yu wrote: > > On 2021/10/20 00:23, Sean Christopherson wrote: > > I think it is necessary to log something when `nx_huge_pages` is `auto` or > > `off`, and the bug is present. > > Why is it necessary? Specifically, what action can be taken based on KVM logging > that can't reasonably be taken based on all the other information provided by the > kernel? We should teach userspace to go look at the vulnerabilites info, not to > scrape the kernel log. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-08 18:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-19 12:18 [PATCH] KVM: x86/mmu: Warn on nx_huge_pages when initializing kvm Li Yu 2021-10-19 12:56 ` Paolo Bonzini 2021-10-19 14:11 ` [PATCH] KVM: x86/mmu: Warn on nx_huge_pages for possible problems Li Yu 2021-10-19 16:23 ` Sean Christopherson 2021-10-20 4:31 ` [PATCH v3] KVM: x86/mmu: Warn on iTLB multi-hit " Li Yu 2021-12-08 18:40 ` Sean Christopherson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).