All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "Jiri Olsa" <jolsa@kernel.org>, "Oleg Nesterov" <oleg@redhat.com>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org, x86@kernel.org,
	"Song Liu" <songliubraving@fb.com>, "Yonghong Song" <yhs@fb.com>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Hao Luo" <haoluo@google.com>,
	"Steven Rostedt" <rostedt@goodmis.org>,
	"Alan Maguire" <alan.maguire@oracle.com>,
	"David Laight" <David.Laight@aculab.com>,
	"Thomas Weißschuh" <thomas@t-8ch.de>,
	"Ingo Molnar" <mingo@kernel.org>
Subject: Re: [PATCHv5 perf/core 09/22] uprobes/x86: Add uprobe syscall to speed up uprobe
Date: Mon, 14 Jul 2025 19:19:35 +0900	[thread overview]
Message-ID: <20250714191935.577ec7df5ae8a73282cddce7@kernel.org> (raw)
In-Reply-To: <20250714093903.GP905792@noisy.programming.kicks-ass.net>

On Mon, 14 Jul 2025 11:39:03 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Mon, Jul 14, 2025 at 05:39:15PM +0900, Masami Hiramatsu wrote:
> 
> > > +	/*
> > > +	 * Some of the uprobe consumers has changed sp, we can do nothing,
> > > +	 * just return via iret.
> > > +	 */
> > 
> > Do we allow consumers to change the `sp`? It seems dangerous
> > because consumer needs to know whether it is called from
> > breakpoint or syscall. Note that it has to set up ax, r11
> > and cx on the stack correctly only if it is called from syscall,
> > that is not compatible with breakpoint mode.
> > 
> > > +	if (regs->sp != sp)
> > > +		return regs->ax;
> > 
> > Shouldn't we recover regs->ip? Or in this case does consumer has
> > to change ip (== return address from trampline) too?
> > 
> > IMHO, it should not allow to change the `sp` and `ip` directly
> > in syscall mode. In case of kprobes, kprobe jump optimization
> > must be disabled explicitly (e.g. setting dummy post_handler)
> > if the handler changes `ip`.
> > 
> > Or, even if allowing to modify `sp` and `ip`, it should be helped
> > by this function, e.g. stack up the dummy regs->ax/r11/cx on the
> > new stack at the new `regs->sp`. This will allow modifying those
> > registries transparently as same as breakpoint mode.
> > In this case, I think we just need to remove above 2 lines.
> 
> There are two syscall return paths; the 'normal' is sysret and for that
> you need to undo all things just right.
> 
> The other is IRET. At which point we can have whatever state we want,
> including modified SP.
> 
> See arch/x86/entry/syscall_64.c:do_syscall_64() and
> arch/x86/entry/entry_64.S:entry_SYSCALL_64
> 
> The IRET path should return pt_regs as is from an interrupt/exception
> very much like INT3.

OK, so SYSRET case, we need to follow;

sys_uprobe -> do_syscall_64 -> entry_SYSCALL_64 -> trampoline -> retaddr

But using IRET to return, we can skip returning to trampoline,

sys_uprobe -> do_syscall_64 -> entry_SYSCALL_64 -> regs->ip

Thus we have to check the way, or in both cases use trampoline hack to
change return address.

Thank you,
-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

  reply	other threads:[~2025-07-14 10:19 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-11  8:29 [PATCHv5 perf/core 00/22] uprobes: Add support to optimize usdt probes on x86_64 Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 01/22] uprobes: Remove breakpoint in unapply_uprobe under mmap_write_lock Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 02/22] uprobes: Rename arch_uretprobe_trampoline function Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 03/22] uprobes: Make copy_from_page global Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 04/22] uprobes: Add uprobe_write function Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 05/22] uprobes: Add nbytes argument to uprobe_write Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 06/22] uprobes: Add is_register argument to uprobe_write and uprobe_write_opcode Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 07/22] uprobes: Add do_ref_ctr argument to uprobe_write function Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 08/22] uprobes/x86: Add mapping for optimized uprobe trampolines Jiri Olsa
2025-07-11 17:46   ` Oleg Nesterov
2025-07-11 19:36     ` Jiri Olsa
2025-07-14  7:23   ` Masami Hiramatsu
2025-07-11  8:29 ` [PATCHv5 perf/core 09/22] uprobes/x86: Add uprobe syscall to speed up uprobe Jiri Olsa
2025-07-14  8:39   ` Masami Hiramatsu
2025-07-14  9:28     ` Peter Zijlstra
2025-07-14 21:29       ` Jiri Olsa
2025-07-14  9:39     ` Peter Zijlstra
2025-07-14 10:19       ` Masami Hiramatsu [this message]
2025-07-14 21:28         ` Jiri Olsa
2025-07-14 23:54           ` Masami Hiramatsu
2025-07-15 12:16             ` Jiri Olsa
2025-07-16  2:39               ` Masami Hiramatsu
2025-07-11  8:29 ` [PATCHv5 perf/core 10/22] uprobes/x86: Add support to optimize uprobes Jiri Olsa
2025-07-14  9:48   ` Peter Zijlstra
2025-07-14 21:29     ` Jiri Olsa
2025-07-17 15:29       ` Jiri Olsa
2025-07-14 10:13   ` Masami Hiramatsu
2025-07-14 21:29     ` Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 11/22] selftests/bpf: Import usdt.h from libbpf/usdt project Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 12/22] selftests/bpf: Reorg the uprobe_syscall test function Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 13/22] selftests/bpf: Rename uprobe_syscall_executed prog to test_uretprobe_multi Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 14/22] selftests/bpf: Add uprobe/usdt syscall tests Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 15/22] selftests/bpf: Add hit/attach/detach race optimized uprobe test Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 16/22] selftests/bpf: Add uprobe syscall sigill signal test Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 17/22] selftests/bpf: Add optimized usdt variant for basic usdt test Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 18/22] selftests/bpf: Add uprobe_regs_equal test Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 19/22] selftests/bpf: Change test_uretprobe_regs_change for uprobe and uretprobe Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 20/22] seccomp: passthrough uprobe systemcall without filtering Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 perf/core 21/22] selftests/seccomp: validate uprobe syscall passes through seccomp Jiri Olsa
2025-07-11  8:29 ` [PATCHv5 22/22] man2: Add uprobe syscall page Jiri Olsa
2025-07-14 14:04   ` Masami Hiramatsu
2025-07-11 17:17 ` [PATCHv5 perf/core 00/22] uprobes: Add support to optimize usdt probes on x86_64 Andrii Nakryiko

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=20250714191935.577ec7df5ae8a73282cddce7@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=David.Laight@aculab.com \
    --cc=alan.maguire@oracle.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=songliubraving@fb.com \
    --cc=thomas@t-8ch.de \
    --cc=x86@kernel.org \
    --cc=yhs@fb.com \
    /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.