All of lore.kernel.org
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: linux-kernel@vger.kernel.org, x86@kernel.org,
	Lai Jiangshan <laijs@linux.alibaba.com>,
	Jan Beulich <jbeulich@suse.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Juergen Gross <jgross@suse.com>, Peter Anvin <hpa@zytor.com>,
	xen-devel@lists.xenproject.org, Andy Lutomirski <luto@kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Stefano Stabellini <sstabellini@kernel.org>
Subject: Re: [PATCH V4 04/50] x86/xen: Add xenpv_restore_regs_and_return_to_usermode()
Date: Tue, 2 Nov 2021 09:58:10 +0100	[thread overview]
Message-ID: <YYD9ohN2Zcy4EdMb@zn.tnic> (raw)
In-Reply-To: <20211026141420.17138-5-jiangshanlai@gmail.com>

On Tue, Oct 26, 2021 at 10:13:34PM +0800, Lai Jiangshan wrote:
> From: Lai Jiangshan <laijs@linux.alibaba.com>
> 
> While in the native case, PER_CPU_VAR(cpu_tss_rw + TSS_sp0) is the
> trampoline stack.  But XEN pv doesn't use trampoline stack, so
> PER_CPU_VAR(cpu_tss_rw + TSS_sp0) is also the kernel stack.  Hence source
> and destination stacks are identical in that case, which means reusing
> swapgs_restore_regs_and_return_to_usermode() in XEN pv would cause %rsp
> to move up to the top of the kernel stack and leave the IRET frame below
> %rsp, which is dangerous to be corrupted if #NMI / #MC hit as either of
> these events occurring in the middle of the stack pushing would clobber
> data on the (original) stack.
> 
> And swapgs_restore_regs_and_return_to_usermode() pushing the IRET frame
> on to the original address is useless and error-prone when there is any
> future attempt to modify the code.
> 
> Fixes: 7f2590a110b8 ("x86/entry/64: Use a per-CPU trampoline stack for IDT entries")
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Peter Anvin <hpa@zytor.com>
> Cc: xen-devel@lists.xenproject.org>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
> ---
>  arch/x86/entry/entry_64.S        |  9 ++++++---
>  arch/x86/entry/entry_64_compat.S |  7 ++++---
>  arch/x86/xen/xen-asm.S           | 27 +++++++++++++++++++++++++++
>  3 files changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index 9d468c8877e2..0dde5a253dda 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -119,7 +119,7 @@ SYM_INNER_LABEL(entry_SYSCALL_64_after_hwframe, SYM_L_GLOBAL)
>  	 * In the Xen PV case we must use iret anyway.
>  	 */
>  
> -	ALTERNATIVE "", "jmp	swapgs_restore_regs_and_return_to_usermode", \
> +	ALTERNATIVE "", "jmp xenpv_restore_regs_and_return_to_usermode", \

Instead of sprinkling all those ALTERNATIVE calls everywhere,
why don't you simply jump to the xenpv-one at the
swapgs_restore_regs_and_return_to_usermode label itself and have a
single ALTERNATIVE there?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette


  reply	other threads:[~2021-11-02  8:59 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
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 [this message]
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=YYD9ohN2Zcy4EdMb@zn.tnic \
    --to=bp@alien8.de \
    --cc=boris.ostrovsky@oracle.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=jiangshanlai@gmail.com \
    --cc=laijs@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=sstabellini@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xenproject.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.