| 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: <CABgObfbKV3TvVeixtVvp6f9Qtr47aiStUBsDH2_c6jrDGiWR_A@mail.gmail.com>
Date: Sun, 15 Oct 2023 13:59:06 +0200
From: Paolo Bonzini <pbonzini@...hat.com>
To: Sasha Levin <sashal@...nel.org>
Cc: linux-kernel@...r.kernel.org, stable@...r.kernel.org,
Sean Christopherson <seanjc@...gle.com>,
Andrew Cooper <Andrew.Cooper3@...rix.com>, tglx@...utronix.de,
mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
x86@...nel.org, akpm@...ux-foundation.org, bhe@...hat.com,
eric.devolder@...cle.com, hbathini@...ux.ibm.com,
sourabhjain@...ux.ibm.com, bhelgaas@...gle.com,
kai.huang@...el.com, peterz@...radead.org, jpoimboe@...nel.org,
tiwai@...e.de, kvm@...r.kernel.org
Subject: Re: [PATCH AUTOSEL 6.5 1/7] x86/reboot: VMCLEAR active VMCSes before
emergency reboot
On Thu, Sep 14, 2023 at 3:55 AM Sasha Levin <sashal@...nel.org> wrote:
>
> From: Sean Christopherson <seanjc@...gle.com>
>
> [ Upstream commit b23c83ad2c638420ec0608a9de354507c41bec29 ]
>
> VMCLEAR active VMCSes before any emergency reboot, not just if the kernel
> may kexec into a new kernel after a crash. Per Intel's SDM, the VMX
> architecture doesn't require the CPU to flush the VMCS cache on INIT. If
> an emergency reboot doesn't RESET CPUs, cached VMCSes could theoretically
> be kept and only be written back to memory after the new kernel is booted,
> i.e. could effectively corrupt memory after reboot.
>
> Opportunistically remove the setting of the global pointer to NULL to make
> checkpatch happy.
Intended as a cleanup but I guess it does not hurt, since it was the first patch
in the large series that included it.
Acked-by: Paolo Bonzini <pbonzini@...hat.com>
Paolo
> Cc: Andrew Cooper <Andrew.Cooper3@...rix.com>
> Link: https://lore.kernel.org/r/20230721201859.2307736-2-seanjc@google.com
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
> arch/x86/include/asm/kexec.h | 2 --
> arch/x86/include/asm/reboot.h | 2 ++
> arch/x86/kernel/crash.c | 31 -------------------------------
> arch/x86/kernel/reboot.c | 22 ++++++++++++++++++++++
> arch/x86/kvm/vmx/vmx.c | 10 +++-------
> 5 files changed, 27 insertions(+), 40 deletions(-)
>
> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
> index 5b77bbc28f969..819046974b997 100644
> --- a/arch/x86/include/asm/kexec.h
> +++ b/arch/x86/include/asm/kexec.h
> @@ -205,8 +205,6 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image);
> #endif
> #endif
>
> -typedef void crash_vmclear_fn(void);
> -extern crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
> extern void kdump_nmi_shootdown_cpus(void);
>
> #endif /* __ASSEMBLY__ */
> diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h
> index 9177b4354c3f5..dc201724a6433 100644
> --- a/arch/x86/include/asm/reboot.h
> +++ b/arch/x86/include/asm/reboot.h
> @@ -25,6 +25,8 @@ void __noreturn machine_real_restart(unsigned int type);
> #define MRR_BIOS 0
> #define MRR_APM 1
>
> +typedef void crash_vmclear_fn(void);
> +extern crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
> void cpu_emergency_disable_virtualization(void);
>
> typedef void (*nmi_shootdown_cb)(int, struct pt_regs*);
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index cdd92ab43cda4..54cd959cb3160 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -48,38 +48,12 @@ struct crash_memmap_data {
> unsigned int type;
> };
>
> -/*
> - * This is used to VMCLEAR all VMCSs loaded on the
> - * processor. And when loading kvm_intel module, the
> - * callback function pointer will be assigned.
> - *
> - * protected by rcu.
> - */
> -crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss = NULL;
> -EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss);
> -
> -static inline void cpu_crash_vmclear_loaded_vmcss(void)
> -{
> - crash_vmclear_fn *do_vmclear_operation = NULL;
> -
> - rcu_read_lock();
> - do_vmclear_operation = rcu_dereference(crash_vmclear_loaded_vmcss);
> - if (do_vmclear_operation)
> - do_vmclear_operation();
> - rcu_read_unlock();
> -}
> -
> #if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
>
> static void kdump_nmi_callback(int cpu, struct pt_regs *regs)
> {
> crash_save_cpu(regs, cpu);
>
> - /*
> - * VMCLEAR VMCSs loaded on all cpus if needed.
> - */
> - cpu_crash_vmclear_loaded_vmcss();
> -
> /*
> * Disable Intel PT to stop its logging
> */
> @@ -133,11 +107,6 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
>
> crash_smp_send_stop();
>
> - /*
> - * VMCLEAR VMCSs loaded on this cpu if needed.
> - */
> - cpu_crash_vmclear_loaded_vmcss();
> -
> cpu_emergency_disable_virtualization();
>
> /*
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index 3adbe97015c13..3fa4c6717a1db 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -787,6 +787,26 @@ void machine_crash_shutdown(struct pt_regs *regs)
> }
> #endif
>
> +/*
> + * This is used to VMCLEAR all VMCSs loaded on the
> + * processor. And when loading kvm_intel module, the
> + * callback function pointer will be assigned.
> + *
> + * protected by rcu.
> + */
> +crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
> +EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss);
> +
> +static inline void cpu_crash_vmclear_loaded_vmcss(void)
> +{
> + crash_vmclear_fn *do_vmclear_operation = NULL;
> +
> + rcu_read_lock();
> + do_vmclear_operation = rcu_dereference(crash_vmclear_loaded_vmcss);
> + if (do_vmclear_operation)
> + do_vmclear_operation();
> + rcu_read_unlock();
> +}
>
> /* This is the CPU performing the emergency shutdown work. */
> int crashing_cpu = -1;
> @@ -798,6 +818,8 @@ int crashing_cpu = -1;
> */
> void cpu_emergency_disable_virtualization(void)
> {
> + cpu_crash_vmclear_loaded_vmcss();
> +
> cpu_emergency_vmxoff();
> cpu_emergency_svm_disable();
> }
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index df461f387e20d..f60fb79fea881 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -41,7 +41,7 @@
> #include <asm/idtentry.h>
> #include <asm/io.h>
> #include <asm/irq_remapping.h>
> -#include <asm/kexec.h>
> +#include <asm/reboot.h>
> #include <asm/perf_event.h>
> #include <asm/mmu_context.h>
> #include <asm/mshyperv.h>
> @@ -754,7 +754,6 @@ static int vmx_set_guest_uret_msr(struct vcpu_vmx *vmx,
> return ret;
> }
>
> -#ifdef CONFIG_KEXEC_CORE
> static void crash_vmclear_local_loaded_vmcss(void)
> {
> int cpu = raw_smp_processor_id();
> @@ -764,7 +763,6 @@ static void crash_vmclear_local_loaded_vmcss(void)
> loaded_vmcss_on_cpu_link)
> vmcs_clear(v->vmcs);
> }
> -#endif /* CONFIG_KEXEC_CORE */
>
> static void __loaded_vmcs_clear(void *arg)
> {
> @@ -8622,10 +8620,9 @@ static void __vmx_exit(void)
> {
> allow_smaller_maxphyaddr = false;
>
> -#ifdef CONFIG_KEXEC_CORE
> RCU_INIT_POINTER(crash_vmclear_loaded_vmcss, NULL);
> synchronize_rcu();
> -#endif
> +
> vmx_cleanup_l1d_flush();
> }
>
> @@ -8674,10 +8671,9 @@ static int __init vmx_init(void)
> pi_init_cpu(cpu);
> }
>
> -#ifdef CONFIG_KEXEC_CORE
> rcu_assign_pointer(crash_vmclear_loaded_vmcss,
> crash_vmclear_local_loaded_vmcss);
> -#endif
> +
> vmx_check_vmcs12_offsets();
>
> /*
> --
> 2.40.1
>
Powered by blists - more mailing lists