Re: [PATCH v4 4/9] rethook: x86: Add rethook x86 implementation

[Masami Hiramatsu <mhiramat@kernel.org>]

On Tue, 25 Jan 2022 01:10:09 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Add rethook for x86 implementation. Most of the code
> has been copied from kretprobes on x86.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  Changes in v4:
>   - fix stack backtrace as same as kretprobe does.
> ---
>  arch/x86/Kconfig              |    1 
>  arch/x86/include/asm/unwind.h |    4 +
>  arch/x86/kernel/Makefile      |    1 
>  arch/x86/kernel/rethook.c     |  115 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 121 insertions(+)
>  create mode 100644 arch/x86/kernel/rethook.c
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 5c2ccb85f2ef..0a7d48a63787 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -219,6 +219,7 @@ config X86
>  	select HAVE_KPROBES_ON_FTRACE
>  	select HAVE_FUNCTION_ERROR_INJECTION
>  	select HAVE_KRETPROBES
> +	select HAVE_RETHOOK
>  	select HAVE_KVM
>  	select HAVE_LIVEPATCH			if X86_64
>  	select HAVE_MIXED_BREAKPOINTS_REGS
> diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
> index 2a1f8734416d..9fe5f73f22f1 100644
> --- a/arch/x86/include/asm/unwind.h
> +++ b/arch/x86/include/asm/unwind.h
> @@ -5,6 +5,7 @@
>  #include <linux/sched.h>
>  #include <linux/ftrace.h>
>  #include <linux/kprobes.h>
> +#include <linux/rethook.h>
>  #include <asm/ptrace.h>
>  #include <asm/stacktrace.h>
>  
> @@ -107,6 +108,9 @@ static inline
>  unsigned long unwind_recover_kretprobe(struct unwind_state *state,
>  				       unsigned long addr, unsigned long *addr_p)
>  {
> +	if (IS_ENABLED(CONFIG_RETHOOK) && is_rethook_trampoline(addr))
> +		return rethook_find_ret_addr(state->task, (unsigned long)addr_p,
> +					     &state->kr_cur);

Hm, I found that this doesn't work since state->kr_cur is not defined when
CONFIG_KRETPROBES=n. Even if I define it with CONFIG_RETHOOK=y, if both
CONFIG_RETHOOK and CONFIG_KRETPROBES are 'n', the compiler caused a build
error. So I decided to use #ifdef here in the next version.

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>