| 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
| ||
|
Message-ID: <20150325195941.GM25884@l.oracle.com>
Date: Wed, 25 Mar 2015 15:59:41 -0400
From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To: "Luis R. Rodriguez" <mcgrof@...not-panic.com>
Cc: luto@...capital.net, mingo@...hat.com, tglx@...utronix.de,
hpa@...or.com, jgross@...e.com, JBeulich@...e.com, bp@...e.de,
suresh.b.siddha@...el.com, venkatesh.pallipadi@...el.com,
airlied@...hat.com, linux-kernel@...r.kernel.org,
linux-fbdev@...r.kernel.org, x86@...nel.org,
xen-devel@...ts.xenproject.org,
"Luis R. Rodriguez" <mcgrof@...e.com>, Ingo Molnar <mingo@...e.hu>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Antonino Daplas <adaplas@...il.com>,
Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
Tomi Valkeinen <tomi.valkeinen@...com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Stefan Bader <stefan.bader@...onical.com>,
ville.syrjala@...ux.intel.com, david.vrabel@...rix.com,
toshi.kani@...com, bhelgaas@...gle.com,
Roger Pau Monné <roger.pau@...rix.com>,
xen-devel@...ts.xensource.com
Subject: Re: [PATCH v1 02/47] x86: mtrr: generalize run time disabling of MTRR
On Fri, Mar 20, 2015 at 04:17:52PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@...e.com>
>
> It is possible to enable CONFIG_MTRR and up with it
> disabled at run time and yet CONFIG_X86_PAT continues
> to kick through fully functionally. This can happen
s/fully/full/ ?
> for instance on Xen where MTRR is not supported but
> PAT is, this can happen now on Linux as of commit
> 47591df50 by Juergen introduced as of v3.19.
s/3.19/4.0/
>
> Technically we should assume the proper CPU
> bits would be set to disable MTRR but we can't
> always rely on this. At least on the Xen Hypervisor
> for instance only X86_FEATURE_MTRR was disabled
> as of Xen 4.4 through Xen commit 586ab6a [0],
> but not X86_FEATURE_K6_MTRR, X86_FEATURE_CENTAUR_MCR,
> or X86_FEATURE_CYRIX_ARR for instance.
Oh, could you send an patch for that to Xen please?
>
> x86 mtrr code relies on quite a bit of checks for
> mtrr_if being set to check to see if MTRR did get
> set up, instead of using that lets provide a generic
> setter which when set we know MTRR is enabled. This
s/we know MTRR is enabled/will let us know that MTRR is enabled/
> also adds a few checks where they were not before
> which could potentially safeguard ourselves against
> incorrect usage of MTRR where this was not desirable.
>
> Where possible match error codes as if MTRR was
> disabled on arch/x86/include/asm/mtrr.h.
>
> Lastly, since disabling MTRR can happen at run time
> and we could end up with PAT enabled best record now
> on our logs when MTRR is disabled.
>
> [0] ~/devel/xen (git::stable-4.5)$ git describe --contains 586ab6a
> 4.4.0-rc1~18
>
> Cc: Andy Lutomirski <luto@...capital.net>
> Cc: Suresh Siddha <suresh.b.siddha@...el.com>
> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@...el.com>
> Cc: Ingo Molnar <mingo@...e.hu>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Juergen Gross <jgross@...e.com>
> Cc: Daniel Vetter <daniel.vetter@...ll.ch>
> Cc: Dave Airlie <airlied@...hat.com>
> Cc: Antonino Daplas <adaplas@...il.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@...com>
> Cc: Dave Hansen <dave.hansen@...ux.intel.com>
> Cc: venkatesh.pallipadi@...el.com
> Cc: Stefan Bader <stefan.bader@...onical.com>
> Cc: konrad.wilk@...cle.com
> Cc: ville.syrjala@...ux.intel.com
> Cc: david.vrabel@...rix.com
> Cc: jbeulich@...e.com
> Cc: toshi.kani@...com
> Cc: bhelgaas@...gle.com
> Cc: Roger Pau Monné <roger.pau@...rix.com>
> Cc: linux-fbdev@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> Cc: xen-devel@...ts.xensource.com
> Signed-off-by: Luis R. Rodriguez <mcgrof@...e.com>
> ---
> arch/x86/include/asm/mtrr.h | 2 ++
> arch/x86/kernel/cpu/mtrr/cleanup.c | 2 +-
> arch/x86/kernel/cpu/mtrr/generic.c | 5 +++--
> arch/x86/kernel/cpu/mtrr/if.c | 3 +++
> arch/x86/kernel/cpu/mtrr/main.c | 31 ++++++++++++++++++++++---------
> 5 files changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h
> index f768f62..cade917 100644
> --- a/arch/x86/include/asm/mtrr.h
> +++ b/arch/x86/include/asm/mtrr.h
> @@ -31,6 +31,7 @@
> * arch_phys_wc_add and arch_phys_wc_del.
> */
> # ifdef CONFIG_MTRR
> +extern int mtrr_enabled;
> extern u8 mtrr_type_lookup(u64 addr, u64 end);
> extern void mtrr_save_fixed_ranges(void *);
> extern void mtrr_save_state(void);
> @@ -50,6 +51,7 @@ extern int mtrr_trim_uncached_memory(unsigned long end_pfn);
> extern int amd_special_default_mtrr(void);
> extern int phys_wc_to_mtrr_index(int handle);
> # else
> +static const int mtrr_enabled;
> static inline u8 mtrr_type_lookup(u64 addr, u64 end)
> {
> /*
> diff --git a/arch/x86/kernel/cpu/mtrr/cleanup.c b/arch/x86/kernel/cpu/mtrr/cleanup.c
> index 5f90b85..784dc55 100644
> --- a/arch/x86/kernel/cpu/mtrr/cleanup.c
> +++ b/arch/x86/kernel/cpu/mtrr/cleanup.c
> @@ -880,7 +880,7 @@ int __init mtrr_trim_uncached_memory(unsigned long end_pfn)
> * Make sure we only trim uncachable memory on machines that
> * support the Intel MTRR architecture:
> */
> - if (!is_cpu(INTEL) || disable_mtrr_trim)
> + if (!is_cpu(INTEL) || disable_mtrr_trim || !mtrr_enabled)
> return 0;
>
> rdmsr(MSR_MTRRdefType, def, dummy);
> diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
> index 09c82de..df321b2 100644
> --- a/arch/x86/kernel/cpu/mtrr/generic.c
> +++ b/arch/x86/kernel/cpu/mtrr/generic.c
> @@ -116,7 +116,8 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat)
> u8 prev_match, curr_match;
>
> *repeat = 0;
> - if (!mtrr_state_set)
> + /* generic_mtrr_ops is only set for generic_mtrr_ops */
> + if (!mtrr_state_set || !mtrr_enabled)
> return 0xFF;
>
> if (!mtrr_state.enabled)
> @@ -290,7 +291,7 @@ static void get_fixed_ranges(mtrr_type *frs)
>
> void mtrr_save_fixed_ranges(void *info)
> {
> - if (cpu_has_mtrr)
> + if (mtrr_enabled && cpu_has_mtrr)
> get_fixed_ranges(mtrr_state.fixed_ranges);
> }
>
> diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
> index d76f13d..e9e001a 100644
> --- a/arch/x86/kernel/cpu/mtrr/if.c
> +++ b/arch/x86/kernel/cpu/mtrr/if.c
> @@ -436,6 +436,9 @@ static int __init mtrr_if_init(void)
> {
> struct cpuinfo_x86 *c = &boot_cpu_data;
>
> + if (!mtrr_enabled)
> + return 0;
> +
> if ((!cpu_has(c, X86_FEATURE_MTRR)) &&
> (!cpu_has(c, X86_FEATURE_K6_MTRR)) &&
> (!cpu_has(c, X86_FEATURE_CYRIX_ARR)) &&
> diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
> index ea5f363..7db9c47 100644
> --- a/arch/x86/kernel/cpu/mtrr/main.c
> +++ b/arch/x86/kernel/cpu/mtrr/main.c
> @@ -59,6 +59,7 @@
> #define MTRR_TO_PHYS_WC_OFFSET 1000
>
> u32 num_var_ranges;
> +int mtrr_enabled;
>
> unsigned int mtrr_usage_table[MTRR_MAX_VAR_RANGES];
> static DEFINE_MUTEX(mtrr_mutex);
> @@ -84,6 +85,9 @@ static int have_wrcomb(void)
> {
> struct pci_dev *dev;
>
> + if (!mtrr_enabled)
> + return 0;
> +
> dev = pci_get_class(PCI_CLASS_BRIDGE_HOST << 8, NULL);
> if (dev != NULL) {
> /*
> @@ -286,7 +290,7 @@ int mtrr_add_page(unsigned long base, unsigned long size,
> int i, replace, error;
> mtrr_type ltype;
>
> - if (!mtrr_if)
> + if (!mtrr_enabled)
> return -ENXIO;
>
> error = mtrr_if->validate_add_page(base, size, type);
> @@ -388,6 +392,8 @@ int mtrr_add_page(unsigned long base, unsigned long size,
>
> static int mtrr_check(unsigned long base, unsigned long size)
> {
> + if (!mtrr_enabled)
> + return -ENODEV;
> if ((base & (PAGE_SIZE - 1)) || (size & (PAGE_SIZE - 1))) {
> pr_warning("mtrr: size and base must be multiples of 4 kiB\n");
> pr_debug("mtrr: size: 0x%lx base: 0x%lx\n", size, base);
> @@ -463,8 +469,8 @@ int mtrr_del_page(int reg, unsigned long base, unsigned long size)
> unsigned long lbase, lsize;
> int error = -EINVAL;
>
> - if (!mtrr_if)
> - return -ENXIO;
> + if (!mtrr_enabled)
> + return -ENODEV;
>
> max = num_var_ranges;
> /* No CPU hotplug when we change MTRR entries */
> @@ -523,6 +529,8 @@ int mtrr_del_page(int reg, unsigned long base, unsigned long size)
> */
> int mtrr_del(int reg, unsigned long base, unsigned long size)
> {
> + if (!mtrr_enabled)
> + return -ENODEV;
> if (mtrr_check(base, size))
> return -EINVAL;
> return mtrr_del_page(reg, base >> PAGE_SHIFT, size >> PAGE_SHIFT);
> @@ -545,7 +553,7 @@ int arch_phys_wc_add(unsigned long base, unsigned long size)
> {
> int ret;
>
> - if (pat_enabled)
> + if (pat_enabled || !mtrr_enabled)
> return 0; /* Success! (We don't need to do anything.) */
>
> ret = mtrr_add(base, size, MTRR_TYPE_WRCOMB, true);
> @@ -734,6 +742,7 @@ void __init mtrr_bp_init(void)
> }
>
> if (mtrr_if) {
> + mtrr_enabled = true;
> set_num_var_ranges();
> init_table();
> if (use_intel()) {
> @@ -744,12 +753,13 @@ void __init mtrr_bp_init(void)
> mtrr_if->set_all();
> }
> }
> - }
> + } else
> + pr_info("mtrr: system does not support MTRR\n");
'pr_warn' ?
> }
>
> void mtrr_ap_init(void)
> {
> - if (!use_intel() || mtrr_aps_delayed_init)
> + if (!use_intel() || mtrr_aps_delayed_init || !mtrr_enabled)
> return;
> /*
> * Ideally we should hold mtrr_mutex here to avoid mtrr entries
> @@ -774,6 +784,9 @@ void mtrr_save_state(void)
> {
> int first_cpu;
>
> + if (!mtrr_enabled)
> + return;
> +
> get_online_cpus();
> first_cpu = cpumask_first(cpu_online_mask);
> smp_call_function_single(first_cpu, mtrr_save_fixed_ranges, NULL, 1);
> @@ -782,7 +795,7 @@ void mtrr_save_state(void)
>
> void set_mtrr_aps_delayed_init(void)
> {
> - if (!use_intel())
> + if (!use_intel() || !mtrr_enabled)
> return;
>
> mtrr_aps_delayed_init = true;
> @@ -810,7 +823,7 @@ void mtrr_aps_init(void)
>
> void mtrr_bp_restore(void)
> {
> - if (!use_intel())
> + if (!use_intel() || !mtrr_enabled)
> return;
>
> mtrr_if->set_all();
> @@ -818,7 +831,7 @@ void mtrr_bp_restore(void)
>
> static int __init mtrr_init_finialize(void)
> {
> - if (!mtrr_if)
> + if (!mtrr_enabled)
> return 0;
>
> if (use_intel()) {
> --
> 2.3.2.209.gd67f9d5.dirty
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists