Date:   Thu, 2 Feb 2023 13:44:42 -0800
From:   Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
To:     Breno Leitao <leitao@debian.org>
Cc:     tglx@linutronix.de, bp@alien8.de, paul@paul-moore.com,
        leit@meta.com, x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] cpu/bugs: Disable CPU mitigations at compilation time
Message-ID: <20230202214442.ffqwk2can6b5ivpz@desk>
References: <20230202180858.1539234-1-leitao@debian.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20230202180858.1539234-1-leitao@debian.org>
Precedence: bulk

On Thu, Feb 02, 2023 at 10:08:58AM -0800, Breno Leitao wrote:
> Right now it is not possible to disable CPU vulnerabilities mitigations
> at build time. Mitigation needs to be disabled passing kernel
> parameters, such as 'mitigations=off'.
> 
> This patch creates an easy way to disable mitigation during compilation

s/This patch creates/Create/

  Describe your changes in imperative mood, e.g. “make xyzzy do frotz”
  instead of “[This patch] makes xyzzy do frotz” or “[I] changed xyzzy to
  do frotz”, as if you are giving orders to the codebase to change its
  behaviour.

  https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes

> time (CONFIG_DEFAULT_CPU_MITIGATIONS_OFF), so, insecure kernel users don't
> need to deal with kernel parameters when booting insecure kernels.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  kernel/cpu.c     |  5 +++++
>  security/Kconfig | 11 +++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 6c0a92ca6bb5..497e9a3d3d77 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -2727,8 +2727,13 @@ enum cpu_mitigations {
>  	CPU_MITIGATIONS_AUTO_NOSMT,
>  };
>  
> +#ifdef CONFIG_DEFAULT_CPU_MITIGATIONS_OFF
> +static enum cpu_mitigations cpu_mitigations __ro_after_init =
> +	CPU_MITIGATIONS_OFF;
> +#else
>  static enum cpu_mitigations cpu_mitigations __ro_after_init =
>  	CPU_MITIGATIONS_AUTO;
> +#endif

With the 80 char limit gone, I think below is more readable:

#ifdef CONFIG_DEFAULT_CPU_MITIGATIONS_OFF
static enum cpu_mitigations cpu_mitigations __ro_after_init = CPU_MITIGATIONS_OFF;
#else
static enum cpu_mitigations cpu_mitigations __ro_after_init = CPU_MITIGATIONS_AUTO;
#endif

>  static int __init mitigations_parse_cmdline(char *arg)
>  {
> diff --git a/security/Kconfig b/security/Kconfig
> index e6db09a779b7..a70427dc6ace 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -258,6 +258,17 @@ config LSM
>  
>  	  If unsure, leave this as the default.
>  
> +config CONFIG_DEFAULT_CPU_MITIGATIONS_OFF

CONFIG_ is implicit, this needs to be:

config DEFAULT_CPU_MITIGATIONS_OFF

> +	bool "Disable mitigations for CPU vulnerabilities by default"
> +	default n
> +	help
> +	  This option disable mitigations for CPU vulnerabilities by default.

s/disable/disables/

> +	  This improves system performance, but it may also expose users
> +	  to several CPU vulnerabilities.
> +	  This has the same effect as passing `mitigations=off` kernel
> +	  parameter. The mitigations could be enabled back passing the
> +	  'mitigations' parameter.
> +
>  source "security/Kconfig.hardening"
>  
>  endmenu
> -- 
> 2.30.2
>