linux-kernel - Re: [patch 5/8] x86/entry/common: Provide trace/kprobe safe exit to user space functions

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: <71271e39-1dbd-0bd6-2435-fc8a30f47b7d@oracle.com>
Date:   Thu, 27 Feb 2020 16:43:43 +0100
From:   Alexandre Chartre <alexandre.chartre@...cle.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>
Cc:     x86@...nel.org, Steven Rostedt <rostedt@...dmis.org>,
        Brian Gerst <brgerst@...il.com>,
        Juergen Gross <jgross@...e.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [patch 5/8] x86/entry/common: Provide trace/kprobe safe exit to
 user space functions


On 2/25/20 11:08 PM, Thomas Gleixner wrote:
> Split prepare_enter_to_user_mode() and mark it notrace/noprobe so the irq
> flags tracing on return can be put into it.

This splits prepare_exit_to_usermode() not prepare_enter_to_user_mode().


> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
>   arch/x86/entry/common.c |   15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -196,7 +196,7 @@ static void exit_to_usermode_loop(struct
>   }
>   
>   /* Called with IRQs disabled. */
> -__visible inline void prepare_exit_to_usermode(struct pt_regs *regs)
> +static inline void __prepare_exit_to_usermode(struct pt_regs *regs)
>   {
>   	struct thread_info *ti = current_thread_info();
>   	u32 cached_flags;
> @@ -241,6 +241,12 @@ static void exit_to_usermode_loop(struct
>   	mds_user_clear_cpu_buffers();
>   }
>   
> +__visible inline notrace void prepare_exit_to_usermode(struct pt_regs *regs)
> +{
> +	__prepare_exit_to_usermode(regs);
> +}
> +NOKPROBE_SYMBOL(prepare_exit_to_usermode);


Would it be worth grouping local_irq_disable() and prepare_exit_to_usermode()
(similarly to what was done entry with syscall_entry_fixups()) and then put
trace_hardirqs_on() there too. For example:

static __always_inline void syscall_exit_fixups(struct pt_regs *regs)
{
         local_irq_disable();
         prepare_exit_to_usermode(regs);
         trace_hardirqs_on();
}

Or is this planned once prepare_exit_from_usermode() is not used by idtentry
anymore?

Thanks,

alex.

>   #define SYSCALL_EXIT_WORK_FLAGS				\
>   	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT |	\
>   	 _TIF_SINGLESTEP | _TIF_SYSCALL_TRACEPOINT)
> @@ -271,7 +277,7 @@ static void syscall_slow_exit_work(struc
>    * Called with IRQs on and fully valid regs.  Returns with IRQs off in a
>    * state such that we can immediately switch to user mode.
>    */
> -__visible inline void syscall_return_slowpath(struct pt_regs *regs)
> +__visible inline notrace void syscall_return_slowpath(struct pt_regs *regs)
>   {
>   	struct thread_info *ti = current_thread_info();
>   	u32 cached_flags = READ_ONCE(ti->flags);
> @@ -292,8 +298,9 @@ static void syscall_slow_exit_work(struc
>   		syscall_slow_exit_work(regs, cached_flags);
>   
>   	local_irq_disable();
> -	prepare_exit_to_usermode(regs);
> +	__prepare_exit_to_usermode(regs);
>   }
> +NOKPROBE_SYMBOL(syscall_return_slowpath);
>   
>   #ifdef CONFIG_X86_64
>   static __always_inline
> @@ -417,7 +424,7 @@ static __always_inline long do_fast_sysc
>   		/* User code screwed up. */
>   		local_irq_disable();
>   		regs->ax = -EFAULT;
> -		prepare_exit_to_usermode(regs);
> +		__prepare_exit_to_usermode(regs);
>   		return 0;	/* Keep it simple: use IRET. */
>   	}
>   
>