[PATCH v2 0/1] x86/pti: Fix kernel warnings for pti= and nopti cmdline options.

[PATCH v2 0/1] x86/pti: Fix kernel warnings for pti= and nopti cmdline options.

[Jo Van Bulck]

Hi,

This is the third iteration of a patch to improve the cmdline option parsing
for PTI. This reverts largely back to the first iteration and cleans up the
code to remove any attempts at backwards compatible behavior for clearly
conflicting options when users erroneously combine pti= nopti and
mitigations=off as per Dave's suggestions [1].

[1] https://lore.kernel.org/all/b9bbb279-fa8f-0784-900f-114ce186cbb3@intel.com/

Behavior
--------

For reference, behavior with this patch is now as follows in case of any
conflicting options.

1. Latest in order of nopti pti= takes priority:

KERNEL_CMDLINE="nopti pti=on"
[    0.021779] Kernel/User page tables isolation: enabled
Mitigation: PTI

KERNEL_CMDLINE="pti=on nopti"
[    0.010289] Kernel/User page tables isolation: disabled on command line.
Vulnerable

2. Passing mitigations=off will unconditionally disable PTI:

KERNEL_CMDLINE="mitigations=off pti=on"
[    0.008331] Kernel/User page tables isolation: disabled on command line.
Vulnerable

KERNEL_CMDLINE="pti=on mitigations=off"
[    0.008495] Kernel/User page tables isolation: disabled on command line.
Vulnerable

Changelog
---------

v3
  - Revert backwards compatibility ugliness for conflicting options (Dave)

v2
  - Split pti=off and mitigations=off checks (Sohil)
  - Ensure backwards compatibility for conflicting options (Sohil)

Best,
Jo

Jo Van Bulck (1):
  x86/pti: Fix kernel warnings for pti= and nopti cmdline options.

 arch/x86/mm/pti.c | 55 ++++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 27 deletions(-)

-- 
2.25.1

[PATCH 1/1] x86/pti: Fix kernel warnings for pti= and nopti cmdline options.

[Jo Van Bulck]

Parse the pti= and nopti cmdline options using early_param to fix 'Unknown
kernel command line parameters "nopti", will be passed to user space'
warnings in the kernel log when nopti or pti= are passed to the kernel
cmdline on x86 platforms. Additionally allow the kernel to warn for
malformed pti= options.

Link: https://lore.kernel.org/all/b9bbb279-fa8f-0784-900f-114ce186cbb3@intel.com/
Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
---
 arch/x86/mm/pti.c | 55 ++++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
index 78414c6d1..7575e224d 100644
--- a/arch/x86/mm/pti.c
+++ b/arch/x86/mm/pti.c
@@ -69,6 +69,7 @@ static void __init pti_print_if_secure(const char *reason)
 		pr_info("%s\n", reason);
 }
 
+/* Assume mode is auto unless overridden via cmdline below. */
 static enum pti_mode {
 	PTI_AUTO = 0,
 	PTI_FORCE_OFF,
@@ -77,50 +78,50 @@ static enum pti_mode {
 
 void __init pti_check_boottime_disable(void)
 {
-	char arg[5];
-	int ret;
-
-	/* Assume mode is auto unless overridden. */
-	pti_mode = PTI_AUTO;
-
 	if (hypervisor_is_type(X86_HYPER_XEN_PV)) {
 		pti_mode = PTI_FORCE_OFF;
 		pti_print_if_insecure("disabled on XEN PV.");
 		return;
 	}
 
-	ret = cmdline_find_option(boot_command_line, "pti", arg, sizeof(arg));
-	if (ret > 0)  {
-		if (ret == 3 && !strncmp(arg, "off", 3)) {
-			pti_mode = PTI_FORCE_OFF;
-			pti_print_if_insecure("disabled on command line.");
-			return;
-		}
-		if (ret == 2 && !strncmp(arg, "on", 2)) {
-			pti_mode = PTI_FORCE_ON;
-			pti_print_if_secure("force enabled on command line.");
-			goto enable;
-		}
-		if (ret == 4 && !strncmp(arg, "auto", 4)) {
-			pti_mode = PTI_AUTO;
-			goto autosel;
-		}
-	}
-
-	if (cmdline_find_option_bool(boot_command_line, "nopti") ||
-	    cpu_mitigations_off()) {
+	if (cpu_mitigations_off())
 		pti_mode = PTI_FORCE_OFF;
+	if (pti_mode == PTI_FORCE_OFF) {
 		pti_print_if_insecure("disabled on command line.");
 		return;
 	}
+	if (pti_mode == PTI_FORCE_ON) {
+		pti_print_if_secure("force enabled on command line.");
+		goto enable;
+	}
 
-autosel:
 	if (!boot_cpu_has_bug(X86_BUG_CPU_MELTDOWN))
 		return;
 enable:
 	setup_force_cpu_cap(X86_FEATURE_PTI);
 }
 
+static int __init pti_parse_cmdline(char *arg)
+{
+	if (!strcmp(arg, "off"))
+		pti_mode = PTI_FORCE_OFF;
+	else if (!strcmp(arg, "on"))
+		pti_mode = PTI_FORCE_ON;
+	else if (!strcmp(arg, "auto"))
+		pti_mode = PTI_AUTO;
+	else
+		return -EINVAL;
+	return 0;
+}
+early_param("pti", pti_parse_cmdline);
+
+static int __init pti_parse_cmdline_nopti(char *arg)
+{
+	pti_mode = PTI_FORCE_OFF;
+	return 0;
+}
+early_param("nopti", pti_parse_cmdline_nopti);
+
 pgd_t __pti_set_user_pgtbl(pgd_t *pgdp, pgd_t pgd)
 {
 	/*
-- 
2.25.1

Re: [PATCH 1/1] x86/pti: Fix kernel warnings for pti= and nopti cmdline options.

[Sohil Mehta]

On 8/12/2023 8:54 AM, Jo Van Bulck wrote:
>  arch/x86/mm/pti.c | 55 ++++++++++++++++++++++++-----------------------
>  1 file changed, 28 insertions(+), 27 deletions(-)
> 

This version is very similar to the original patch and much simpler.
Sorry about the unnecessary churn.

Apart from the minor nits below,
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>


> +	if (cpu_mitigations_off())
>  		pti_mode = PTI_FORCE_OFF;
> +	if (pti_mode == PTI_FORCE_OFF) {
>  		pti_print_if_insecure("disabled on command line.");
>  		return;
>  	}

A new line here would be useful.

> +	if (pti_mode == PTI_FORCE_ON) {
> +		pti_print_if_secure("force enabled on command line.");
> +		goto enable;
> +	}
>  
> -autosel:
>  	if (!boot_cpu_has_bug(X86_BUG_CPU_MELTDOWN))
>  		return;
>  enable:
>  	setup_force_cpu_cap(X86_FEATURE_PTI);
>  }
>  

Was there an issue with the flow you had in the original patch? It was
avoiding the goto label and flow was a bit more linear.

> if (pti_mode == PTI_AUTO && !boot_cpu_has_bug(X86_BUG_CPU_MELTDOWN))
>  		return;
> 
> if (pti_mode == PTI_FORCE_ON)
> 	pti_print_if_secure("force enabled on command line.");
> 
> setup_force_cpu_cap(X86_FEATURE_PTI);

Sohil

Re: [PATCH 1/1] x86/pti: Fix kernel warnings for pti= and nopti cmdline options.

[Jo Van Bulck]

On 14.08.23 14:12, Sohil Mehta wrote:
> On 8/12/2023 8:54 AM, Jo Van Bulck wrote:
>>   arch/x86/mm/pti.c | 55 ++++++++++++++++++++++++-----------------------
>>   1 file changed, 28 insertions(+), 27 deletions(-)
>>
> 
> This version is very similar to the original patch and much simpler.
> Sorry about the unnecessary churn.
> 
> Apart from the minor nits below,
> Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>

No problem, thanks for the help!

> 
>> +	if (cpu_mitigations_off())
>>   		pti_mode = PTI_FORCE_OFF;
>> +	if (pti_mode == PTI_FORCE_OFF) {
>>   		pti_print_if_insecure("disabled on command line.");
>>   		return;
>>   	}
> 
> A new line here would be useful.

Added in next revision.

> Was there an issue with the flow you had in the original patch? It was
> avoiding the goto label and flow was a bit more linear.

No, the original flow also works and I agree that an explicit PTI_AUTO 
check may indeed be preferable. Reverting this in the next patch iteration.

Best,
Jo

Re: [PATCH v2 0/1] x86/pti: Fix kernel warnings for pti= and nopti cmdline options.

[Sohil Mehta]

Hi Jo,

On 8/12/2023 8:54 AM, Jo Van Bulck wrote:
> Hi,
> 
> This is the third iteration of a patch to improve the cmdline option parsing
> for PTI.

You missed updating the version number in the cover letter subject.
Also, it is useful to have the version number in the individual patch
subject as well. (Just something to keep in mind for future patches.)

Usually git takes care of it automatically, if you do this:

	git format-patch --cover-letter -v3 -1 -o patches/

Sohil

Re: [PATCH v2 0/1] x86/pti: Fix kernel warnings for pti= and nopti cmdline options.

[Jo Van Bulck]

On 14.08.23 13:43, Sohil Mehta wrote:
> You missed updating the version number in the cover letter subject.
> Also, it is useful to have the version number in the individual patch
> subject as well. (Just something to keep in mind for future patches.)
> 
> Usually git takes care of it automatically, if you do this:
> 
> 	git format-patch --cover-letter -v3 -1 -o patches/

Thank you for catching this and pointing out the git option, this is 
indeed very helpful to keep in mind!

Best,
Jo