linux-kernel - Re: [PATCH v2] x86/sev: Make enc_dec_hypercall() accept a size instead of npages

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: <ecbadef3-8982-bd1f-54e6-99678c2518d1@amd.com>
Date:   Tue, 22 Aug 2023 16:28:21 -0500
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Steve Rutherford <srutherford@...gle.com>,
        Borislav Petkov <bp@...en8.de>,
        Thomas Gleixner <tglx@...utronix.de>, pankaj.gupta@....com
Cc:     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, bhillier@...gle.com
Subject: Re: [PATCH v2] x86/sev: Make enc_dec_hypercall() accept a size
 instead of npages

On 8/21/23 17:58, Steve Rutherford 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.
> 
> 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>

> ---
>   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)