linux-kernel - Re: [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume

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: <CAAhV-H4AgHQs4pMqGqe7WfwCA+u7mO3U+=hcm8ZWk5DQHhsO1w@mail.gmail.com>
Date: Tue, 13 May 2025 23:13:35 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Tiezhu Yang <yangtiezhu@...ngson.cn>
Cc: loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era

Hi, Tiezhu,

On Tue, May 13, 2025 at 5:21 PM Tiezhu Yang <yangtiezhu@...ngson.cn> wrote:
>
> arch_uprobe_skip_sstep() returns true if instruction was emulated,
> that is to say, there is no need to single step for the emulated
> instructions, it will point to the destination address directly
> after the exception, so the resume_era related code is redundant,
> just remove them.
>
> Fixes: 19bc6cb64092 ("LoongArch: Add uprobes support")
> Signed-off-by: Tiezhu Yang <yangtiezhu@...ngson.cn>
> ---
>  arch/loongarch/include/asm/uprobes.h | 1 -
>  arch/loongarch/kernel/uprobes.c      | 7 +------
>  2 files changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/uprobes.h b/arch/loongarch/include/asm/uprobes.h
> index 99a0d198927f..025fc3f0a102 100644
> --- a/arch/loongarch/include/asm/uprobes.h
> +++ b/arch/loongarch/include/asm/uprobes.h
> @@ -15,7 +15,6 @@ typedef u32 uprobe_opcode_t;
>  #define UPROBE_XOLBP_INSN      __emit_break(BRK_UPROBE_XOLBP)
>
>  struct arch_uprobe {
> -       unsigned long   resume_era;
>         u32     insn[2];
>         u32     ixol[2];
>         bool    simulate;
> diff --git a/arch/loongarch/kernel/uprobes.c b/arch/loongarch/kernel/uprobes.c
> index 0ab9d8d631c4..6022eb0f71db 100644
> --- a/arch/loongarch/kernel/uprobes.c
> +++ b/arch/loongarch/kernel/uprobes.c
> @@ -52,11 +52,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
>
>         WARN_ON_ONCE(current->thread.trap_nr != UPROBE_TRAP_NR);
>         current->thread.trap_nr = utask->autask.saved_trap_nr;
> -
> -       if (auprobe->simulate)
> -               instruction_pointer_set(regs, auprobe->resume_era);
> -       else
> -               instruction_pointer_set(regs, utask->vaddr + LOONGARCH_INSN_SIZE);
> +       instruction_pointer_set(regs, utask->vaddr + LOONGARCH_INSN_SIZE);
This seems wrong. If in the simulate case, regs->csr_era has already
pointed to the correct destination address, then here we should only
handle the non-simulate case.

Huacai

>
>         return 0;
>  }
> @@ -86,7 +82,6 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
>
>         insn.word = auprobe->insn[0];
>         arch_simulate_insn(insn, regs);
> -       auprobe->resume_era = regs->csr_era;
>
>         return true;
>  }
> --
> 2.42.0
>
>