arch/x86/mm/pti.c | 56 +++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 29 deletions(-)
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.
Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
---
Resending this as I haven't heard back yet. I'd be happy to incorporate any
feedback.
Also adding test output before/after patch for reference:
dmesg | grep -e "page tables isolation" -e "Command line" \
-e "Malformed" -e "Unknown kernel command line parameters" \
&& cat /sys/devices/system/cpu/vulnerabilities/meltdown
Before patch
============
KERNEL_CMDLINE="nopti"
[ 0.000000] Command line: root=/dev/vda console=ttyS0 nopti
[ 0.009875] Kernel/User page tables isolation: disabled on command line.
[ 0.021498] Unknown kernel command line parameters "nopti", will be passed to user space.
Vulnerable
KERNEL_CMDLINE="pti=off"
[ 0.000000] Command line: root=/dev/vda console=ttyS0 pti=off
[ 0.009564] Kernel/User page tables isolation: disabled on command line.
[ 0.019542] Unknown kernel command line parameters "pti=off", will be passed to user space.
Vulnerable
KERNEL_CMDLINE="pti=invalid"
[ 0.000000] Command line: root=/dev/vda console=ttyS0 pti=invalid
[ 0.021409] Unknown kernel command line parameters "pti=invalid", will be passed to user space.
[ 0.022411] Kernel/User page tables isolation: enabled
Mitigation: PTI
After patch
===========
KERNEL_CMDLINE="nopti"
[ 0.000000] Command line: root=/dev/vda console=ttyS0 nopti
[ 0.009775] Kernel/User page tables isolation: disabled on command line.
Vulnerable
KERNEL_CMDLINE="pti=off"
[ 0.000000] Command line: root=/dev/vda console=ttyS0 pti=off
[ 0.009879] Kernel/User page tables isolation: disabled on command line.
Vulnerable
KERNEL_CMDLINE="pti=invalid"
[ 0.000000] Command line: root=/dev/vda console=ttyS0 pti=invalid
[ 0.000000] Malformed early option 'pti'
[ 0.020892] Kernel/User page tables isolation: enabled
Mitigation: PTI
arch/x86/mm/pti.c | 56 +++++++++++++++++++++++------------------------
1 file changed, 27 insertions(+), 29 deletions(-)
diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
index 78414c6d1..ea5841cf9 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,47 @@ 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 (pti_mode == PTI_FORCE_OFF || cpu_mitigations_off()) {
pti_mode = PTI_FORCE_OFF;
pti_print_if_insecure("disabled on command line.");
return;
}
-autosel:
- if (!boot_cpu_has_bug(X86_BUG_CPU_MELTDOWN))
+ if (pti_mode == PTI_AUTO && !boot_cpu_has_bug(X86_BUG_CPU_MELTDOWN))
return;
-enable:
+
+ if (pti_mode == PTI_FORCE_ON)
+ pti_print_if_secure("force enabled on command line.");
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)
{
/*
base-commit: 1399419a8db7b3d6083b47062358d95dc8ec9663
--
2.25.1On 8/8/2023 12:56 PM, Jo Van Bulck wrote:
> -
> - if (cmdline_find_option_bool(boot_command_line, "nopti") ||
> - cpu_mitigations_off()) {
> + if (pti_mode == PTI_FORCE_OFF || cpu_mitigations_off()) {
Can mitigations be off through some other mechanisms such as kernel config?
Maybe split the mitigations_off check into a separate if and it's own
unique print message?
The existing code might have the same issue as well.
Also, with the separated check you can avoid the unnecessary re-setting
of pti_mode when pti_mode == PTI_FORCE_OFF is true.
> pti_mode = PTI_FORCE_OFF;> pti_print_if_insecure("disabled on command line.");
> return;
> }
>
> -autosel:
> - if (!boot_cpu_has_bug(X86_BUG_CPU_MELTDOWN))
> + if (pti_mode == PTI_AUTO && !boot_cpu_has_bug(X86_BUG_CPU_MELTDOWN))
> return;
> -enable:
> +
> + if (pti_mode == PTI_FORCE_ON)
> + pti_print_if_secure("force enabled on command line.");
> 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);
> +
In the rare case that both pti= and nopti is set the existing code seems
to ignore the nopti option. Would the new implementation do the same?
Sohil
Thank you for the code review! On 08.08.23 17:13, Sohil Mehta wrote:> Can mitigations be off through some other mechanisms such as kernel config? Yes, from the kernel documentation [1]: "It can be enabled by setting CONFIG_PAGE_TABLE_ISOLATION=y at compile time. Once enabled at compile-time, it can be disabled at boot with the 'nopti' or 'pti=' kernel parameters" In my understanding, if PTI is disabled at compile-time the full pti.c file is excluded and this code is never executed. I validated that, when compiling with CONFIG_PAGE_TABLE_ISOLATION=n, any nopti/pti= parameters are reported as unknown and /sys/devices/system/cpu/vulnerabilities/meltdown is reported as vulnerable. I validated this both with and without the proposed patch. > Maybe split the mitigations_off check into a separate if and it's own > unique print message? > Also, with the separated check you can avoid the unnecessary re-setting > of pti_mode when pti_mode == PTI_FORCE_OFF is true. Thanks, makes sense. I'll make sure to do this in the next patch revision. > In the rare case that both pti= and nopti is set the existing code seems > to ignore the nopti option. Would the new implementation do the same? Good point. In my understanding, passing such conflicting options is undefined as per the specification [2] and I'm not sure if backwards compatibility is a requirement? That being said, I can see the argument that in this case of security-sensitive functionality, it may be desirable to maintain identical behavior for identical kernel parameter combinations and sequences. The current patch does indeed _not_ guarantee this. Particularly, I found there are currently 2 divergent cases: CASE 1: PTI= > NOPTI ==================== Before patch pti= always takes priority: KERNEL_CMDLINE="nopti pti=on" [ 0.022721] Unknown kernel command line parameters "nopti pti=on", will be passed to user space. [ 0.024146] Kernel/User page tables isolation: enabled Mitigation: PTI KERNEL_CMDLINE="pti=on nopti" [ 0.020566] Unknown kernel command line parameters "nopti pti=on", will be passed to user space. [ 0.021576] Kernel/User page tables isolation: enabled Mitigation: PTI After patch behavior depends on which option comes last in order: 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 CASE 2: MITIGATIONS=off ======================= Before patch pti= always overrides mitigations=: KERNEL_CMDLINE="mitigations=off pti=on" [ 0.017404] Unknown kernel command line parameters "pti=on", will be passed to user space. [ 0.018239] Kernel/User page tables isolation: enabled Mitigation: PTI KERNEL_CMDLINE="pti=on mitigations=off" [ 0.017356] Unknown kernel command line parameters "pti=on", will be passed to user space. [ 0.018232] Kernel/User page tables isolation: enabled Mitigation: PTI After patch, mitigations=off always takes priority: 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 --> I can update the patch to ensure backwards-compatible behavior in both cases for the next patch revision. [1] https://www.kernel.org/doc/html/latest/arch/x86/pti.html [2] https://www.kernel.org/doc/html/latest/admin-guide/kernel-parameters.html
On 8/11/2023 11:23 AM, Jo Van Bulck wrote: > On 08.08.23 17:13, Sohil Mehta wrote:> Can mitigations be off through > some other mechanisms such as kernel config? > I validated this both with and without the proposed patch. > Great, thanks for checking that. The existing print is accurate then. If it is printed "disabled on command line.", then PTI has been disabled due to a command line option. >> Maybe split the mitigations_off check into a separate if and it's own >> unique print message? >> Also, with the separated check you can avoid the unnecessary re-setting >> of pti_mode when pti_mode == PTI_FORCE_OFF is true. > > Thanks, makes sense. I'll make sure to do this in the next patch revision. > Based on above, even when you split the if check only a single print would be enough, right? >> In the rare case that both pti= and nopti is set the existing code seems >> to ignore the nopti option. Would the new implementation do the same? > > Good point. In my understanding, passing such conflicting options is > undefined as per the specification [2] and I'm not sure if backwards > compatibility is a requirement? > > That being said, I can see the argument that in this case of > security-sensitive functionality, it may be desirable to maintain > identical behavior for identical kernel parameter combinations and > sequences. I don't believe that is a requirement either. Sometimes kernel command lines can get very long and people make mistakes. I just thought it is neat that the current code is defaulting that way and we should probably keep the same behavior since it makes sense. > > --> I can update the patch to ensure backwards-compatible behavior in > both cases for the next patch revision. > I agree, in both cases pti= overriding the other option (nopti or mitigations=off) sounds reasonable to me. Sohil
On 11.08.23 12:13, Sohil Mehta wrote:> Based on above, even when you split the if check only a single print > would be enough, right? Yes, I agree these both cases can simply print "disabled on command line." (as in the original code) IMHO > I don't believe that is a requirement either. Sometimes kernel command > lines can get very long and people make mistakes. I just thought it is > neat that the current code is defaulting that way and we should probably > keep the same behavior since it makes sense. Makes sense indeed. > I agree, in both cases pti= overriding the other option (nopti or > mitigations=off) sounds reasonable to me. I prepared a revised patch for this and will post this shortly. Best, Jo
© 2016 - 2025 Red Hat, Inc.