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: <20250515235210.ooq7ogymcdvbtakd@desk>
Date: Thu, 15 May 2025 16:52:10 -0700
From: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
To: David Kaplan <david.kaplan@....com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Josh Poimboeuf <jpoimboe@...nel.org>,
	Ingo Molnar <mingo@...hat.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	"H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/bugs: Restructure ITS mitigation

On Thu, May 15, 2025 at 08:47:56AM -0500, David Kaplan wrote:
> Restructure the ITS mitigation to use select/update/apply functions like
> the other mitigations.
> 
> There is a particularly complex interaction between ITS and Retbleed as CDT
> (Call Depth Tracking) is a mitigation for both, and either its=stuff or
> retbleed=stuff will attempt to enable CDT.
> 
> retbleed_update_mitigation() runs first and will check the necessary
> pre-conditions for CDT if either ITS or Retbleed stuffing is selected.  If
> checks pass and ITS stuffing is selected, it will select stuffing for
> Retbleed as well.
> 
> its_update_mitigation() runs after and will either select stuffing if
> retbleed stuffing was enabled, or fall back to the default (aligned thunks)
> if stuffing could not be enabled.
> 
> Enablement of CDT is done exclusively in retbleed_apply_mitigation().
> its_apply_mitigation() is only used to enable aligned thunks.
> 
> Signed-off-by: David Kaplan <david.kaplan@....com>
> ---
>  arch/x86/kernel/cpu/bugs.c | 167 ++++++++++++++++++++-----------------
>  1 file changed, 90 insertions(+), 77 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index dd8b50b4ceaa..db26fb5a0a13 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -92,6 +92,8 @@ static void __init bhi_select_mitigation(void);
>  static void __init bhi_update_mitigation(void);
>  static void __init bhi_apply_mitigation(void);
>  static void __init its_select_mitigation(void);
> +static void __init its_update_mitigation(void);
> +static void __init its_apply_mitigation(void);
>  
>  /* The base value of the SPEC_CTRL MSR without task-specific bits set */
>  u64 x86_spec_ctrl_base;
> @@ -235,6 +237,11 @@ void __init cpu_select_mitigations(void)
>  	 * spectre_v2=ibrs.
>  	 */
>  	retbleed_update_mitigation();
> +	/*
> +	 * its_update_mitigation() depends on spectre_v2_update_mitigation()
> +	 * and retbleed_update_mitigation().
> +	 */
> +	its_update_mitigation();
>  
>  	/*
>  	 * spectre_v2_user_update_mitigation() depends on
> @@ -263,6 +270,7 @@ void __init cpu_select_mitigations(void)
>  	srbds_apply_mitigation();
>  	srso_apply_mitigation();
>  	gds_apply_mitigation();
> +	its_apply_mitigation();
>  	bhi_apply_mitigation();
>  }
>  
> @@ -1125,6 +1133,14 @@ enum retbleed_mitigation {
>  	RETBLEED_MITIGATION_STUFF,
>  };
>  
> +enum its_mitigation {
> +	ITS_MITIGATION_OFF,
> +	ITS_MITIGATION_AUTO,
> +	ITS_MITIGATION_VMEXIT_ONLY,
> +	ITS_MITIGATION_ALIGNED_THUNKS,
> +	ITS_MITIGATION_RETPOLINE_STUFF,
> +};

This is in between retbleed declarations, I would suggest to move this
before retbleed mitigation starts.

enum its_mitigation {
        ITS_MITIGATION_OFF,
        ITS_MITIGATION_AUTO,
        ITS_MITIGATION_VMEXIT_ONLY,
        ITS_MITIGATION_ALIGNED_THUNKS,
        ITS_MITIGATION_RETPOLINE_STUFF,
};

static enum its_mitigation its_mitigation __ro_after_init =
        IS_ENABLED(CONFIG_MITIGATION_ITS) ? ITS_MITIGATION_AUTO : ITS_MITIGATION_OFF;

#undef pr_fmt
#define pr_fmt(fmt)     "RETBleed: " fmt

enum retbleed_mitigation {

>  static const char * const retbleed_strings[] = {
>  	[RETBLEED_MITIGATION_NONE]	= "Vulnerable",
>  	[RETBLEED_MITIGATION_UNRET]	= "Mitigation: untrained return thunk",
> @@ -1137,6 +1153,9 @@ static const char * const retbleed_strings[] = {
>  static enum retbleed_mitigation retbleed_mitigation __ro_after_init =
>  	IS_ENABLED(CONFIG_MITIGATION_RETBLEED) ? RETBLEED_MITIGATION_AUTO : RETBLEED_MITIGATION_NONE;
>  
> +static enum its_mitigation its_mitigation __ro_after_init =
> +	IS_ENABLED(CONFIG_MITIGATION_ITS) ? ITS_MITIGATION_AUTO : ITS_MITIGATION_OFF;

Ditto.

>  static int __ro_after_init retbleed_nosmt = false;
>  
>  static int __init retbleed_parse_cmdline(char *str)
> @@ -1242,11 +1261,19 @@ static void __init retbleed_update_mitigation(void)
>  	/*
>  	 * retbleed=stuff is only allowed on Intel.  If stuffing can't be used
>  	 * then a different mitigation will be selected below.
> +	 *
> +	 * its=stuff will also attempt to enable stuffing.
>  	 */
> -	if (retbleed_mitigation == RETBLEED_MITIGATION_STUFF) {
> +	if (retbleed_mitigation == RETBLEED_MITIGATION_STUFF ||
> +	    its_mitigation == ITS_MITIGATION_RETPOLINE_STUFF) {
>  		if (spectre_v2_enabled != SPECTRE_V2_RETPOLINE) {

SPECTRE_V2_EIBRS_RETPOLINE also enables retpoline.

>  			pr_err("WARNING: retbleed=stuff depends on spectre_v2=retpoline\n");

This can be updated to:

  			pr_err("WARNING: retbleed=stuff depends on retpoline\n");

>  			retbleed_mitigation = RETBLEED_MITIGATION_AUTO;
> +		} else {
> +			if (retbleed_mitigation != RETBLEED_MITIGATION_STUFF)
> +				pr_info("Retbleed mitigation updated to stuffing\n");
> +
> +			retbleed_mitigation = RETBLEED_MITIGATION_STUFF;
>  		}
>  	}
>  	/*
> @@ -1338,20 +1365,6 @@ static void __init retbleed_apply_mitigation(void)
>  #undef pr_fmt
>  #define pr_fmt(fmt)     "ITS: " fmt
>  
> -enum its_mitigation_cmd {
> -	ITS_CMD_OFF,
> -	ITS_CMD_ON,
> -	ITS_CMD_VMEXIT,
> -	ITS_CMD_RSB_STUFF,
> -};
> -
> -enum its_mitigation {
> -	ITS_MITIGATION_OFF,
> -	ITS_MITIGATION_VMEXIT_ONLY,
> -	ITS_MITIGATION_ALIGNED_THUNKS,
> -	ITS_MITIGATION_RETPOLINE_STUFF,
> -};
> -
>  static const char * const its_strings[] = {
>  	[ITS_MITIGATION_OFF]			= "Vulnerable",
>  	[ITS_MITIGATION_VMEXIT_ONLY]		= "Mitigation: Vulnerable, KVM: Not affected",
> @@ -1359,11 +1372,6 @@ static const char * const its_strings[] = {
>  	[ITS_MITIGATION_RETPOLINE_STUFF]	= "Mitigation: Retpolines, Stuffing RSB",
>  };
>  
> -static enum its_mitigation its_mitigation __ro_after_init = ITS_MITIGATION_ALIGNED_THUNKS;
> -
> -static enum its_mitigation_cmd its_cmd __ro_after_init =
> -	IS_ENABLED(CONFIG_MITIGATION_ITS) ? ITS_CMD_ON : ITS_CMD_OFF;
> -
>  static int __init its_parse_cmdline(char *str)
>  {
>  	if (!str)
> @@ -1375,16 +1383,16 @@ static int __init its_parse_cmdline(char *str)
>  	}
>  
>  	if (!strcmp(str, "off")) {
> -		its_cmd = ITS_CMD_OFF;
> +		its_mitigation = ITS_MITIGATION_OFF;
>  	} else if (!strcmp(str, "on")) {
> -		its_cmd = ITS_CMD_ON;
> +		its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
>  	} else if (!strcmp(str, "force")) {
> -		its_cmd = ITS_CMD_ON;
> +		its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
>  		setup_force_cpu_bug(X86_BUG_ITS);
>  	} else if (!strcmp(str, "vmexit")) {
> -		its_cmd = ITS_CMD_VMEXIT;
> +		its_mitigation = ITS_MITIGATION_VMEXIT_ONLY;
>  	} else if (!strcmp(str, "stuff")) {
> -		its_cmd = ITS_CMD_RSB_STUFF;
> +		its_mitigation = ITS_MITIGATION_RETPOLINE_STUFF;
>  	} else {
>  		pr_err("Ignoring unknown indirect_target_selection option (%s).", str);
>  	}
> @@ -1395,85 +1403,90 @@ early_param("indirect_target_selection", its_parse_cmdline);
>  
>  static void __init its_select_mitigation(void)
>  {
> -	enum its_mitigation_cmd cmd = its_cmd;
> -
>  	if (!boot_cpu_has_bug(X86_BUG_ITS) || cpu_mitigations_off()) {
>  		its_mitigation = ITS_MITIGATION_OFF;
>  		return;
>  	}
>  
> -	/* Retpoline+CDT mitigates ITS, bail out */
> -	if (boot_cpu_has(X86_FEATURE_RETPOLINE) &&
> -	    boot_cpu_has(X86_FEATURE_CALL_DEPTH)) {
> -		its_mitigation = ITS_MITIGATION_RETPOLINE_STUFF;
> -		goto out;
> -	}
> +	if (its_mitigation == ITS_MITIGATION_AUTO)
> +		its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
> +
> +	if (its_mitigation == ITS_MITIGATION_OFF)
> +		return;
>  
> -	/* Exit early to avoid irrelevant warnings */
> -	if (cmd == ITS_CMD_OFF) {
> -		its_mitigation = ITS_MITIGATION_OFF;
> -		goto out;
> -	}
> -	if (spectre_v2_enabled == SPECTRE_V2_NONE) {
> -		pr_err("WARNING: Spectre-v2 mitigation is off, disabling ITS\n");
> -		its_mitigation = ITS_MITIGATION_OFF;
> -		goto out;
> -	}
>  	if (!IS_ENABLED(CONFIG_MITIGATION_RETPOLINE) ||
>  	    !IS_ENABLED(CONFIG_MITIGATION_RETHUNK)) {
>  		pr_err("WARNING: ITS mitigation depends on retpoline and rethunk support\n");
>  		its_mitigation = ITS_MITIGATION_OFF;
> -		goto out;
> +		return;
>  	}
> +
>  	if (IS_ENABLED(CONFIG_DEBUG_FORCE_FUNCTION_ALIGN_64B)) {
>  		pr_err("WARNING: ITS mitigation is not compatible with CONFIG_DEBUG_FORCE_FUNCTION_ALIGN_64B\n");
>  		its_mitigation = ITS_MITIGATION_OFF;
> -		goto out;
> -	}
> -	if (boot_cpu_has(X86_FEATURE_RETPOLINE_LFENCE)) {
> -		pr_err("WARNING: ITS mitigation is not compatible with lfence mitigation\n");
> -		its_mitigation = ITS_MITIGATION_OFF;
> -		goto out;
> +		return;
>  	}
>  
> -	if (cmd == ITS_CMD_RSB_STUFF &&
> -	    (!boot_cpu_has(X86_FEATURE_RETPOLINE) || !IS_ENABLED(CONFIG_MITIGATION_CALL_DEPTH_TRACKING))) {
> +	if (its_mitigation == ITS_MITIGATION_RETPOLINE_STUFF &&
> +	    !IS_ENABLED(CONFIG_MITIGATION_CALL_DEPTH_TRACKING)) {
>  		pr_err("RSB stuff mitigation not supported, using default\n");
> -		cmd = ITS_CMD_ON;
> +		its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;

This and ...

>  	}
>  
> -	switch (cmd) {
> -	case ITS_CMD_OFF:
> +	if (its_mitigation == ITS_MITIGATION_VMEXIT_ONLY &&
> +	    !boot_cpu_has_bug(X86_BUG_ITS_NATIVE_ONLY))
> +		its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;

... this are essentially resetting the mitigation to default. This will be
more clear if you could change the mitigation to ITS_MITIGATION_AUTO here,
and at the end have:

	if (its_mitigation == ITS_MITIGATION_AUTO)
		its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
> +
> +}
> +
> +static void __init its_update_mitigation(void)
> +{
> +	if (!boot_cpu_has_bug(X86_BUG_ITS) || cpu_mitigations_off())
> +		return;
> +
> +	switch (spectre_v2_enabled) {
> +	case SPECTRE_V2_NONE:
> +		pr_err("WARNING: Spectre-v2 mitigation is off, disabling ITS\n");
>  		its_mitigation = ITS_MITIGATION_OFF;
>  		break;
> -	case ITS_CMD_VMEXIT:
> -		if (boot_cpu_has_bug(X86_BUG_ITS_NATIVE_ONLY)) {
> -			its_mitigation = ITS_MITIGATION_VMEXIT_ONLY;
> -			goto out;
> -		}
> -		fallthrough;
> -	case ITS_CMD_ON:
> -		its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
> -		if (!boot_cpu_has(X86_FEATURE_RETPOLINE))
> -			setup_force_cpu_cap(X86_FEATURE_INDIRECT_THUNK_ITS);
> -		setup_force_cpu_cap(X86_FEATURE_RETHUNK);
> -		set_return_thunk(its_return_thunk);
> +	case SPECTRE_V2_RETPOLINE:

Also SPECTRE_V2_EIBRS_RETPOLINE.

> +		/* Retpoline+CDT mitigates ITS */
> +		if (retbleed_mitigation == RETBLEED_MITIGATION_STUFF)



> +			its_mitigation = ITS_MITIGATION_RETPOLINE_STUFF;
>  		break;
> -	case ITS_CMD_RSB_STUFF:
> -		its_mitigation = ITS_MITIGATION_RETPOLINE_STUFF;
> -		setup_force_cpu_cap(X86_FEATURE_RETHUNK);
> -		setup_force_cpu_cap(X86_FEATURE_CALL_DEPTH);
> -		set_return_thunk(call_depth_return_thunk);
> -		if (retbleed_mitigation == RETBLEED_MITIGATION_NONE) {
> -			retbleed_mitigation = RETBLEED_MITIGATION_STUFF;
> -			pr_info("Retbleed mitigation updated to stuffing\n");
> -		}
> +	case SPECTRE_V2_LFENCE:
> +	case SPECTRE_V2_EIBRS_LFENCE:
> +		pr_err("WARNING: ITS mitigation is not compatible with lfence mitigation\n");
> +		its_mitigation = ITS_MITIGATION_OFF;
> +		break;
> +	default:
>  		break;
>  	}
> -out:
> +
> +	/*
> +	 * retbleed_update_mitigation() will try to do stuffing if its=stuff.
> +	 * If it can't, such as if spectre_v2!=retpoline, then fall back to
> +	 * aligned thunks.
> +	 */
> +	if (its_mitigation == ITS_MITIGATION_RETPOLINE_STUFF &&
> +	    retbleed_mitigation != RETBLEED_MITIGATION_STUFF)
> +		its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;

The =stuff mitigation depends on retpoline, not really on retbleed.
Shouldn't this be handled in the switch (spectre_v2_enabled) above?

>  	pr_info("%s\n", its_strings[its_mitigation]);
>  }
>  
> +static void __init its_apply_mitigation(void)
> +{
> +	/* its=stuff forces retbleed stuffing and is enabled there. */

Oh, this is why you are depending on retbleed_mitigation above, this part is
a bit confusing.

Will think about it more later, trying to have a couple of days off.

> +	if (its_mitigation != ITS_MITIGATION_ALIGNED_THUNKS)
> +		return;
> +
> +	if (!boot_cpu_has(X86_FEATURE_RETPOLINE))
> +		setup_force_cpu_cap(X86_FEATURE_INDIRECT_THUNK_ITS);
> +
> +	setup_force_cpu_cap(X86_FEATURE_RETHUNK);
> +	set_return_thunk(its_return_thunk);
> +}

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ