| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 26 Feb 2023 17:12:04 +0000
From: "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To: Juergen Gross <jgross@...e.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"x86@...nel.org" <x86@...nel.org>
CC: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>
Subject: RE: [PATCH v3 03/12] x86/mtrr: support setting MTRR state for
software defined MTRRs
From: Juergen Gross <jgross@...e.com> Sent: Thursday, February 23, 2023 1:33 AM
>
> When running virtualized, MTRR access can be reduced (e.g. in Xen PV
> guests or when running as a SEV-SNP guest under Hyper-V). Typically
> the hypervisor will reset the MTRR feature in CPUID data, resulting
> in no MTRR memory type information being available for the kernel.
>
> This has turned out to result in problems:
>
> - Hyper-V SEV-SNP guests using uncached mappings where they shouldn't
> - Xen PV dom0 mapping memory as WB which should be UC- instead
>
> Solve those problems by supporting to set a static MTRR state,
> overwriting the empty state used today. In case such a state has been
> set, don't call get_mtrr_state() in mtrr_bp_init(). The set state
> will only be used by mtrr_type_lookup(), as in all other cases
> mtrr_enabled() is being checked, which will return false. Accept the
> overwrite call only for selected cases when running as a guest.
> Disable X86_FEATURE_MTRR in order to avoid any MTRR modifications by
> just refusing them.
>
> Signed-off-by: Juergen Gross <jgross@...e.com>
> ---
> V2:
> - new patch
> V3:
> - omit fixed MTRRs, as those are currently not needed
> - disable X86_FEATURE_MTRR instead of testing it
> - provide a stub for !CONFIG_MTRR (Michael Kelley)
> - use cpu_feature_enabled() (Boris Petkov)
> - add tests for mtrr_overwrite_state() being allowed (Boris Petkov)
> ---
> arch/x86/include/asm/mtrr.h | 8 ++++++
> arch/x86/kernel/cpu/mtrr/generic.c | 43 ++++++++++++++++++++++++++++++
> arch/x86/kernel/cpu/mtrr/mtrr.c | 9 +++++++
> arch/x86/kernel/setup.c | 2 ++
> 4 files changed, 62 insertions(+)
>
> diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h
> index f0eeaf6e5f5f..f1cb81330a64 100644
> --- a/arch/x86/include/asm/mtrr.h
> +++ b/arch/x86/include/asm/mtrr.h
> @@ -31,6 +31,8 @@
> */
> # ifdef CONFIG_MTRR
> void mtrr_bp_init(void);
> +void mtrr_overwrite_state(struct mtrr_var_range *var, unsigned int num_var,
> + mtrr_type def_type);
> extern u8 mtrr_type_lookup(u64 addr, u64 end, u8 *uniform);
> extern void mtrr_save_fixed_ranges(void *);
> extern void mtrr_save_state(void);
> @@ -48,6 +50,12 @@ void mtrr_disable(void);
> void mtrr_enable(void);
> void mtrr_generic_set_state(void);
> # else
> +static inline void mtrr_overwrite_state(struct mtrr_var_range *var,
> + unsigned int num_var,
> + mtrr_type def_type)
> +{
> +}
> +
> static inline u8 mtrr_type_lookup(u64 addr, u64 end, u8 *uniform)
> {
> /*
> diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
> index ee09d359e08f..40c59d522f57 100644
> --- a/arch/x86/kernel/cpu/mtrr/generic.c
> +++ b/arch/x86/kernel/cpu/mtrr/generic.c
> @@ -8,10 +8,12 @@
> #include <linux/init.h>
> #include <linux/io.h>
> #include <linux/mm.h>
> +#include <linux/cc_platform.h>
>
> #include <asm/processor-flags.h>
> #include <asm/cacheinfo.h>
> #include <asm/cpufeature.h>
> +#include <asm/hypervisor.h>
> #include <asm/tlbflush.h>
> #include <asm/mtrr.h>
> #include <asm/msr.h>
> @@ -240,6 +242,47 @@ static u8 mtrr_type_lookup_variable(u64 start, u64 end, u64
> *partial_end,
> return mtrr_state.def_type;
> }
>
> +/**
> + * mtrr_overwrite_state - set static MTRR state
> + *
> + * Used to set MTRR state via different means (e.g. with data obtained from
> + * a hypervisor).
> + * Is allowed only for special cases when running virtualized. Must be called
> + * from the x86_init.hyper.init_platform() hook. X86_FEATURE_MTRR must be off.
> + */
> +void mtrr_overwrite_state(struct mtrr_var_range *var, unsigned int num_var,
> + mtrr_type def_type)
> +{
> + unsigned int i;
> +
> + if (WARN_ON(mtrr_state_set ||
> + hypervisor_is_type(X86_HYPER_NATIVE) ||
> + !cpu_feature_enabled(X86_FEATURE_HYPERVISOR) ||
> + (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP) &&
With current upstream code, this test doesn't allow Hyper-V SNP vTOM VMs
to do the overwrite, as current upstream vTOM code doesn't participate in the
cc_platform_has() mechanism. That's being reworked in a separate patch
set. Can you add this test to cover the SNP vTOM case?
!hv_is_isolation_supported() &&
This is the same test used in __set_memory_enc_dec(), for example. You'll
have to add #include <asm/mshyperv.h>. There's already a stub that returns
'false' so that everything works when building with CONFIG_HYPERV=n.
Once my other patch set is accepted, I'll revise this to remove use of
hv_is_isolation_supported() outside of Hyper-V specific code, and use
the newer cc_platform_has() instead.
Michael
> + !cpu_feature_enabled(X86_FEATURE_XENPV) &&
> + !cpu_feature_enabled(X86_FEATURE_TDX_GUEST))))
> + return;
> +
> + /* Disable MTRR in order to disable MTRR modifications. */
> + setup_clear_cpu_cap(X86_FEATURE_MTRR);
> +
> + if (var) {
> + if (num_var > MTRR_MAX_VAR_RANGES) {
> + pr_warn("Trying to overwrite MTRR state with %u variable entries\n",
> + num_var);
> + num_var = MTRR_MAX_VAR_RANGES;
> + }
> + for (i = 0; i < num_var; i++)
> + mtrr_state.var_ranges[i] = var[i];
> + num_var_ranges = num_var;
> + }
> +
> + mtrr_state.def_type = def_type;
> + mtrr_state.enabled |= MTRR_STATE_MTRR_ENABLED;
> +
> + mtrr_state_set = 1;
> +}
> +
> /**
> * mtrr_type_lookup - look up memory type in MTRR
> *
> diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c
> index 7596ebeab929..5fe62ee0361b 100644
> --- a/arch/x86/kernel/cpu/mtrr/mtrr.c
> +++ b/arch/x86/kernel/cpu/mtrr/mtrr.c
> @@ -666,6 +666,15 @@ void __init mtrr_bp_init(void)
> const char *why = "(not available)";
> unsigned int phys_addr;
>
> + if (mtrr_state.enabled) {
> + /* Software overwrite of MTRR state, only for generic case. */
> + mtrr_calc_physbits(true);
> + init_table();
> + pr_info("MTRRs set to read-only\n");
> +
> + return;
> + }
> +
> phys_addr = mtrr_calc_physbits(boot_cpu_has(X86_FEATURE_MTRR));
>
> if (boot_cpu_has(X86_FEATURE_MTRR)) {
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 16babff771bd..0cccfeb67c3a 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1037,6 +1037,8 @@ void __init setup_arch(char **cmdline_p)
> /*
> * VMware detection requires dmi to be available, so this
> * needs to be done after dmi_setup(), for the boot CPU.
> + * For some guest types (Xen PV, SEV-SNP, TDX) it is required to be
> + * called before cache_bp_init() for setting up MTRR state.
> */
> init_hypervisor_platform();
>
> --
> 2.35.3
Powered by blists - more mailing lists