linux-kernel - Re: [PATCH 15/14] x86/gds: Lock GDS mitigation when keylocker feature is present

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: <142380d5-27bd-4ddc-8c33-03a2f8701fa2@intel.com>
Date: Mon, 22 Apr 2024 00:35:45 -0700
From: "Chang S. Bae" <chang.seok.bae@...el.com>
To: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
	<linux-kernel@...r.kernel.org>
CC: <x86@...nel.org>, <dan.j.williams@...el.com>, <bernie.keany@...el.com>,
	<charishma1.gairuboyina@...el.com>, Josh Poimboeuf <jpoimboe@...nel.org>,
	<daniel.sneddon@...ux.intel.com>, <antonio.gomez.iglesias@...ux.intel.com>
Subject: Re: [PATCH 15/14] x86/gds: Lock GDS mitigation when keylocker feature
 is present

On 4/19/2024 10:47 AM, Pawan Gupta wrote:
>   
>   	/*
> @@ -840,6 +843,11 @@ static void __init gds_select_mitigation(void)
>   		gds_mitigation = GDS_MITIGATION_FULL_LOCKED;
>   	}
>   
> +	/* Keylocker can only be enabled when GDS mitigation is locked */
> +	if (boot_cpu_has(X86_FEATURE_KEYLOCKER) &&
> +	    gds_mitigation == GDS_MITIGATION_FULL)
> +		gds_mitigation = GDS_MITIGATION_FULL_LOCKED;
> +

I'm having trouble understanding this change:

gds_select_mitigation()
{
	...
	if (gds_mitigation == GDS_MITIGATION_FORCE)
		gds_mitigation = GDS_MITIGATION_FULL;

	rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl);
	if (mcu_ctrl & GDS_MITG_LOCKED) {
		...
		gds_mitigation = GDS_MITIGATION_FULL_LOCKED;
	}

	if (boot_cpu_has(X86_FEATURE_KEYLOCKER) &&
	    gds_mitigation == GDS_MITIGATION_FULL)
		gds_mitigation = GDS_MITIGATION_FULL_LOCKED;

As I understand it, gds_mitigation is set to GDS_MITIGATION_FULL only if 
the gds force option is enabled but IA32_MCU_OPT_CTRL[GDS_MITG_LOCK] is 
not set.

Then, if the CPU has Key Locker, this code sets gds_mitigation to 
GDS_MITIGATION_FULL_LOCKED, which seems contradictory. I'm not sure why 
this change is necessary.

I'm also not convinced that the Key Locker series needs to modify this 
function. The Key Locker setup code should simply check the current 
mitigation status and enable the feature only if proper mitigation is in 
place. Am I missing something here?

Thanks,
Chang