linux-kernel - Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early

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: <20251009185134.fb4evjrk76rwxv37@desk>
Date: Thu, 9 Oct 2025 11:51:34 -0700
From: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
To: Petr Tesarik <ptesarik@...e.com>
Cc: "H. Peter Anvin" <hpa@...or.com>, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
	"open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with
 early_param()

On Fri, Sep 26, 2025 at 08:01:02PM +0200, Petr Tesarik wrote:
> Use early_param() to get the value of the tsx= command line parameter.
> Although cmdline_find_option() works fine, the option is later reported
> as unknown and passed to user space. The latter is not a real issue, but
> the former is confusing and makes people wonder if the tsx= parameter had
> any effect and double-check for typos unnecessarily.
> 
> The behavior changes slightly if "tsx" is given without any argument (which
> is invalid syntax). Prior to this patch, the kernel logged an error message
> and disabled TSX. With this patch, the parameter is ignored. The new
> behavior is consistent with other parameters, e.g. "tsx_async_abort".
> 
> Signed-off-by: Petr Tesarik <ptesarik@...e.com>
> ---
>  arch/x86/kernel/cpu/tsx.c | 41 ++++++++++++++++++++++-----------------
>  1 file changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
> index 167dfd38b87a2..bb407331f64b5 100644
> --- a/arch/x86/kernel/cpu/tsx.c
> +++ b/arch/x86/kernel/cpu/tsx.c
> @@ -20,14 +20,14 @@
>  #define pr_fmt(fmt) "tsx: " fmt
>  
>  enum tsx_ctrl_states {
> +	TSC_CTRL_UNSPECIFIED,

s/TSC/TSX/

>  	TSX_CTRL_ENABLE,
>  	TSX_CTRL_DISABLE,
>  	TSX_CTRL_RTM_ALWAYS_ABORT,
>  	TSX_CTRL_NOT_SUPPORTED,
>  };
>  
> -static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init =
> -	TSX_CTRL_NOT_SUPPORTED;
> +static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init;
>  
>  static void tsx_disable(void)
>  {
> @@ -164,11 +164,28 @@ static void tsx_dev_mode_disable(void)
>  	}
>  }
>  
> -void __init tsx_init(void)
> +static int __init tsx_parse_cmdline(char *str)
>  {
> -	char arg[5] = {};
> -	int ret;
> +	if (!str)
> +		return -EINVAL;
> +
> +	if (!strcmp(str, "on")) {
> +		tsx_ctrl_state = TSX_CTRL_ENABLE;
> +	} else if (!strcmp(str, "off")) {
> +		tsx_ctrl_state = TSX_CTRL_DISABLE;
> +	} else if (!strcmp(str, "auto")) {
> +		tsx_ctrl_state = x86_get_tsx_auto_mode();

NACK, this introduces a subtle bug. With this change x86_get_tsx_auto_mode()
would return TSX_CTRL_ENABLE always, irrespective of whether the CPU has
X86_BUG_TAA or not. This is because early_param() is executed before
cpu_set_bug_bits().

> +	} else {
> +		tsx_ctrl_state = TSX_CTRL_DISABLE;
> +		pr_err("invalid option, defaulting to off\n");
> +	}
>  
> +	return 0;
> +}
> +early_param("tsx", tsx_parse_cmdline);

Rather, a patch to add a comment would be better.

---
diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
index 49782724a943..8fd1c16a38ec 100644
--- a/arch/x86/kernel/cpu/tsx.c
+++ b/arch/x86/kernel/cpu/tsx.c
@@ -194,6 +194,11 @@ void __init tsx_init(void)
 		return;
 	}
 
+	/*
+	 * Cmdline is not processed via early_param() to ensure
+	 * cpu_set_bug_bits() is executed already. And x86_get_tsx_auto_mode()
+	 * returns the appropriate default based on X86_BUG_TAA.
+	 */
 	ret = cmdline_find_option(boot_command_line, "tsx", arg, sizeof(arg));
 	if (ret >= 0) {
 		if (!strcmp(arg, "on")) {