linux-kernel - Re: [PATCH 03/15] x86/kprobes: Fix frame pointer annotations

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: <20190611081237.GO3436@hirez.programming.kicks-ass.net>
Date:   Tue, 11 Jun 2019 10:12:37 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     Masami Hiramatsu <mhiramat@...nel.org>, x86@...nel.org,
        linux-kernel@...r.kernel.org,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Andy Lutomirski <luto@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Jason Baron <jbaron@...mai.com>, Jiri Kosina <jkosina@...e.cz>,
        David Laight <David.Laight@...LAB.COM>,
        Borislav Petkov <bp@...en8.de>,
        Julia Cartwright <julia@...com>, Jessica Yu <jeyu@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>, Nadav Amit <namit@...are.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Edward Cree <ecree@...arflare.com>,
        Daniel Bristot de Oliveira <bristot@...hat.com>
Subject: Re: [PATCH 03/15] x86/kprobes: Fix frame pointer annotations

On Fri, Jun 07, 2019 at 09:36:02AM -0400, Josh Poimboeuf wrote:
> On Fri, Jun 07, 2019 at 10:02:10PM +0900, Masami Hiramatsu wrote:
> > On Wed, 05 Jun 2019 15:07:56 +0200
> > Peter Zijlstra <peterz@...radead.org> wrote:
> > 
> > > The kprobe trampolines have a FRAME_POINTER annotation that makes no
> > > sense. It marks the frame in the middle of pt_regs, at the place of
> > > saving BP.
> > 
> > commit ee213fc72fd67 introduced this code, and this is for unwinder which
> > uses frame pointer. I think current code stores the address of previous
> > (original context's) frame pointer into %rbp. So with that, if unwinder
> > tries to decode frame pointer, it can get the original %rbp value,
> > instead of &pt_regs from current %rbp.

The way I read that code is that we'll put the value of SP into BP at
the point where we've done 'PUSH BP', which is right in the middle of
that PUSH sequence. So while it works for a FP based unwinder, it
doesn't 'properly' identify the current frame.

> > > Change it to mark the pt_regs frame as per the ENCODE_FRAME_POINTER
> > > from the respective entry_*.S.
> > > 
> > 
> > With this change, I think stack unwinder can not get the original %rbp
> > value. Peter, could you check the above commit?
> 
> The unwinder knows how to decode the encoded frame pointer.  So it can
> find regs by decoding the new rbp value, and it also knows that regs->bp
> is the original rbp value.

Right, as Josh says the unwinder has a special case for this and it
knows these 'odd' BP values (either MSB or LSB set) indicate a pt_regs
set.