linux-kernel - Re: [PATCH v16 10/13] x86/tsc: Switch Secure TSC guests away from kvm-clock

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: <7851bbe9-82f7-4487-8097-d373bba1a18e@amd.com>
Date: Wed, 8 Jan 2025 16:15:10 +0530
From: "Nikunj A. Dadhania" <nikunj@....com>
To: Borislav Petkov <bp@...en8.de>
Cc: linux-kernel@...r.kernel.org, thomas.lendacky@....com, x86@...nel.org,
 kvm@...r.kernel.org, mingo@...hat.com, tglx@...utronix.de,
 dave.hansen@...ux.intel.com, pgonda@...gle.com, seanjc@...gle.com,
 pbonzini@...hat.com, francescolavra.fl@...il.com
Subject: Re: [PATCH v16 10/13] x86/tsc: Switch Secure TSC guests away from
 kvm-clock



On 1/7/2025 8:46 PM, Borislav Petkov wrote:
> On Mon, Jan 06, 2025 at 06:16:30PM +0530, Nikunj A Dadhania wrote:
>>  static int kvm_cs_enable(struct clocksource *cs)
>>  {
>> +	/*
>> +	 * TSC clocksource should be used for a guest with Secure TSC enabled,
>> +	 * taint the kernel and warn when the user changes the clocksource to
>> +	 * kvm-clock.
>> +	 */
>> +	if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) {
>> +		add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
>> +		WARN_ONCE(1, "For Secure TSC guest, changing the clocksource is not allowed!\n");
> 
> So this thing is trying to state that changing the clocksource is not allowed
> but it still allows it. Why not simply do this:
> 
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index 960260a8d884..d8fef3a65a35 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -151,14 +151,10 @@ bool kvm_check_and_clear_guest_paused(void)
>  
>  static int kvm_cs_enable(struct clocksource *cs)
>  {
> -	/*
> -	 * TSC clocksource should be used for a guest with Secure TSC enabled,
> -	 * taint the kernel and warn when the user changes the clocksource to
> -	 * kvm-clock.
> -	 */
> +	/* Only the TSC should be used in a Secure TSC guest. */
>  	if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) {
> -		add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
> -		WARN_ONCE(1, "For Secure TSC guest, changing the clocksource is not allowed!\n");
> +		WARN_ONCE(1, "Secure TSC guest, changing the clocksource is not allowed!\n");
> +		return 1;
>  	}
>  
>  	vclocks_set_used(VDSO_CLOCKMODE_PVCLOCK);
> 
> ?

Works as expected:

$ echo 'kvm-clock' > /sys/devices/system/clocksource/clocksource0/current_clocksource
[   30.333603] ------------[ cut here ]------------
[   30.334802] Secure TSC guest, changing the clocksource is not allowed!
[   30.336460] WARNING: CPU: 0 PID: 19 at arch/x86/kernel/kvmclock.c:156 kvm_cs_enable+0x57/0x70

Regards
Nikunj