netdev - Re: [PATCH 7/9] x86/unwind/orc: Fall back to using frame pointers for generated code

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: <20190613220054.tmonrgfdeie2kl74@ast-mbp.dhcp.thefacebook.com>
Date:   Thu, 13 Jun 2019 15:00:55 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
        bpf@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>,
        Song Liu <songliubraving@...com>,
        Kairui Song <kasong@...hat.com>
Subject: Re: [PATCH 7/9] x86/unwind/orc: Fall back to using frame pointers
 for generated code

On Thu, Jun 13, 2019 at 08:21:04AM -0500, Josh Poimboeuf wrote:
> The ORC unwinder can't unwind through BPF JIT generated code because
> there are no ORC entries associated with the code.
> 
> If an ORC entry isn't available, try to fall back to frame pointers.  If
> BPF and other generated code always do frame pointer setup (even with
> CONFIG_FRAME_POINTERS=n) then this will allow ORC to unwind through most
> generated code despite there being no corresponding ORC entries.
> 
> Fixes: d15d356887e7 ("perf/x86: Make perf callchains work without CONFIG_FRAME_POINTER")
> Reported-by: Song Liu <songliubraving@...com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
> ---
>  arch/x86/kernel/unwind_orc.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
> index 33b66b5c5aec..72b997eaa1fc 100644
> --- a/arch/x86/kernel/unwind_orc.c
> +++ b/arch/x86/kernel/unwind_orc.c
> @@ -82,9 +82,9 @@ static struct orc_entry *orc_find(unsigned long ip);
>   * But they are copies of the ftrace entries that are static and
>   * defined in ftrace_*.S, which do have orc entries.
>   *
> - * If the undwinder comes across a ftrace trampoline, then find the
> + * If the unwinder comes across a ftrace trampoline, then find the
>   * ftrace function that was used to create it, and use that ftrace
> - * function's orc entrie, as the placement of the return code in
> + * function's orc entry, as the placement of the return code in
>   * the stack will be identical.
>   */
>  static struct orc_entry *orc_ftrace_find(unsigned long ip)
> @@ -128,6 +128,16 @@ static struct orc_entry null_orc_entry = {
>  	.type = ORC_TYPE_CALL
>  };
>  
> +/* Fake frame pointer entry -- used as a fallback for generated code */
> +static struct orc_entry orc_fp_entry = {
> +	.type		= ORC_TYPE_CALL,
> +	.sp_reg		= ORC_REG_BP,
> +	.sp_offset	= 16,
> +	.bp_reg		= ORC_REG_PREV_SP,
> +	.bp_offset	= -16,
> +	.end		= 0,
> +};
> +
>  static struct orc_entry *orc_find(unsigned long ip)
>  {
>  	static struct orc_entry *orc;
> @@ -392,8 +402,16 @@ bool unwind_next_frame(struct unwind_state *state)
>  	 * calls and calls to noreturn functions.
>  	 */
>  	orc = orc_find(state->signal ? state->ip : state->ip - 1);
> -	if (!orc)
> -		goto err;
> +	if (!orc) {
> +		/*
> +		 * As a fallback, try to assume this code uses a frame pointer.
> +		 * This is useful for generated code, like BPF, which ORC
> +		 * doesn't know about.  This is just a guess, so the rest of
> +		 * the unwind is no longer considered reliable.
> +		 */
> +		orc = &orc_fp_entry;
> +		state->error = true;

That seems fragile.
Can't we populate orc_unwind tables after JIT ?