| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <adf3a609-656f-82a2-6a4e-199a12de298b@intel.com>
Date: Wed, 11 Jan 2023 17:43:18 -0800
From: "Chen, Yian" <yian.chen@...el.com>
To: Sohil Mehta <sohil.mehta@...el.com>,
<linux-kernel@...r.kernel.org>, <x86@...nel.org>,
Andy Lutomirski <luto@...nel.org>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Ravi Shankar <ravi.v.shankar@...el.com>,
"Tony Luck" <tony.luck@...el.com>, Paul Lai <paul.c.lai@...el.com>
Subject: Re: [PATCH 4/7] x86/vsyscall: Setup vsyscall to compromise LASS
protection
On 1/10/2023 4:34 PM, Sohil Mehta wrote:
> On 1/9/2023 9:52 PM, Yian Chen wrote:
>
>> The user can also opt-out LASS in config file to build kernel
>> binary.
>
> This line is unnecessary.
>
Sure, this line can be dropped.
>> Signed-off-by: Yian Chen <yian.chen@...el.com>
>> Reviewed-by: Tony Luck <tony.luck@...el.com>
>> ---
>> Documentation/admin-guide/kernel-parameters.txt | 12 ++++++++----
>> arch/x86/entry/vsyscall/vsyscall_64.c | 14 ++++++++++++++
>> 2 files changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt
>> b/Documentation/admin-guide/kernel-parameters.txt
>> index 6cfa6e3996cf..3988e0c8c175 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -6755,10 +6755,14 @@
>> versions of glibc use these calls. Because these
>> functions are at fixed addresses, they make nice
>> targets for exploits that can control RIP.
>> -
>> - emulate [default] Vsyscalls turn into traps and are
>> - emulated reasonably safely. The vsyscall
>> - page is readable.
>
> The existing documentation here is incorrect. The default vsyscall mode
> is actually xonly. This has been so since:
> commit 625b7b7f79c6 (x86/vsyscall: Change the default vsyscall mode to
> xonly)
>
Yes, you are right. but this patch can overwrite and correct existing
one. I am assuming we don't need to correct the existing document first
before update it for LASS.
>> + In newer versions of Intel platforms that come with
>
> Words such as "newer" in the kernel start losing meaning very quickly.
> Also, this comment looks out of place in between the vsyscall sub-options.
>
>> + LASS(Linear Address Space separation) protection,
>> + vsyscall is disabled by default. Enabling vsyscall
>> + via the parameter overrides LASS protection.
>> +
Sure, I will take out this part change.
>
>
> IIUC, you are making the default mode somewhat dynamic.
>
> vsyscall = xonly (if LASS is not enabled)
> vsyscall = none (if LASS is enabled)
>
Yes, this looks better.
> The decision to disable vsyscall doesn't happen at compile time but it
> is taken at runtime when the LASS feature is detected. This would make
> the system behavior highly platform dependent.
>
> Instead of doing this dance, can we provide a simplified behavior to the
> user/admin and move the decision making to compile time?
>
Current strategy is to disable vsyscall by default only for LASS capable
platforms. So that the dynamic decision is likely a necessary.
> Option 1: A bigger hammer
> Set the default vsyscall option as CONFIG_LEGACY_VSYSCALL_NONE.
> CONFIG_X86_LASS is set by default. Changing the compile time VSYSCALL
> option would disable LASS.
>
This means to disable vsyscall by default for all platforms, doen't
matter LASS. I am not sure if we want to go that far.
> Option 2: A smaller hammer
> CONFIG_X86_LASS is off by default. Vsyscall default stays as
> CONFIG_LEGACY_VSYSCALL_XONLY. Selecting LASS automatically chooses
> CONFIG_LEGACY_VSYSCALL_NONE. In this case, even if the hardware doesn't
> support LASS, vsyscall would still remain disabled.
>
This turns out to disable LASS by default. Then the LASS may not be
taken in the first place.
> In both of these cases, any command line input to override the vsyscall
> behavior can disable LASS as well.
>
>
>> + emulate [default if not LASS capable] Vsyscalls
>> + turn into traps and are emulated reasonably
>> + safely. The vsyscall page is readable.
>> xonly Vsyscalls turn into traps and are
>> emulated reasonably safely. The vsyscall
>> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c
>> b/arch/x86/entry/vsyscall/vsyscall_64.c
>> index 4af81df133ee..2691f26835d1 100644
>> --- a/arch/x86/entry/vsyscall/vsyscall_64.c
>> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
>> @@ -63,6 +63,12 @@ static int __init vsyscall_setup(char *str)
>> else
>> return -EINVAL;
>> + if (cpu_feature_enabled(X86_FEATURE_LASS) &&
>> + vsyscall_mode != NONE) {
>> + setup_clear_cpu_cap(X86_FEATURE_LASS);
>> + pr_warn("LASS disabled by command line enabling
>> vsyscall\n");
>
> A warning seems too drastic here. A pr_info() should probably suffice.
>
sure I will modify it to use pr_info.
>> + }
>> +
>> return 0;
>> }
>> @@ -379,6 +385,14 @@ void __init map_vsyscall(void)
>> extern char __vsyscall_page;
>> unsigned long physaddr_vsyscall = __pa_symbol(&__vsyscall_page);
>> + /*
>> + * When LASS is on, vsyscall triggers a #GP fault,
>> + * so that force vsyscall_mode to NONE.
>> + */
>
> This comment doesn't make much sense nor does it provide the full
> picture. Some of the reasoning from the cover letter/commit log can be
> duplicated here.
>
sure, How about a more detail inline comment as following:
+ /*
+ * When LASS protection is on, user space vsyscall triggers
+ * a #GP fault since the vsyscall page is
+ * 0xffffffffff600000-0xffffffffff601000,
+ * so that force vsyscall_mode to NONE and disable this mapping.
+ */
>> + if (cpu_feature_enabled(X86_FEATURE_LASS)) {
>> + vsyscall_mode = NONE;
>> + return;
>> + }
>> /*
>> * For full emulation, the page needs to exist for real. In
>> * execute-only mode, there is no PTE at all backing the vsyscall
>
Thanks,
Yian
Powered by blists - more mailing lists