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

[Jo Van Bulck]

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