From: Lai Jiangshan <laijs@linux.alibaba.com>
To: Borislav Petkov <bp@alien8.de>, Lai Jiangshan <jiangshanlai@gmail.com>
Cc: linux-kernel@vger.kernel.org, x86@kernel.org,
Josh Poimboeuf <jpoimboe@redhat.com>,
"Chang S . Bae" <chang.seok.bae@intel.com>,
Sasha Levin <sashal@kernel.org>,
Andy Lutomirski <luto@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>,
Dave Hansen <dave.hansen@linux.intel.com>,
"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH V4 01/50] x86/entry: Add fence for kernel entry swapgs in paranoid_entry()
Date: Thu, 28 Oct 2021 08:51:59 +0800 [thread overview]
Message-ID: <ed97c3be-da36-05a0-1409-56d8955e6eb7@linux.alibaba.com> (raw)
In-Reply-To: <YXlDu38aFAeBut5k@zn.tnic>
On 2021/10/27 20:19, Borislav Petkov wrote:
> On Tue, Oct 26, 2021 at 10:13:31PM +0800, Lai Jiangshan wrote:
>> From: Lai Jiangshan <laijs@linux.alibaba.com>
>>
>> Commit 18ec54fdd6d18 ("x86/speculation: Prepare entry code for Spectre
>> v1 swapgs mitigations") adds FENCE_SWAPGS_{KERNEL|USER}_ENTRY
>> for conditional swapgs. And in paranoid_entry(), it uses only
>> FENCE_SWAPGS_KERNEL_ENTRY for both branches. It is because the fence
>> is required for both cases since the CR3 write is conditinal even PTI
>> is enabled.
>>
>> But commit 96b2371413e8f ("x86/entry/64: Switch CR3 before SWAPGS in
>> paranoid entry") switches the code order and changes the branches.
>> And it misses the needed FENCE_SWAPGS_KERNEL_ENTRY for user gsbase case.
>>
>> Add it back.
>>
>> Fixes: Commit 96b2371413e8f ("x86/entry/64: Switch CR3 before SWAPGS in paranoid entry")
>> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
>> Cc: Chang S. Bae <chang.seok.bae@intel.com>
>> Cc: Sasha Levin <sashal@kernel.org>
>> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
>> ---
>> arch/x86/entry/entry_64.S | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
>> index e38a4cf795d9..1f98188e83ef 100644
>> --- a/arch/x86/entry/entry_64.S
>> +++ b/arch/x86/entry/entry_64.S
>> @@ -898,6 +898,7 @@ SYM_CODE_START_LOCAL(paranoid_entry)
>> rdmsr
>> testl %edx, %edx
>> jns .Lparanoid_entry_swapgs
>> + FENCE_SWAPGS_KERNEL_ENTRY
>
> Wouldn't it be cleaner to simply move the FENCE up from under the swapgs
> to before the jns instruction?
Sure, I will move it just before the testl instruction.
>
> This way you kill speculation for both cases and have a single fence.
>
next prev parent reply other threads:[~2021-10-28 0:52 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-26 14:13 [PATCH V4 00/50] x86/entry/64: Convert a bunch of ASM entry code into C code Lai Jiangshan
2021-10-26 14:13 ` [PATCH V4 01/50] x86/entry: Add fence for kernel entry swapgs in paranoid_entry() Lai Jiangshan
2021-10-27 12:19 ` Borislav Petkov
2021-10-28 0:51 ` Lai Jiangshan [this message]
2021-10-26 14:13 ` [PATCH V4 02/50] x86/entry: Use the correct fence macro after swapgs in kernel CR3 Lai Jiangshan
2021-10-26 14:13 ` [PATCH V4 03/50] x86/traps: Remove stack-protector from traps.c Lai Jiangshan
2021-10-26 14:13 ` [PATCH V4 04/50] x86/xen: Add xenpv_restore_regs_and_return_to_usermode() Lai Jiangshan
2021-11-02 8:58 ` Borislav Petkov
2021-11-02 9:19 ` Lai Jiangshan
2021-11-02 9:49 ` Borislav Petkov
2021-11-02 11:22 ` H. Peter Anvin
2021-11-02 14:27 ` Borislav Petkov
2021-10-26 14:20 ` [PATCH V4 05/50] x86/entry: Use swapgs and native_iret directly in swapgs_restore_regs_and_return_to_usermode Lai Jiangshan
2021-10-26 14:20 ` [PATCH V4 06/50] compiler_types.h: Add __noinstr_section() for noinstr Lai Jiangshan
2021-10-26 14:24 ` Miguel Ojeda
2021-10-26 14:20 ` [PATCH V4 07/50] x86/entry: Introduce __entry_text for entry code written in C Lai Jiangshan
2021-10-26 14:20 ` [PATCH V4 08/50] x86/entry: Move PTI_USER_* to arch/x86/include/asm/processor-flags.h Lai Jiangshan
2021-10-26 14:20 ` [PATCH V4 09/50] x86: Remove unused kernel_to_user_p4dp() and user_to_kernel_p4dp() Lai Jiangshan
2021-10-26 14:20 ` [PATCH V4 10/50] x86: Replace PTI_PGTABLE_SWITCH_BIT with PTI_USER_PGTABLE_BIT Lai Jiangshan
2021-10-26 14:27 ` [PATCH V4 11/50] x86: Mark __native_read_cr3() & native_write_cr3() as __always_inline Lai Jiangshan
2021-10-26 14:27 ` [PATCH V4 12/50] x86/traps: Move the declaration of native_irq_return_iret into proto.h Lai Jiangshan
2021-10-26 14:27 ` [PATCH V4 13/50] x86/entry: Add arch/x86/entry/entry64.c for C entry code Lai Jiangshan
2021-10-26 14:27 ` [PATCH V4 14/50] x86/entry: Expose the address of .Lgs_change to entry64.c Lai Jiangshan
2021-10-26 14:27 ` [PATCH V4 15/50] x86/entry: Add C verion of SWITCH_TO_KERNEL_CR3 as switch_to_kernel_cr3() Lai Jiangshan
2021-10-26 14:27 ` [PATCH V4 16/50] x86/traps: Add fence_swapgs_{user,kernel}_entry() Lai Jiangshan
2021-10-26 14:27 ` [PATCH V4 17/50] x86/entry: Add C {user,kernel}_entry_swapgs_and_fence() Lai Jiangshan
2021-10-26 14:27 ` [PATCH V4 18/50] x86/traps: Move pt_regs only in fixup_bad_iret() Lai Jiangshan
2021-10-26 14:30 ` [PATCH V4 19/50] x86/entry: Switch the stack after error_entry() returns Lai Jiangshan
2021-10-26 14:30 ` [PATCH V4 20/50] x86/entry: move PUSH_AND_CLEAR_REGS out of error_entry Lai Jiangshan
2021-10-26 14:30 ` [PATCH V4 21/50] x86/entry: Move cld to the start of idtentry Lai Jiangshan
2021-10-26 14:30 ` [PATCH V4 22/50] x86/entry: Don't call error_entry for XENPV Lai Jiangshan
2021-10-26 14:30 ` [PATCH V4 23/50] x86/entry: Convert SWAPGS to swapgs in error_entry() Lai Jiangshan
2021-10-26 14:32 ` [PATCH V4 24/50] x86/entry: Implement the whole error_entry() as C code Lai Jiangshan
2021-10-26 14:32 ` [PATCH V4 25/50] x86/entry: Use idtentry macro for entry_INT80_compat Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 26/50] x86/entry: Convert SWAPGS to swapgs in entry_SYSENTER_compat() Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 27/50] x86: Remove the definition of SWAPGS Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 28/50] x86/entry: Make paranoid_exit() callable Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 29/50] x86/entry: Call paranoid_exit() in asm_exc_nmi() Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 30/50] x86/entry: move PUSH_AND_CLEAR_REGS out of paranoid_entry Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 31/50] x86/entry: Add the C version ist_switch_to_kernel_cr3() Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 32/50] x86/entry: Skip CR3 write when the saved CR3 is kernel CR3 in RESTORE_CR3 Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 33/50] x86/entry: Add the C version ist_restore_cr3() Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 34/50] x86/entry: Add the C version get_percpu_base() Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 35/50] x86/entry: Add the C version ist_switch_to_kernel_gsbase() Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 36/50] x86/entry: Implement the C version ist_paranoid_entry() Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 37/50] x86/entry: Implement the C version ist_paranoid_exit() Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 38/50] x86/entry: Add a C macro to define the function body for IST in .entry.text Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 39/50] x86/debug, mce: Use C entry code Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 40/50] x86/idtentry.h: Move the definitions *IDTENTRY_{MCE|DEBUG}* up Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 41/50] x86/nmi: Use DEFINE_IDTENTRY_NMI for nmi Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 42/50] x86/nmi: Use C entry code Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 43/50] x86/entry: Add a C macro to define the function body for IST in .entry.text with an error code Lai Jiangshan
2021-10-26 14:34 ` [PATCH V4 44/50] x86/doublefault: Use C entry code Lai Jiangshan
2021-10-26 14:38 ` [PATCH V4 45/50] x86/sev: Add and use ist_vc_switch_off_ist() Lai Jiangshan
2021-10-26 14:38 ` [PATCH V4 46/50] x86/sev: Use C entry code Lai Jiangshan
2021-10-26 14:38 ` [PATCH V4 47/50] x86/entry: Remove ASM function paranoid_entry() and paranoid_exit() Lai Jiangshan
2021-10-26 14:38 ` [PATCH V4 48/50] x86/entry: Remove the unused ASM macros Lai Jiangshan
2021-10-26 14:38 ` [PATCH V4 49/50] x86/entry: Remove save_ret from PUSH_AND_CLEAR_REGS Lai Jiangshan
2021-10-26 14:38 ` [PATCH V4 50/50] 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=ed97c3be-da36-05a0-1409-56d8955e6eb7@linux.alibaba.com \
--to=laijs@linux.alibaba.com \
--cc=bp@alien8.de \
--cc=chang.seok.bae@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=jiangshanlai@gmail.com \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=sashal@kernel.org \
--cc=tglx@linutronix.de \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.