2020-02-25 23:31:13

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 5/8] x86/entry/common: Provide trace/kprobe safe exit to user space functions

Split prepare_enter_to_user_mode() and mark it notrace/noprobe so the irq
flags tracing on return can be put into it.

Signed-off-by: Thomas Gleixner <[email protected]>
---
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);
+
#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. */
}



2020-02-26 05:46:33

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [patch 5/8] x86/entry/common: Provide trace/kprobe safe exit to user space functions

On 2/25/20 2: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.

Is our tooling clever enough for thsi to do anything? You have a static
inline that is only called in one place, and the caller is notrace and
NOKPROBE. Does this actually allow tracing in the static inline callee?

--Andy

2020-02-26 08:15:55

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [patch 5/8] x86/entry/common: Provide trace/kprobe safe exit to user space functions

On Tue, Feb 25, 2020 at 09:45:11PM -0800, Andy Lutomirski wrote:
> On 2/25/20 2: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.
>
> Is our tooling clever enough for thsi to do anything? You have a static
> inline that is only called in one place, and the caller is notrace and
> NOKPROBE. Does this actually allow tracing in the static inline callee?

tracing, no, but the NOKPROBE on inline functions is buggered afaiu.

2020-02-27 15:45:02

by Alexandre Chartre

[permalink] [raw]
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 <[email protected]>
> ---
> 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. */
> }
>
>

2020-02-27 15:54:57

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 5/8] x86/entry/common: Provide trace/kprobe safe exit to user space functions

Alexandre Chartre <[email protected]> writes:

> 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().

Ooops

>> /* 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?

This should be consolidated at the very end when all the interrupt muck
is done, but maybe I forgot.