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] [day] [month] [year] [list]

Message-ID: <20250827203240.664a030c@pumpkin>
Date: Wed, 27 Aug 2025 20:32:40 +0100
From: David Laight <david.laight.linux@...il.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: jolsa@...nel.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, 26 Aug 2025 10:18:40 +0200
Peter Zijlstra <peterz@...radead.org> wrote:

> On Tue, Aug 26, 2025 at 06:51:58AM +0100, David Laight wrote:
> 
> > > @@ -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;  
> 
> I considered that, but that was actually more confusing. Yes the return
> check is neat, but urgh.
> 
> The tri-state return is: 
> 
> <0 -- error
>  0 -- false
>  1 -- true
> 
> and that is converted to the 'normal' convention:
> 
> <0 -- error
>  0 -- success
> 
> 
> Making that intermediate:
> 
> <0 -- error
>  0 -- true
>  1 -- false
> 
> is just asking for trouble later.

I'm sure the function name could be changed to make it all work :-)

	David