linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Lai Jiangshan <laijs@linux.alibaba.com>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	linux-kernel@vger.kernel.org
Cc: Josh Poimboeuf <jpoimboe@redhat.com>,
	"Chang S . Bae" <chang.seok.bae@intel.com>,
	Sasha Levin <sashal@kernel.org>,
	Andy Lutomirski <luto@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH V2 01/41] x86/entry: Fix swapgs fence
Date: Mon, 27 Sep 2021 09:50:22 +0200	[thread overview]
Message-ID: <875yumbgox.ffs@tglx> (raw)
In-Reply-To: <445de475-c223-be11-325f-fa6679e45cb0@linux.alibaba.com>

Lai,

On Mon, Sep 27 2021 at 11:27, Lai Jiangshan wrote:
> On 2021/9/27 09:10, Lai Jiangshan wrote:
>
> The commit c75890700455 ("x86/entry/64: Remove unneeded kernel CR3 switching")
> ( https://lore.kernel.org/all/20200419144049.1906-2-laijs@linux.alibaba.com/ )
> also made it wrong.

Duh, did not spot that either.

> When the SWITCH_TO_KERNEL_CR3 in the path is removed, FENCE_SWAPGS_USER_ENTRY
> should also be changed to FENCE_SWAPGS_KERNEL_ENTRY. (Or just jmp to
> .Lerror_entry_done_lfence which has FENCE_SWAPGS_KERNEL_ENTRY already.)

Yes.

> And FENCE_SWAPGS_USER_ENTRY could be documented with "it should be followed with
> serializing operations such as SWITCH_TO_KERNEL_CR3".

It does not matter whether the serializing is before or after. The
problem is:

    if (from_user)
    	swapgs();

can take the wrong path speculatively which means the speculation is
then based on the wrong GS.

We have these sequences in the non paranoid entries:

    if (from_user) {
       pti_switch_cr3();
       swapgs();
    }

    if (from_user) {
       swapgs();
       pti_switch_cr3();
    }

and with mitigation these become:

    if (from_user) {
       pti_switch_cr3();
       swapgs();
       lfence_if_not_pti();
    } else {
       lfence();
    }

    if (from_user) {
       swapgs();
       lfence_if_not_pti();
       pti_switch_cr3();
    } else {
       lfence();
    }

When PTI is enabled then the CR3 write is sufficient because it's fully
serializing. If PTI is off the LFENCE is required. On which side the CR3
write is before or after SWAPGS does not matter. 

>  Or we can add a SWAPGS_AND_SWITCH_TO_KERNEL_CR3 to combine them.

No. We really don't want to go there.

Thanks,

        tglx


  reply	other threads:[~2021-09-27  7:50 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-26 15:07 [PATCH V2 00/41] x86/entry/64: Convert a bunch of ASM entry code into C code Lai Jiangshan
2021-09-26 15:07 ` [PATCH V2 01/41] x86/entry: Fix swapgs fence Lai Jiangshan
2021-09-26 20:43   ` Thomas Gleixner
2021-09-27  1:10     ` Lai Jiangshan
2021-09-27  3:27       ` Lai Jiangshan
2021-09-27  7:50         ` Thomas Gleixner [this message]
2021-09-26 15:07 ` [PATCH V2 02/41] x86/traps: Remove stack-protector from traps.c Lai Jiangshan
2021-09-27 10:19   ` Borislav Petkov
2021-09-27 10:49     ` Lai Jiangshan
2021-09-27 11:01       ` Borislav Petkov
2021-09-27 14:38         ` Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 03/41] compiler_types.h: Add __noinstr_section() for noinstr Lai Jiangshan
2021-09-27 18:09   ` Kees Cook
2021-09-26 15:08 ` [PATCH V2 04/41] x86/entry: Introduce __entry_text for entry code written in C Lai Jiangshan
2021-09-30 11:49   ` Borislav Petkov
2021-09-26 15:08 ` [PATCH V2 05/41] x86/entry: Move PTI_USER_* to arch/x86/include/asm/processor-flags.h Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 06/41] x86: Mark __native_read_cr3() & native_write_cr3() as __always_inline Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 07/41] x86/traps: Move the declaration of native_irq_return_iret into proto.h Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 08/41] x86/entry: Add arch/x86/entry/entry64.c for C entry code Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 09/41] x86/entry: Expose the address of .Lgs_change to entry64.c Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 10/41] x86/entry: Add C verion of SWITCH_TO_KERNEL_CR3 as switch_to_kernel_cr3() Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 11/41] x86/entry: Add C user_entry_swapgs_and_fence() and kernel_entry_fence_no_swapgs() Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 12/41] x86/traps: Move pt_regs only in fixup_bad_iret() Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 13/41] x86/entry: Switch the stack after error_entry() returns Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 14/41] x86/entry: move PUSH_AND_CLEAR_REGS out of error_entry Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 15/41] objtool: Allow .entry.text function using CLD instruction Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 16/41] x86/entry: Implement the whole error_entry() as C code Lai Jiangshan
2021-09-28 21:34   ` Brian Gerst
2021-09-29  8:45     ` Peter Zijlstra
2021-09-26 15:08 ` [PATCH V2 17/41] x86/entry: Make paranoid_exit() callable Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 18/41] x86/entry: Call paranoid_exit() in asm_exc_nmi() Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 19/41] x86/entry: move PUSH_AND_CLEAR_REGS out of paranoid_entry Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 20/41] x86/entry: Add the C version ist_switch_to_kernel_cr3() Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 21/41] x86/entry: Add the C version ist_restore_cr3() Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 22/41] x86/entry: Add the C version get_percpu_base() Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 23/41] x86/entry: Add the C version ist_switch_to_kernel_gsbase() Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 24/41] x86/entry: Implement the C version ist_paranoid_entry() Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 25/41] x86/entry: Implement the C version ist_paranoid_exit() Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 26/41] x86/entry: Add a C macro to define the function body for IST in .entry.text Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 27/41] x86/mce: Remove stack protector from mce/core.c Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 28/41] x86/debug, mce: Use C entry code Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 29/41] x86/idtentry.h: Move the definitions *IDTENTRY_{MCE|DEBUG}* up Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 30/41] x86/nmi: Use DEFINE_IDTENTRY_NMI for nmi Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 31/41] x86/nmi: Remove stack protector from nmi.c Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 32/41] x86/nmi: Use C entry code Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 33/41] x86/entry: Add a C macro to define the function body for IST in .entry.text with an error code Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 34/41] x86/doublefault: Use C entry code Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 35/41] x86/sev: Add and use ist_vc_switch_off_ist() Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 36/41] x86/sev: Remove stack protector from sev.c Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 37/41] x86/sev: Use C entry code Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 38/41] x86/entry: Remove ASM function paranoid_entry() and paranoid_exit() Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 39/41] x86/entry: Remove the unused ASM macros Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 40/41] x86/entry: Remove save_ret from PUSH_AND_CLEAR_REGS Lai Jiangshan
2021-09-26 15:08 ` [PATCH V2 41/41] x86/syscall/64: Move the checking for sysret to C code Lai Jiangshan

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=875yumbgox.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=bp@alien8.de \
    --cc=chang.seok.bae@intel.com \
    --cc=hpa@zytor.com \
    --cc=jiangshanlai@gmail.com \
    --cc=jpoimboe@redhat.com \
    --cc=laijs@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=sashal@kernel.org \
    --cc=x86@kernel.org \
    /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).