Re: [patch part-II V2 02/13] x86/entry: Mark enter_from_user_mode() notrace and NOKPROBE

[Frederic Weisbecker <frederic@kernel.org>]

On Sun, Mar 08, 2020 at 11:24:01PM +0100, Thomas Gleixner wrote:
> Both the callers in the low level ASM code and __context_tracking_exit()
> which is invoked from enter_from_user_mode() via user_exit_irqoff() are
> marked NOKPROBE. Allowing enter_from_user_mode() to be probed is
> inconsistent at best.
> 
> Aside of that while function tracing per se is safe the function trace
> entry/exit points can be used via BPF as well which is not safe to use
> before context tracking has reached CONTEXT_KERNEL and adjusted RCU.
> 
> Mark it notrace and NOKROBE.

Ok for the NOKPROBE, also I remember from the inclusion of kprobes
that spreading those NOKPROBE couldn't be more than some sort of best
effort to mitigate the accidents and it's up to the user to keep some
common sense and try to stay away from the borderline functions. The same
would apply to breakpoints, steps, etc...

Now for the BPF and function tracer, the latter has been made robust to
deal with these fragile RCU blind spots. Probably the same requirements should be
expected from the function tracer users. Perhaps we should have a specific
version of __register_ftrace_function() which protects the given probes
inside rcu_nmi_enter()? As it seems the BPF maintainer doesn't want the whole
BPF execution path to be hammered.

Thanks.

> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/entry/common.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -40,11 +40,12 @@
>  
>  #ifdef CONFIG_CONTEXT_TRACKING
>  /* Called on entry from user mode with IRQs off. */
> -__visible inline void enter_from_user_mode(void)
> +__visible inline notrace void enter_from_user_mode(void)
>  {
>  	CT_WARN_ON(ct_state() != CONTEXT_USER);
>  	user_exit_irqoff();
>  }
> +NOKPROBE_SYMBOL(enter_from_user_mode);
>  #else
>  static inline void enter_from_user_mode(void) {}
>  #endif
>