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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ