linux-kernel - Re: [PATCH -tip v7 09/13] kprobes: Setup instruction pointer in __kretprobe_trampoline

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]

Date:   Thu, 17 Jun 2021 13:21:59 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Masami Hiramatsu <mhiramat@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...nel.org>, X86 ML <x86@...nel.org>,
        Daniel Xu <dxu@...uu.xyz>,
        open list <linux-kernel@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>, Jakub Kicinski <kuba@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Kernel Team <kernel-team@...com>, Yonghong Song <yhs@...com>,
        linux-ia64@...r.kernel.org,
        Abhishek Sagar <sagar.abhishek@...il.com>
Subject: Re: [PATCH -tip v7 09/13] kprobes: Setup instruction pointer in
 __kretprobe_trampoline_handler

On Thu, Jun 17, 2021 at 10:45:41AM -0700, Andrii Nakryiko wrote:
> > > > > I know I suggested this patch, but I believe it would only be useful in
> > > > > combination with the use of UNWIND_HINT_REGS in SAVE_REGS_STRING.  But I
> > > > > think that would be tricky to pull off correctly.  Instead, we have
> > > > > UNWIND_HINT_FUNC, which is working fine.
> > > > >
> > > > > So I'd suggest dropping this patch, as the unwinder isn't actually
> > > > > reading regs->ip after all.
> > > >
> > > > ... and I guess this means patches 6-8 are no longer necessary.
> > >
> > > OK, I also confirmed that dropping those patche does not make any change
> > > on the stacktrace.
> > > Let me update the series without those.
> >
> > Oops, Andrii, can you also test the kernel without this patch?
> > (you don't need to drop patch 6-8)
> 
> Hi Masami,
> 
> Dropping this patch and leaving all the other in place breaks stack
> traces from kretprobes for BPF. I double checked with and without this
> patch. Without this patch we are back to having broken stack traces. I
> see either
> 
>   kretprobe_trampoline+0x0
> 
> or
> 
>   ftrace_trampoline+0xc8
>   kretprobe_trampoline+0x0
> 
> Is there any problem if you leave this patch as is?

Hm, I must be missing something then.  The patch is probably fine to
keep, we just may need to improve the commit log so that it makes sense
to me.

Which unwinder are you using (CONFIG_UNWINDER_*)?

-- 
Josh