| 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: <CAFn7gfRibD3YCBdXgtHuR0hMzJb+MYBNWHN5h+KJ1wJGzfL1sg@mail.gmail.com>
Date: Thu, 24 Aug 2023 15:48:18 -0700
From: Ben Hillier <bhillier@...gle.com>
To: Steve Rutherford <srutherford@...gle.com>
Cc: Borislav Petkov <bp@...en8.de>,
Thomas Gleixner <tglx@...utronix.de>, thomas.lendacky@....com,
pankaj.gupta@....com, Paolo Bonzini <pbonzini@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Ingo Molnar <mingo@...hat.com>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H . Peter Anvin" <hpa@...or.com>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, David.Kaplan@....com,
jacobhxu@...gle.com, patelsvishal@...gle.com,
stable@...r.kernel.org
Subject: Re: [PATCH v3] x86/sev: Make enc_dec_hypercall() accept a size
instead of npages
On Thu, Aug 24, 2023 at 3:37 PM Steve Rutherford <srutherford@...gle.com> wrote:
>
> enc_dec_hypercall() accepted a page count instead of a size, which
> forced its callers to round up. As a result, non-page aligned
> vaddrs caused pages to be spuriously marked as decrypted via the
> encryption status hypercall, which in turn caused consistent
> corruption of pages during live migration. Live migration requires
> accurate encryption status information to avoid migrating pages
> from the wrong perspective.
>
> Cc: stable@...r.kernel.org
> Fixes: 064ce6c550a0 ("mm: x86: Invoke hypercall when page encryption status is changed")
> Signed-off-by: Steve Rutherford <srutherford@...gle.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@....com>
> Reviewed-by: Pankaj Gupta <pankaj.gupta@....com>
Ran test comparing the c-bit status in the guest page tables to the
host perspective. Before the patch, there was a c-bit status mismatch.
Adding the patch fixed these mismatched c-bits.
Tested-by: Ben Hillier <bhillier@...gle.com>
> ---
> arch/x86/include/asm/mem_encrypt.h | 6 +++---
> arch/x86/kernel/kvm.c | 4 +---
> arch/x86/mm/mem_encrypt_amd.c | 13 ++++++-------
> 3 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> index 7f97a8a97e24..473b16d73b47 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -50,8 +50,8 @@ void __init sme_enable(struct boot_params *bp);
>
> int __init early_set_memory_decrypted(unsigned long vaddr, unsigned long size);
> int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size);
> -void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages,
> - bool enc);
> +void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr,
> + unsigned long size, bool enc);
>
> void __init mem_encrypt_free_decrypted_mem(void);
>
> @@ -85,7 +85,7 @@ early_set_memory_decrypted(unsigned long vaddr, unsigned long size) { return 0;
> static inline int __init
> early_set_memory_encrypted(unsigned long vaddr, unsigned long size) { return 0; }
> static inline void __init
> -early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages, bool enc) {}
> +early_set_mem_enc_dec_hypercall(unsigned long vaddr, unsigned long size, bool enc) {}
>
> static inline void mem_encrypt_free_decrypted_mem(void) { }
>
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 6a36db4f79fd..b8ab9ee5896c 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -966,10 +966,8 @@ static void __init kvm_init_platform(void)
> * Ensure that _bss_decrypted section is marked as decrypted in the
> * shared pages list.
> */
> - nr_pages = DIV_ROUND_UP(__end_bss_decrypted - __start_bss_decrypted,
> - PAGE_SIZE);
> early_set_mem_enc_dec_hypercall((unsigned long)__start_bss_decrypted,
> - nr_pages, 0);
> + __end_bss_decrypted - __start_bss_decrypted, 0);
>
> /*
> * If not booted using EFI, enable Live migration support.
> diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
> index 54bbd5163e8d..6faea41e99b6 100644
> --- a/arch/x86/mm/mem_encrypt_amd.c
> +++ b/arch/x86/mm/mem_encrypt_amd.c
> @@ -288,11 +288,10 @@ static bool amd_enc_cache_flush_required(void)
> return !cpu_feature_enabled(X86_FEATURE_SME_COHERENT);
> }
>
> -static void enc_dec_hypercall(unsigned long vaddr, int npages, bool enc)
> +static void enc_dec_hypercall(unsigned long vaddr, unsigned long size, bool enc)
> {
> #ifdef CONFIG_PARAVIRT
> - unsigned long sz = npages << PAGE_SHIFT;
> - unsigned long vaddr_end = vaddr + sz;
> + unsigned long vaddr_end = vaddr + size;
>
> while (vaddr < vaddr_end) {
> int psize, pmask, level;
> @@ -342,7 +341,7 @@ static bool amd_enc_status_change_finish(unsigned long vaddr, int npages, bool e
> snp_set_memory_private(vaddr, npages);
>
> if (!cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
> - enc_dec_hypercall(vaddr, npages, enc);
> + enc_dec_hypercall(vaddr, npages << PAGE_SHIFT, enc);
>
> return true;
> }
> @@ -466,7 +465,7 @@ static int __init early_set_memory_enc_dec(unsigned long vaddr,
>
> ret = 0;
>
> - early_set_mem_enc_dec_hypercall(start, PAGE_ALIGN(size) >> PAGE_SHIFT, enc);
> + early_set_mem_enc_dec_hypercall(start, size, enc);
> out:
> __flush_tlb_all();
> return ret;
> @@ -482,9 +481,9 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size)
> return early_set_memory_enc_dec(vaddr, size, true);
> }
>
> -void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages, bool enc)
> +void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr, unsigned long size, bool enc)
> {
> - enc_dec_hypercall(vaddr, npages, enc);
> + enc_dec_hypercall(vaddr, size, enc);
> }
>
> void __init sme_early_init(void)
> --
> 2.42.0.rc1.204.g551eb34607-goog
>
Powered by blists - more mailing lists