Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH v3] LoongArch/rethook: Replace kretprobe with rethook on
 LoongArch
To:     Haoran Jiang <jianghaoran@kylinos.cn>
References: <20230613011439.59168-1-jianghaoran@kylinos.cn>
 <20230613060329.356201-1-jianghaoran@kylinos.cn>
Cc:     chenhuacai@kernel.org, conor.dooley@microchip.com,
        hejinyang@loongson.cn, jhrhhao@163.com, kernel@xen0n.name,
        l3b2w1@gmail.com, linux-kernel@vger.kernel.org,
        loongarch@lists.linux.dev, masahiroy@kernel.org,
        mhiramat@kernel.org, palmer@rivosinc.com, tangyouling@loongson.cn,
        zhangqing@loongson.cn
From:   Tiezhu Yang <yangtiezhu@loongson.cn>
Message-ID: <8ffd58dd-2e0c-0078-5e3f-a1139f05720b@loongson.cn>
Date:   Tue, 13 Jun 2023 16:04:21 +0800
User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <20230613060329.356201-1-jianghaoran@kylinos.cn>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 06/13/2023 02:03 PM, Haoran Jiang wrote:
> That's an adaptation of commit f3a112c0c40d ("x86,rethook,kprobes:
> Replace kretprobe with rethook on x86")and commit b57c2f124098
> ("riscv: add riscv rethook implementation") to LoongArch.
> Mainly refer to this commit
> b57c2f124098 ("riscv: add riscv rethook implementation").
>
> Replaces the kretprobe code with rethook on LoongArch. With this patch,
> kretprobe on LoongArch uses the rethook instead of kretprobe specific
> trampoline code.

Maybe Huacai can refine the commit message.

>
> Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>

...

> diff --git a/arch/loongarch/kernel/kprobes.c b/arch/loongarch/kernel/kprobes.c
> index 56c8c4b09a42..dbce23ba9970 100644
> --- a/arch/loongarch/kernel/kprobes.c
> +++ b/arch/loongarch/kernel/kprobes.c
> @@ -378,26 +378,6 @@ int __init arch_init_kprobes(void)
>  	return 0;
>  }
>
> -/* ASM function that handles the kretprobes must not be probed */
> -NOKPROBE_SYMBOL(__kretprobe_trampoline);
> -
> -/* Called from __kretprobe_trampoline */
> -void __used *trampoline_probe_handler(struct pt_regs *regs)
> -{
> -	return (void *)kretprobe_trampoline_handler(regs, NULL);
> -}
> -NOKPROBE_SYMBOL(trampoline_probe_handler);
> -
> -void arch_prepare_kretprobe(struct kretprobe_instance *ri,
> -			    struct pt_regs *regs)
> -{
> -	ri->ret_addr = (kprobe_opcode_t *)regs->regs[1];
> -	ri->fp = NULL;
> -
> -	/* Replace the return addr with trampoline addr */
> -	regs->regs[1] = (unsigned long)&__kretprobe_trampoline;
> -}
> -NOKPROBE_SYMBOL(arch_prepare_kretprobe);
>

Please remove the blank line.

>  int arch_trampoline_kprobe(struct kprobe *p)
>  {
> diff --git a/arch/loongarch/kernel/rethook.c b/arch/loongarch/kernel/rethook.c
> new file mode 100644
> index 000000000000..b5e7f62f7dea
> --- /dev/null
> +++ b/arch/loongarch/kernel/rethook.c
> @@ -0,0 +1,27 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Generic return hook for LoongArch.
> + */
> +
> +#include <linux/kprobes.h>
> +#include <linux/rethook.h>
> +#include "rethook.h"
> +
> +/* This is called from arch_rethook_trampoline() */
> +unsigned long __used arch_rethook_trampoline_callback(struct pt_regs *regs)
> +{
> +	return rethook_trampoline_handler(regs, 0);
> +}
> +

Ditto

> +NOKPROBE_SYMBOL(arch_rethook_trampoline_callback);
> +
> +void arch_rethook_prepare(struct rethook_node *rhn, struct pt_regs *regs, bool mcount)
> +{
> +	rhn->ret_addr = regs->regs[1];
> +	rhn->frame = 0;
> +
> +	/* replace return addr with trampoline */
> +	regs->regs[1] = (unsigned long)arch_rethook_trampoline;
> +}
> +

Ditto

> +NOKPROBE_SYMBOL(arch_rethook_prepare);

Like s390 and x86, please add:

/* assembler function that handles the rethook must not be probed itself */
NOKPROBE_SYMBOL(arch_rethook_trampoline);

> diff --git a/arch/loongarch/kernel/rethook.h b/arch/loongarch/kernel/rethook.h

...

> -SYM_CODE_END(__kretprobe_trampoline)
> +SYM_CODE_END(arch_rethook_trampoline)

Thanks,
Tiezhu