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: <20250826065158.1b7ad5fc@pumpkin>
Date: Tue, 26 Aug 2025 06:51:58 +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 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;

	David

>  	}
>  	return uprobe_write_opcode(auprobe, vma, vaddr, UPROBE_SWBP_INSN,
> @@ -1090,17 +1086,13 @@ int set_orig_insn(struct arch_uprobe *au
>  		  unsigned long vaddr)
>  {
>  	if (test_bit(ARCH_UPROBE_FLAG_CAN_OPTIMIZE, &auprobe->flags)) {
> -		struct mm_struct *mm = vma->vm_mm;
> -		bool optimized = false;
> -		int err;
> -
> -		err = is_optimized(mm, vaddr, &optimized);
> -		if (err)
> -			return err;
> -		if (optimized) {
> -			err = swbp_unoptimize(auprobe, vma, vaddr);
> -			WARN_ON_ONCE(err);
> -			return err;
> +		int ret = is_optimized(vma->vm_mm, vaddr);
> +		if (ret < 0)
> +			return ret;
> +		if (ret) {
> +			ret = swbp_unoptimize(auprobe, vma, vaddr);
> +			WARN_ON_ONCE(ret);
> +			return ret;
>  		}
>  	}
>  	return uprobe_write_opcode(auprobe, vma, vaddr, *(uprobe_opcode_t *)&auprobe->insn,
> 
> 
>