Re: [PATCH] x86/optprobe: Fix OPTPROBE vs UACCESS

[Masami Hiramatsu]

Hi Peter,

On Thu, 5 Mar 2020 10:21:30 +0100
Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:

> 
> While looking at an objtool UACCESS warning, it suddenly occurred to me
> that it is entirely possible to have an OPTPROBE right in the middle of
> an UACCESS region.
> 
> In this case we must of course clear FLAGS.AC while running the KPROBE.
> Luckily the trampoline already saves/restores [ER]FLAGS, so all we need
> to do is inject a CLAC. Unfortunately we cannot use ALTERNATIVE() in the
> trampoline text, so we have to frob that manually.

Good catch! so this prevents optprobe handler to access user space
avoiding SMAP feature.

Acked-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>

Thank you,

> 
> Fixes: ca0bbc70f147 ("sched/x86_64: Don't save flags on context switch")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> ---
>  arch/x86/include/asm/kprobes.h |  1 +
>  arch/x86/kernel/kprobes/opt.c  | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h
> index 95b1f053bd96..073eb7ad2f56 100644
> --- a/arch/x86/include/asm/kprobes.h
> +++ b/arch/x86/include/asm/kprobes.h
> @@ -36,6 +36,7 @@ typedef u8 kprobe_opcode_t;
>  
>  /* optinsn template addresses */
>  extern __visible kprobe_opcode_t optprobe_template_entry[];
> +extern __visible kprobe_opcode_t optprobe_template_clac[];
>  extern __visible kprobe_opcode_t optprobe_template_val[];
>  extern __visible kprobe_opcode_t optprobe_template_call[];
>  extern __visible kprobe_opcode_t optprobe_template_end[];
> diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
> index 3f45b5c43a71..7a3416c9d0dc 100644
> --- a/arch/x86/kernel/kprobes/opt.c
> +++ b/arch/x86/kernel/kprobes/opt.c
> @@ -92,6 +92,9 @@ asm (
>  			/* We don't bother saving the ss register */
>  			"	pushq %rsp\n"
>  			"	pushfq\n"
> +			".global optprobe_template_clac\n"
> +			"optprobe_template_clac:\n"
> +			ASM_NOP3
>  			SAVE_REGS_STRING
>  			"	movq %rsp, %rsi\n"
>  			".global optprobe_template_val\n"
> @@ -111,6 +114,9 @@ asm (
>  #else /* CONFIG_X86_32 */
>  			"	pushl %esp\n"
>  			"	pushfl\n"
> +			".global optprobe_template_clac\n"
> +			"optprobe_template_clac:\n"
> +			ASM_NOP3
>  			SAVE_REGS_STRING
>  			"	movl %esp, %edx\n"
>  			".global optprobe_template_val\n"
> @@ -134,6 +140,8 @@ asm (
>  void optprobe_template_func(void);
>  STACK_FRAME_NON_STANDARD(optprobe_template_func);
>  
> +#define TMPL_CLAC_IDX \
> +	((long)optprobe_template_clac - (long)optprobe_template_entry)
>  #define TMPL_MOVE_IDX \
>  	((long)optprobe_template_val - (long)optprobe_template_entry)
>  #define TMPL_CALL_IDX \
> @@ -389,6 +397,12 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op,
>  	op->optinsn.size = ret;
>  	len = TMPL_END_IDX + op->optinsn.size;
>  
> +	if (static_cpu_has(X86_FEATURE_SMAP)) {
> +		buf[TMPL_CLAC_IDX+0] = 0x0f;
> +		buf[TMPL_CLAC_IDX+1] = 0x01;
> +		buf[TMPL_CLAC_IDX+2] = 0xca;
> +	}
> +
>  	/* Set probe information */
>  	synthesize_set_arg1(buf + TMPL_MOVE_IDX, (unsigned long)op);
>  


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>