linux-kernel - Re: [PATCH v3 02/14] x86/sev: Make the VMPL0 checking function more generic

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]

Date: Fri, 12 Apr 2024 18:41:27 +0200
From: "Gupta, Pankaj" <pankaj.gupta@....com>
To: Tom Lendacky <thomas.lendacky@....com>, linux-kernel@...r.kernel.org,
 x86@...nel.org, linux-coco@...ts.linux.dev, svsm-devel@...onut-svsm.dev
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 "H. Peter Anvin" <hpa@...or.com>, Andy Lutomirski <luto@...nel.org>,
 Peter Zijlstra <peterz@...radead.org>,
 Dan Williams <dan.j.williams@...el.com>, Michael Roth
 <michael.roth@....com>, Ashish Kalra <ashish.kalra@....com>
Subject: Re: [PATCH v3 02/14] x86/sev: Make the VMPL0 checking function more
 generic

On 3/25/2024 11:26 PM, Tom Lendacky wrote:
> Currently, the enforce_vmpl0() function uses a set argument when testing
> for VMPL0 and terminates the guest if the guest is not running at VMPL0.
> 
> Make the function more generic by moving it into the common code, renaming
> it, allowing it to take an argument for use in the VMPL0 check (RMPADJUST
> instruction) and return the result of the check, allowing the caller to
> determine the action taken based on the result.
> 
> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>

This is preparatory patch for patch3.

Reviewed-by: Pankaj Gupta <pankaj.gupta@....com>

> ---
>   arch/x86/boot/compressed/sev.c | 13 ++++++-------
>   1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index 5ad0ff4664f1..49dc9661176d 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -335,10 +335,9 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code)
>   		sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SEV_ES_GEN_REQ);
>   }
>   
> -static void enforce_vmpl0(void)
> +static bool running_at_vmpl0(void *va)
>   {
>   	u64 attrs;
> -	int err;
>   
>   	/*
>   	 * RMPADJUST modifies RMP permissions of a lesser-privileged (numerically
> @@ -347,12 +346,11 @@ static void enforce_vmpl0(void)
>   	 *
>   	 * If the guest is running at VMPL0, it will succeed. Even if that operation
>   	 * modifies permission bits, it is still ok to do so currently because Linux
> -	 * SNP guests are supported only on VMPL0 so VMPL1 or higher permission masks
> -	 * changing is a don't-care.
> +	 * SNP guests running at VMPL0 only run at VMPL0, so VMPL1 or higher
> +	 * permission mask changes are a don't-care.
>   	 */
>   	attrs = 1;
> -	if (rmpadjust((unsigned long)&boot_ghcb_page, RMP_PG_SIZE_4K, attrs))
> -		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_NOT_VMPL0);
> +	return !rmpadjust((unsigned long)va, RMP_PG_SIZE_4K, attrs);
>   }
>   
>   /*
> @@ -588,7 +586,8 @@ void sev_enable(struct boot_params *bp)
>   		if (!(get_hv_features() & GHCB_HV_FT_SNP))
>   			sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
>   
> -		enforce_vmpl0();
> +		if (!running_at_vmpl0(&boot_ghcb_page))
> +			sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_NOT_VMPL0);
>   	}
>   
>   	if (snp && !(sev_status & MSR_AMD64_SEV_SNP_ENABLED))