All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	Chen Zhongjin <chenzhongjin@huawei.com>,
	"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
	Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH 2/2] x86/entry: Fix unwinding from kprobe on PUSH/POP instruction
Date: Wed, 15 Feb 2023 11:25:54 +0100	[thread overview]
Message-ID: <Y+yzMmL7gUprDru3@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20230214170552.glhdytvunczyxxao@treble>

On Tue, Feb 14, 2023 at 09:05:52AM -0800, Josh Poimboeuf wrote:
> On Tue, Feb 14, 2023 at 12:35:04PM +0100, Peter Zijlstra wrote:
> > On Mon, Feb 13, 2023 at 11:43:57PM +0900, Masami Hiramatsu wrote:
> > 
> > > > Fix it by annotating the #BP exception as a non-signal stack frame,
> > > > which tells the ORC unwinder to decrement the instruction pointer before
> > > > looking up the corresponding ORC entry.
> > > 
> > > Just to make it clear, this sounds like a 'hack' use of non-signal stack
> > > frame. If so, can we change the flag name as 'literal' or 'non-literal' etc?
> > > I concern that the 'signal' flag is used differently in the future.
> 
> Agreed, though I'm having trouble coming up with a succinct yet
> scrutable name.  If length wasn't an issue it would be something like
> 
>   "decrement_return_address_when_looking_up_the_next_orc_entry"
> 
> > Oooh, bike-shed :-) Let me suggest trap=1, where a trap is a fault with
> > a different return address, specifically the instruction after the
> > faulting instruction.
> 
> I think "trap" doesn't work because
> 
>  1) It's more than just traps, it's also function calls.  We have
>     traps/calls in one bucket (decrement IP); and everything else
>     (faults, aborts, irqs) in the other (don't decrement IP).
> 
>  2) It's not necessarily all traps which need the flag, just those that
>     affect a previously-but-now-overwritten stack-modifying instruction.
>     So #OF (which we don't use?) and trap-class #DB don't seem to be
>     affected.  In practice maybe this distinction doesn't matter, but
>     for example there's no reason for ORC try to distinguish trap #DB
>     from non-trap #DB at runtime.

Well, I was specifically thinking about #DB, why don't we need to
decrement when we put a hardware breakpoint on a stack modifying op?

  reply	other threads:[~2023-02-15 10:26 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-10 22:42 [PATCH 0/2] x86/unwind/orc: Fix unwinding from kprobe on PUSH/POP instruction Josh Poimboeuf
2023-02-10 22:42 ` [PATCH 1/2] x86/unwind/orc: Add 'signal' field to ORC metadata Josh Poimboeuf
2023-02-11 11:52   ` [tip: objtool/core] " tip-bot2 for Josh Poimboeuf
2023-02-10 22:42 ` [PATCH 2/2] x86/entry: Fix unwinding from kprobe on PUSH/POP instruction Josh Poimboeuf
2023-02-11 11:52   ` [tip: objtool/core] " tip-bot2 for Josh Poimboeuf
2023-02-13 14:43   ` [PATCH 2/2] " Masami Hiramatsu
2023-02-14 11:35     ` Peter Zijlstra
2023-02-14 17:05       ` Josh Poimboeuf
2023-02-15 10:25         ` Peter Zijlstra [this message]
2023-02-15 23:16           ` Josh Poimboeuf
2023-02-16 10:46             ` Peter Zijlstra
2023-02-16 11:30               ` Peter Zijlstra
2023-02-16 14:35                 ` Masami Hiramatsu
2023-02-16 16:58                   ` Josh Poimboeuf
2023-02-16 11:58   ` Peter Zijlstra
2023-02-16 16:06     ` Josh Poimboeuf
2023-02-17 12:44       ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y+yzMmL7gUprDru3@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=anil.s.keshavamurthy@intel.com \
    --cc=chenzhongjin@huawei.com \
    --cc=davem@davemloft.net \
    --cc=jpoimboe@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=naveen.n.rao@linux.ibm.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.