linux-kernel - Re: [PATCH 2/6] uprobes/x86: Optimize is

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: <aK1veaIWBv3dZUUP@krava>
Date: Tue, 26 Aug 2025 10:25:29 +0200
From: Jiri Olsa <olsajiri@...il.com>
To: David Laight <david.laight.linux@...il.com>
Cc: Peter Zijlstra <peterz@...radead.org>, oleg@...hat.com,
	andrii@...nel.org, mhiramat@...nel.org,
	linux-kernel@...r.kernel.org, alx@...nel.org, eyal.birger@...il.com,
	kees@...nel.org, bpf@...r.kernel.org,
	linux-trace-kernel@...r.kernel.org, x86@...nel.org,
	songliubraving@...com, yhs@...com, john.fastabend@...il.com,
	haoluo@...gle.com, rostedt@...dmis.org, alan.maguire@...cle.com,
	David.Laight@...lab.com, thomas@...ch.de, mingo@...nel.org,
	rick.p.edgecombe@...el.com
Subject: Re: [PATCH 2/6] uprobes/x86: Optimize is_optimize()

On Tue, Aug 26, 2025 at 06:51:58AM +0100, David Laight wrote:
> On Thu, 21 Aug 2025 14:28:24 +0200
> Peter Zijlstra <peterz@...radead.org> wrote:
> 
> > Make is_optimized() return a tri-state and avoid return through
> > argument. This simplifies things a little.
> > 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> > ---
> >  arch/x86/kernel/uprobes.c |   34 +++++++++++++---------------------
> >  1 file changed, 13 insertions(+), 21 deletions(-)
> > 
> > --- a/arch/x86/kernel/uprobes.c
> > +++ b/arch/x86/kernel/uprobes.c
> > @@ -1047,7 +1047,7 @@ static bool __is_optimized(uprobe_opcode
> >  	return __in_uprobe_trampoline(vaddr + 5 + call->raddr);
> >  }
> >  
> > -static int is_optimized(struct mm_struct *mm, unsigned long vaddr, bool *optimized)
> > +static int is_optimized(struct mm_struct *mm, unsigned long vaddr)
> >  {
> >  	uprobe_opcode_t insn[5];
> >  	int err;
> > @@ -1055,8 +1055,7 @@ static int is_optimized(struct mm_struct
> >  	err = copy_from_vaddr(mm, vaddr, &insn, 5);
> >  	if (err)
> >  		return err;
> > -	*optimized = __is_optimized((uprobe_opcode_t *)&insn, vaddr);
> > -	return 0;
> > +	return __is_optimized((uprobe_opcode_t *)&insn, vaddr);
> >  }
> >  
> >  static bool should_optimize(struct arch_uprobe *auprobe)
> > @@ -1069,17 +1068,14 @@ int set_swbp(struct arch_uprobe *auprobe
> >  	     unsigned long vaddr)
> >  {
> >  	if (should_optimize(auprobe)) {
> > -		bool optimized = false;
> > -		int err;
> > -
> >  		/*
> >  		 * We could race with another thread that already optimized the probe,
> >  		 * so let's not overwrite it with int3 again in this case.
> >  		 */
> > -		err = is_optimized(vma->vm_mm, vaddr, &optimized);
> > -		if (err)
> > -			return err;
> > -		if (optimized)
> > +		int ret = is_optimized(vma->vm_mm, vaddr);
> > +		if (ret < 0)
> > +			return ret;
> > +		if (ret)
> >  			return 0;
> 
> Looks like you should swap over 0 and 1.
> That would then be: if (ret <= 0) return ret;

hum, but if it's not optimized (ret == 0) we need to follow up with
installing breakpoint through following uprobe_write_opcode call

also I noticed we mix int/bool return, perhaps we could do fix below

jirka


---
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 0a8c0a4a5423..853abb2a5638 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -1064,7 +1064,7 @@ static int is_optimized(struct mm_struct *mm, unsigned long vaddr)
 	err = copy_from_vaddr(mm, vaddr, &insn, 5);
 	if (err)
 		return err;
-	return __is_optimized((uprobe_opcode_t *)&insn, vaddr);
+	return __is_optimized((uprobe_opcode_t *)&insn, vaddr) ? 1 : 0;
 }
 
 static bool should_optimize(struct arch_uprobe *auprobe)