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 <[email protected]>
---
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.1
On 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
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
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 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