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: <20241021170340.GB26122@willie-the-truck>
Date: Mon, 21 Oct 2024 18:03:40 +0100
From: Will Deacon <will@...nel.org>
To: "Masami Hiramatsu (Google)" <mhiramat@...nel.org>
Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Florent Revest <revest@...omium.org>,
	linux-trace-kernel@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>,
	Martin KaFai Lau <martin.lau@...ux.dev>, bpf <bpf@...r.kernel.org>,
	Alexei Starovoitov <ast@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
	Alan Maguire <alan.maguire@...cle.com>,
	Mark Rutland <mark.rutland@....com>, linux-arch@...r.kernel.org,
	Catalin Marinas <catalin.marinas@....com>,
	Huacai Chen <chenhuacai@...nel.org>,
	WANG Xuerui <kernel@...0n.name>,
	Michael Ellerman <mpe@...erman.id.au>,
	Nicholas Piggin <npiggin@...il.com>,
	Christophe Leroy <christophe.leroy@...roup.eu>,
	Naveen N Rao <naveen@...nel.org>,
	Madhavan Srinivasan <maddy@...ux.ibm.com>,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Albert Ou <aou@...s.berkeley.edu>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	"H. Peter Anvin" <hpa@...or.com>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Subject: Re: [PATCH v17 01/16] function_graph: Pass ftrace_regs to entryfunc

On Wed, Oct 16, 2024 at 09:57:55AM +0900, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> 
> Pass ftrace_regs to the fgraph_ops::entryfunc(). If ftrace_regs is not
> available, it passes a NULL instead. User callback function can access
> some registers (including return address) via this ftrace_regs.
> 
> Note that the ftrace_regs can be NULL when the arch does NOT define:
> HAVE_DYNAMIC_FTRACE_WITH_ARGS or HAVE_DYNAMIC_FTRACE_WITH_REGS.
> More specifically, if HAVE_DYNAMIC_FTRACE_WITH_REGS is defined but
> not the HAVE_DYNAMIC_FTRACE_WITH_ARGS, and the ftrace ops used to
> register the function callback does not set FTRACE_OPS_FL_SAVE_REGS.
> In this case, ftrace_regs can be NULL in user callback.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: Catalin Marinas <catalin.marinas@....com>
> Cc: Will Deacon <will@...nel.org>
> Cc: Huacai Chen <chenhuacai@...nel.org>
> Cc: WANG Xuerui <kernel@...0n.name>
> Cc: Michael Ellerman <mpe@...erman.id.au>
> Cc: Nicholas Piggin <npiggin@...il.com>
> Cc: Christophe Leroy <christophe.leroy@...roup.eu>
> Cc: Naveen N Rao <naveen@...nel.org>
> Cc: Madhavan Srinivasan <maddy@...ux.ibm.com>
> Cc: Paul Walmsley <paul.walmsley@...ive.com>
> Cc: Palmer Dabbelt <palmer@...belt.com>
> Cc: Albert Ou <aou@...s.berkeley.edu>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Borislav Petkov <bp@...en8.de>
> Cc: Dave Hansen <dave.hansen@...ux.intel.com>
> Cc: x86@...nel.org
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
> 
> ---
>  Changes in v16:
>   - Add a note when the ftrace_regs can be NULL.
>   - Update against for the latest kernel.
>  Changes in v11:
>   - Update for the latest for-next branch.
>  Changes in v8:
>   - Just pass ftrace_regs to the handler instead of adding a new
>     entryregfunc.
>   - Update riscv ftrace_graph_func().
>  Changes in v3:
>   - Update for new multiple fgraph.
> ---
>  arch/arm64/kernel/ftrace.c               |   20 +++++++++++-
>  arch/loongarch/kernel/ftrace_dyn.c       |   10 +++++-
>  arch/powerpc/kernel/trace/ftrace.c       |    2 +
>  arch/powerpc/kernel/trace/ftrace_64_pg.c |   10 ++++--
>  arch/riscv/kernel/ftrace.c               |   17 ++++++++++
>  arch/x86/kernel/ftrace.c                 |   50 +++++++++++++++++++++---------
>  include/linux/ftrace.h                   |   17 ++++++++--
>  kernel/trace/fgraph.c                    |   25 +++++++++------
>  kernel/trace/ftrace.c                    |    3 +-
>  kernel/trace/trace.h                     |    3 +-
>  kernel/trace/trace_functions_graph.c     |    3 +-
>  kernel/trace/trace_irqsoff.c             |    3 +-
>  kernel/trace/trace_sched_wakeup.c        |    3 +-
>  kernel/trace/trace_selftest.c            |    8 +++--
>  14 files changed, 129 insertions(+), 45 deletions(-)
> 
> diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
> index b2d947175cbe..a5a285f8a7ef 100644
> --- a/arch/arm64/kernel/ftrace.c
> +++ b/arch/arm64/kernel/ftrace.c
> @@ -481,7 +481,25 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent,
>  void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>  		       struct ftrace_ops *op, struct ftrace_regs *fregs)
>  {
> -	prepare_ftrace_return(ip, &arch_ftrace_regs(fregs)->lr, arch_ftrace_regs(fregs)->fp);
> +	unsigned long return_hooker = (unsigned long)&return_to_handler;
> +	unsigned long frame_pointer = arch_ftrace_regs(fregs)->fp;
> +	unsigned long *parent = &arch_ftrace_regs(fregs)->lr;
> +	unsigned long old;
> +
> +	if (unlikely(atomic_read(&current->tracing_graph_pause)))
> +		return;
> +
> +	/*
> +	 * Note:
> +	 * No protection against faulting at *parent, which may be seen
> +	 * on other archs. It's unlikely on AArch64.
> +	 */
> +	old = *parent;

Sorry to pick on this line again, but the comment is very non-committal]
and I think this is something on which we need to be definitive.

Either the access can fault, and we should handle it, or it will never
fault and we don't need to handle it. Saying it's unlikely means we
need to handle it :)

Will

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ