linux-kernel - Re: nop5-optimized USDTs WAS: Re: [PATCHv6 perf/core 09/22] uprobes/x86: Add uprobe syscall to speed up uprobe

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: <20250904215617.GR3245006@noisy.programming.kicks-ass.net>
Date: Thu, 4 Sep 2025 23:56:17 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Jiri Olsa <olsajiri@...il.com>, Oleg Nesterov <oleg@...hat.com>,
	Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>,
	Linux trace kernel <linux-trace-kernel@...r.kernel.org>,
	X86 ML <x86@...nel.org>, Song Liu <songliubraving@...com>,
	Yonghong Song <yhs@...com>,
	John Fastabend <john.fastabend@...il.com>,
	Hao Luo <haoluo@...gle.com>, Steven Rostedt <rostedt@...dmis.org>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Alan Maguire <alan.maguire@...cle.com>,
	David Laight <David.Laight@...lab.com>,
	Thomas Weißschuh <thomas@...ch.de>,
	Ingo Molnar <mingo@...nel.org>
Subject: Re: nop5-optimized USDTs WAS: Re: [PATCHv6 perf/core 09/22]
 uprobes/x86: Add uprobe syscall to speed up uprobe

On Thu, Sep 04, 2025 at 02:44:03PM -0700, Andrii Nakryiko wrote:
> On Thu, Sep 4, 2025 at 1:52 PM Peter Zijlstra <peterz@...radead.org> wrote:
> >
> > On Thu, Sep 04, 2025 at 01:49:49PM -0700, Andrii Nakryiko wrote:
> > > On Thu, Sep 4, 2025 at 1:35 PM Peter Zijlstra <peterz@...radead.org> wrote:
> > > >
> > > > On Thu, Sep 04, 2025 at 11:27:45AM -0700, Andrii Nakryiko wrote:
> > > >
> > > > > > > So I've been thinking what's the simplest and most reliable way to
> > > > > > > feature-detect support for this sys_uprobe (e.g., for libbpf to know
> > > > > > > whether we should attach at nop5 vs nop1), and clearly that would be
> > > > > >
> > > > > > wrt nop5/nop1.. so the idea is to have USDT macro emit both nop1,nop5
> > > > > > and store some info about that in the usdt's elf note, right?
> > > >
> > > > Wait, what? You're doing to emit 6 bytes and two nops? Why? Surely the
> > > > old kernel can INT3 on top of a NOP5?
> > > >
> > >
> > > Yes it can, but it's 2x slower in terms of uprobe triggering compared
> > > to nop1.
> >
> > Why? That doesn't really make sense.
> >
> 
> Of course it's silly... It's because nop5 wasn't recognized as one of
> the emulated instructions, so was handled through single-stepping.

*groan*

> > I realize its probably to late to fix the old kernel not to be stupid --
> > this must be something stupid, right? But now I need to know.
> 
> Jiri fixed this, but as you said, too late for old kernels. See [0]
> for the patch that landed not so long ago.
> 
>   [0] https://lore.kernel.org/all/20250414083647.1234007-1-jolsa@kernel.org/

Ooh, that suggests we do something like so:


diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 0a8c0a4a5423..223f8925097b 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -309,6 +309,29 @@ static int uprobe_init_insn(struct arch_uprobe *auprobe, struct insn *insn, bool
 	return -ENOTSUPP;
 }
 
+static bool insn_is_nop(struct insn *insn)
+{
+	return insn->opcode.nbytes == 1 && insn->opcode.bytes[0] == 0x90;
+}
+
+static bool insn_is_nopl(struct insn *insn)
+{
+	if (insn->opcode.nbytes != 2)
+		return false;
+
+	if (insn->opcode.bytes[0] != 0x0f || insn->opcode.bytes[1] != 0x1f)
+		return false;
+
+	if (!insn->modrm.nbytes)
+		return false;
+
+	if (X86_MODRM_REG(insn->modrm.bytes[0]) != 0)
+		return false;
+
+	/* 0f 1f /0 - NOPL */
+	return true;
+}
+
 #ifdef CONFIG_X86_64
 
 struct uretprobe_syscall_args {
@@ -1158,29 +1181,6 @@ void arch_uprobe_optimize(struct arch_uprobe *auprobe, unsigned long vaddr)
 	mmap_write_unlock(mm);
 }
 
-static bool insn_is_nop(struct insn *insn)
-{
-	return insn->opcode.nbytes == 1 && insn->opcode.bytes[0] == 0x90;
-}
-
-static bool insn_is_nopl(struct insn *insn)
-{
-	if (insn->opcode.nbytes != 2)
-		return false;
-
-	if (insn->opcode.bytes[0] != 0x0f || insn->opcode.bytes[1] != 0x1f)
-		return false;
-
-	if (!insn->modrm.nbytes)
-		return false;
-
-	if (X86_MODRM_REG(insn->modrm.bytes[0]) != 0)
-		return false;
-
-	/* 0f 1f /0 - NOPL */
-	return true;
-}
-
 static bool can_optimize(struct insn *insn, unsigned long vaddr)
 {
 	if (!insn->x86_64 || insn->length != 5)
@@ -1428,17 +1428,13 @@ static int branch_setup_xol_ops(struct arch_uprobe *auprobe, struct insn *insn)
 	insn_byte_t p;
 	int i;
 
-	/* x86_nops[insn->length]; same as jmp with .offs = 0 */
-	if (insn->length <= ASM_NOP_MAX &&
-	    !memcmp(insn->kaddr, x86_nops[insn->length], insn->length))
+	if (insn_is_nop(insn) || insn_is_nopl(insn))
 		goto setup;
 
 	switch (opc1) {
 	case 0xeb:	/* jmp 8 */
 	case 0xe9:	/* jmp 32 */
 		break;
-	case 0x90:	/* prefix* + nop; same as jmp with .offs = 0 */
-		goto setup;
 
 	case 0xe8:	/* call relative */
 		branch_clear_offset(auprobe, insn);