linux-kernel - Re: [PATCH 2/3 v4] ftrace/x86: Allow for arguments to be passed in to ftrace

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201109111043.GD2594@hirez.programming.kicks-ass.net>
Date:   Mon, 9 Nov 2020 12:10:43 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Miroslav Benes <mbenes@...e.cz>,
        Thomas Gleixner <tglx@...utronix.de>,
        Masami Hiramatsu <mhiramat@...nel.org>
Subject: Re: [PATCH 2/3 v4] ftrace/x86: Allow for arguments to be passed in
 to ftrace_regs by default

On Fri, Nov 06, 2020 at 04:42:36PM -0500, Steven Rostedt wrote:
> +static __always_inline struct pt_regs *
> +arch_ftrace_get_regs(struct ftrace_regs *fregs)
> +{
> +	/* Only when FL_SAVE_REGS is set, cs will be non zero */
> +	if (!fregs->regs.cs)
> +		return NULL;
> +	return &fregs->regs;
> +}
> +#endif
> +
>  #endif /*  CONFIG_DYNAMIC_FTRACE */
>  #endif /* __ASSEMBLY__ */
>  #endif /* CONFIG_FUNCTION_TRACER */
> diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
> index ac3d5f22fe64..60e3b64f5ea6 100644
> --- a/arch/x86/kernel/ftrace_64.S
> +++ b/arch/x86/kernel/ftrace_64.S
> @@ -140,12 +140,19 @@ SYM_FUNC_START(ftrace_caller)
>  	/* save_mcount_regs fills in first two parameters */
>  	save_mcount_regs
>  
> +	/* Stack - skipping return address of ftrace_caller */
> +	leaq MCOUNT_REG_SIZE+8(%rsp), %rcx
> +	movq %rcx, RSP(%rsp)
> +
>  SYM_INNER_LABEL(ftrace_caller_op_ptr, SYM_L_GLOBAL)
>  	/* Load the ftrace_ops into the 3rd parameter */
>  	movq function_trace_op(%rip), %rdx
>  
> -	/* regs go into 4th parameter (but make it NULL) */
> -	movq $0, %rcx
> +	/* regs go into 4th parameter */
> +	leaq (%rsp), %rcx
> +
> +	/* Only ops with REGS flag set should have CS register set */
> +	movq $0, CS(%rsp)
>  
>  SYM_INNER_LABEL(ftrace_call, SYM_L_GLOBAL)
>  	call ftrace_stub

You now seem to be relying on save_mcount_regs() resulting in a cleared
CS, however, AFAICT CS is uninitialized stack garbage.