linux-kernel - Re: [PATCH 13/15] x86/fsgsbase/64: With FSGSBASE, compare GS bases on paranoid

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9aa3ef0a-84b5-9806-4213-1b63023e6c5a@zytor.com>
Date:   Wed, 21 Mar 2018 14:56:23 -0700
From:   "H. Peter Anvin" <hpa@...or.com>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     David Laight <David.Laight@...lab.com>,
        "Chang S. Bae" <chang.seok.bae@...el.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "markus.t.metzger@...el.com" <markus.t.metzger@...el.com>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "ravi.v.shankar@...el.com" <ravi.v.shankar@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>
Subject: Re: [PATCH 13/15] x86/fsgsbase/64: With FSGSBASE, compare GS bases on
 paranoid_entry

On 03/20/18 17:36, Andy Lutomirski wrote:
>>
>> But we also don't need swapgs when we have rdgsbase/wrgsbase available.
>> We can indeed just unconditionally save it (via rdgsbase) into the stack
>> frame and wrgsbase the correct percpu value.  In that case it might be
>> necessary in order to avoid insane complexity to also save/restore the
>> gs selector.
> 
> This is exactly what the old code did.  I liked the old code better.
> 
>>
>> Is it going to be faster?  *Probably* not as swapgs is designed to be
>> fast; it does, however, eliminate the need to RDMSR/WRMSR inside the
>> kernel task switch as the user space gsbase will simply live on the
>> stack.  (This is assuming we do this unconditionally on every method of
>> kernel entry, including non-paranoid.  I'm not sure if we ever care
>> about the userspace GS/GSBASE inside a paranoid handler, but if we do it
>> would be rather messy to find if we do this conditionally.
>>
>> Now...
>>
>> +       ALTERNATIVE "jmp .Lparanoid_entry_no_fsgsbase", \
>> +               "RDGSBASE %rdx", X86_FEATURE_FSGSBASE
>> +       READ_KERNEL_GSBASE %rax
>>
>> READ_KERNEL_GSBASE here seems like a Really Bad Name[TM] for this macro,
>> since it seems to imply reading MSR_KERNEL_GS_BASE, rather than finding
>> the current percpu offset.  I would prefer calling it something like
>> FIND_PERCPU_BASE or something like that.
> 
> I think we should revert to what the old patches did here.
> 

I don't really understand why you want to do it this way.

1. It means that the location of the user space gs_base is ill-defined,
   whereas with the SWAPGS it is *always* in the same place.

2. It is most likely slower, although I obviously haven't benchmarked
   it.

	-hpa