From: Thomas Gleixner <tglx@linutronix.de>
To: "Chang S. Bae" <chang.seok.bae@intel.com>
Cc: Ingo Molnar <mingo@kernel.org>, Andy Lutomirski <luto@kernel.org>,
"H . Peter Anvin" <hpa@zytor.com>,
Andi Kleen <ak@linux.intel.com>,
Ravi Shankar <ravi.v.shankar@intel.com>,
LKML <linux-kernel@vger.kernel.org>,
Dave Hansen <dave.hansen@linux.intel.com>
Subject: Re: [RESEND PATCH v6 08/12] x86/fsgsbase/64: Use the per-CPU base as GSBASE at the paranoid_entry
Date: Fri, 5 Apr 2019 10:35:48 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.21.1904050007050.1802@nanos.tec.linutronix.de> (raw)
In-Reply-To: <alpine.DEB.2.21.1903251003090.1798@nanos.tec.linutronix.de>
On Mon, 25 Mar 2019, Thomas Gleixner wrote:
> On Fri, 15 Mar 2019, Chang S. Bae wrote:
> > ENTRY(paranoid_exit)
> > UNWIND_HINT_REGS
> > DISABLE_INTERRUPTS(CLBR_ANY)
> > TRACE_IRQS_OFF_DEBUG
> > + ALTERNATIVE "jmp .Lparanoid_exit_no_fsgsbase", "nop",\
> > + X86_FEATURE_FSGSBASE
> > + wrgsbase %rbx
> > + jmp .Lparanoid_exit_no_swapgs;
>
> Again. A few newlines would make it more readable.
>
> This modifies the semantics of paranoid_entry and paranoid_exit. Looking at
> the usage sites there is the following code in the nmi maze:
>
> /*
> * Use paranoid_entry to handle SWAPGS, but no need to use paranoid_exit
> * as we should not be calling schedule in NMI context.
> * Even with normal interrupts enabled. An NMI should not be
> * setting NEED_RESCHED or anything that normal interrupts and
> * exceptions might do.
> */
> call paranoid_entry
> UNWIND_HINT_REGS
>
> /* paranoidentry do_nmi, 0; without TRACE_IRQS_OFF */
> movq %rsp, %rdi
> movq $-1, %rsi
> call do_nmi
>
> /* Always restore stashed CR3 value (see paranoid_entry) */
> RESTORE_CR3 scratch_reg=%r15 save_reg=%r14
>
> testl %ebx, %ebx /* swapgs needed? */
> jnz nmi_restore
> nmi_swapgs:
> SWAPGS_UNSAFE_STACK
> nmi_restore:
> POP_REGS
>
> I might be missing something, but how is that supposed to work when
> paranoid_entry uses FSGSBASE? I think it's broken, but if it's not then
> there is a big fat comment missing explaining why.
So this _is_ broken.
On entry:
rbx = rdgsbase()
wrgsbase(KERNEL_GS)
On exit:
if (ebx == 0)
swapgs
The resulting matrix:
| ENTRY GS | RBX | EXIT | GS on IRET | RESULT
| | | | |
1 | KERNEL_GS | KERNEL_GS | EBX == 0 | USER_GS | FAIL
| | | | |
2 | KERNEL_GS | KERNEL_GS | EBX != 0 | KERNEL_GS | ok
| | | | |
3 | USER_GS | USER_GS | EBX == 0 | USER_GS | ok
| | | | |
4 | USER_GS | USER_GS | EBX != 0 | KERNEL_GS | FAIL
#1 Just works by chance because it's unlikely that the lower 32bits of a
per CPU kernel GS are all 0.
But it's just a question of probability that this turns into a
non-debuggable once per year crash (think KASLR).
#4 This can happen when the NMI hits the kernel in some other entry code
_BEFORE_ or _AFTER_ swapgs.
User space using GS addressing with GS[31:0] != 0 will crash and burn.
IIRC FSGSBASE is about fast user space GS switching with (almost) no
limits on the value ...
Oh well.
tglx
next prev parent reply other threads:[~2019-04-05 8:35 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-15 20:06 [RESEND PATCH v6 00/12] x86: Enable FSGSBASE instructions Chang S. Bae
2019-03-15 20:06 ` [RESEND PATCH v6 01/12] x86/fsgsbase/64: Add 'unsafe_fsgsbase' to enable CR4.FSGSBASE Chang S. Bae
2019-03-15 20:06 ` [RESEND PATCH v6 02/12] kbuild: Raise the minimum required binutils version to 2.21 Chang S. Bae
2019-03-15 20:06 ` [RESEND PATCH v6 03/12] x86/fsgsbase/64: Add intrinsics for FSGSBASE instructions Chang S. Bae
2019-03-15 20:06 ` [RESEND PATCH v6 04/12] x86/fsgsbase/64: Enable FSGSBASE instructions in the helper functions Chang S. Bae
2019-03-25 11:38 ` Thomas Gleixner
2019-03-25 12:46 ` Thomas Gleixner
2019-03-25 13:05 ` Thomas Gleixner
2019-03-26 0:38 ` Andi Kleen
2019-03-26 15:01 ` New feature/ABI review process [was Re: [RESEND PATCH v6 04/12] x86/fsgsbase/64:..] Thomas Gleixner
2019-03-26 22:56 ` Andi Kleen
2019-03-27 21:15 ` Thomas Gleixner
2019-03-15 20:06 ` [RESEND PATCH v6 05/12] x86/fsgsbase/64: Preserve FS/GS state in __switch_to() if FSGSBASE is on Chang S. Bae
2019-03-15 20:06 ` [RESEND PATCH v6 06/12] x86/fsgsbase/64: When copying a thread, use the FSGSBASE instructions if available Chang S. Bae
2019-03-15 20:06 ` [RESEND PATCH v6 07/12] x86/fsgsbase/64: Introduce the FIND_PERCPU_BASE macro Chang S. Bae
2019-03-25 9:02 ` Thomas Gleixner
2019-05-01 13:52 ` Bae, Chang Seok
2019-03-15 20:06 ` [RESEND PATCH v6 08/12] x86/fsgsbase/64: Use the per-CPU base as GSBASE at the paranoid_entry Chang S. Bae
2019-03-25 9:44 ` Thomas Gleixner
2019-04-05 8:35 ` Thomas Gleixner [this message]
2019-04-05 13:50 ` Andy Lutomirski
2019-05-01 13:52 ` Bae, Chang Seok
2019-05-01 17:40 ` Andy Lutomirski
2019-05-01 18:01 ` Bae, Chang Seok
[not found] ` <7029A32B-958E-4C1E-8B5F-D49BA68E4755@intel.com>
2019-05-01 20:25 ` Andy Lutomirski
2019-05-01 21:04 ` Bae, Chang Seok
2019-05-02 0:29 ` Andy Lutomirski
2019-05-06 22:56 ` Bae, Chang Seok
2019-03-15 20:06 ` [RESEND PATCH v6 09/12] selftests/x86/fsgsbase: Test WRGSBASE Chang S. Bae
2019-03-15 20:06 ` [RESEND PATCH v6 10/12] x86/fsgsbase/64: Enable FSGSBASE by default and add a chicken bit Chang S. Bae
2019-03-15 20:06 ` [RESEND PATCH v6 11/12] x86/elf: Enumerate kernel FSGSBASE capability in AT_HWCAP2 Chang S. Bae
2019-03-15 20:06 ` [RESEND PATCH v6 12/12] x86/fsgsbase/64: Add documentation for FSGSBASE Chang S. Bae
2019-03-30 16:15 ` Randy Dunlap
2019-03-26 0:43 ` [RESEND PATCH v6 00/12] x86: Enable FSGSBASE instructions Andy Lutomirski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.21.1904050007050.1802@nanos.tec.linutronix.de \
--to=tglx@linutronix.de \
--cc=ak@linux.intel.com \
--cc=chang.seok.bae@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@kernel.org \
--cc=ravi.v.shankar@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).