Re: [PATCH v2 1/2] ftrace: Make ftrace_regs abstract from direct use

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 08 Oct 2024 19:05:28 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> From: Steven Rostedt <rostedt@xxxxxxxxxxx>
> 
> ftrace_regs was created to hold registers that store information to save
> function parameters, return value and stack. Since it is a subset of
> pt_regs, it should only be used by its accessor functions. But because
> pt_regs can easily be taken from ftrace_regs (on most archs), it is
> tempting to use it directly. But when running on other architectures, it
> may fail to build or worse, build but crash the kernel!
> 
> Instead, make struct ftrace_regs an empty structure and have the
> architectures define __arch_ftrace_regs and all the accessor functions
> will typecast to it to get to the actual fields. This will help avoid
> usage of ftrace_regs directly.
> 
> Link: https://lore.kernel.org/all/20241007171027.629bdafd@xxxxxxxxxxxxxxxxxx/
> 

This looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

For x86 and generic part.

Thank you,

> Acked-by: Catalin Marinas <catalin.marinas@xxxxxxx>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
> Changes since v1: https://lore.kernel.org/20241007204743.41314f1d@xxxxxxxxxxxxxxxxxx
> 
> - Moved the non ftrace args code from asm-generic/ftrace.h to linux/ftrace.h
>   those archs have their own asm/ftrace.h and are not using asm-generic.
>   The default has to be in linux/ftrace.h
> 
> - simplified arch_ftrace_get_regs() and made it a static inline function
> 
>  arch/arm64/include/asm/ftrace.h          | 20 +++++++++--------
>  arch/arm64/kernel/asm-offsets.c          | 22 +++++++++----------
>  arch/arm64/kernel/ftrace.c               | 10 ++++-----
>  arch/loongarch/include/asm/ftrace.h      | 22 ++++++++++---------
>  arch/loongarch/kernel/ftrace_dyn.c       |  2 +-
>  arch/powerpc/include/asm/ftrace.h        | 21 ++++++++++--------
>  arch/powerpc/kernel/trace/ftrace.c       |  4 ++--
>  arch/powerpc/kernel/trace/ftrace_64_pg.c |  2 +-
>  arch/riscv/include/asm/ftrace.h          | 21 ++++++++++--------
>  arch/riscv/kernel/asm-offsets.c          | 28 ++++++++++++------------
>  arch/riscv/kernel/ftrace.c               |  2 +-
>  arch/s390/include/asm/ftrace.h           | 23 ++++++++++---------
>  arch/s390/kernel/asm-offsets.c           |  4 ++--
>  arch/s390/kernel/ftrace.c                |  2 +-
>  arch/s390/lib/test_unwind.c              |  4 ++--
>  arch/x86/include/asm/ftrace.h            | 25 +++++++++++----------
>  arch/x86/kernel/ftrace.c                 |  2 +-
>  include/linux/ftrace.h                   | 21 +++++++++++++++---
>  kernel/trace/ftrace.c                    |  2 +-
>  19 files changed, 134 insertions(+), 103 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
> index dc9cf0bd2a4c..bbb69c7751b9 100644
> --- a/arch/arm64/include/asm/ftrace.h
> +++ b/arch/arm64/include/asm/ftrace.h
> @@ -56,6 +56,8 @@ unsigned long ftrace_call_adjust(unsigned long addr);
>  #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
>  struct dyn_ftrace;
>  struct ftrace_ops;
> +struct ftrace_regs;
> +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs))
>  
>  #define arch_ftrace_get_regs(regs) NULL
>  
> @@ -63,7 +65,7 @@ struct ftrace_ops;
>   * Note: sizeof(struct ftrace_regs) must be a multiple of 16 to ensure correct
>   * stack alignment
>   */
> -struct ftrace_regs {
> +struct __arch_ftrace_regs {
>  	/* x0 - x8 */
>  	unsigned long regs[9];
>  
> @@ -83,47 +85,47 @@ struct ftrace_regs {
>  static __always_inline unsigned long
>  ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
>  {
> -	return fregs->pc;
> +	return arch_ftrace_regs(fregs)->pc;
>  }
>  
>  static __always_inline void
>  ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
>  				    unsigned long pc)
>  {
> -	fregs->pc = pc;
> +	arch_ftrace_regs(fregs)->pc = pc;
>  }
>  
>  static __always_inline unsigned long
>  ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
>  {
> -	return fregs->sp;
> +	return arch_ftrace_regs(fregs)->sp;
>  }
>  
>  static __always_inline unsigned long
>  ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
>  {
>  	if (n < 8)
> -		return fregs->regs[n];
> +		return arch_ftrace_regs(fregs)->regs[n];
>  	return 0;
>  }
>  
>  static __always_inline unsigned long
>  ftrace_regs_get_return_value(const struct ftrace_regs *fregs)
>  {
> -	return fregs->regs[0];
> +	return arch_ftrace_regs(fregs)->regs[0];
>  }
>  
>  static __always_inline void
>  ftrace_regs_set_return_value(struct ftrace_regs *fregs,
>  			     unsigned long ret)
>  {
> -	fregs->regs[0] = ret;
> +	arch_ftrace_regs(fregs)->regs[0] = ret;
>  }
>  
>  static __always_inline void
>  ftrace_override_function_with_return(struct ftrace_regs *fregs)
>  {
> -	fregs->pc = fregs->lr;
> +	arch_ftrace_regs(fregs)->pc = arch_ftrace_regs(fregs)->lr;
>  }
>  
>  int ftrace_regs_query_register_offset(const char *name);
> @@ -143,7 +145,7 @@ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs,
>  	 * The ftrace trampoline will return to this address instead of the
>  	 * instrumented function.
>  	 */
> -	fregs->direct_tramp = addr;
> +	arch_ftrace_regs(fregs)->direct_tramp = addr;
>  }
>  #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
>  
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index 27de1dddb0ab..a5de57f68219 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -84,19 +84,19 @@ int main(void)
>    DEFINE(PT_REGS_SIZE,		sizeof(struct pt_regs));
>    BLANK();
>  #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> -  DEFINE(FREGS_X0,		offsetof(struct ftrace_regs, regs[0]));
> -  DEFINE(FREGS_X2,		offsetof(struct ftrace_regs, regs[2]));
> -  DEFINE(FREGS_X4,		offsetof(struct ftrace_regs, regs[4]));
> -  DEFINE(FREGS_X6,		offsetof(struct ftrace_regs, regs[6]));
> -  DEFINE(FREGS_X8,		offsetof(struct ftrace_regs, regs[8]));
> -  DEFINE(FREGS_FP,		offsetof(struct ftrace_regs, fp));
> -  DEFINE(FREGS_LR,		offsetof(struct ftrace_regs, lr));
> -  DEFINE(FREGS_SP,		offsetof(struct ftrace_regs, sp));
> -  DEFINE(FREGS_PC,		offsetof(struct ftrace_regs, pc));
> +  DEFINE(FREGS_X0,		offsetof(struct __arch_ftrace_regs, regs[0]));
> +  DEFINE(FREGS_X2,		offsetof(struct __arch_ftrace_regs, regs[2]));
> +  DEFINE(FREGS_X4,		offsetof(struct __arch_ftrace_regs, regs[4]));
> +  DEFINE(FREGS_X6,		offsetof(struct __arch_ftrace_regs, regs[6]));
> +  DEFINE(FREGS_X8,		offsetof(struct __arch_ftrace_regs, regs[8]));
> +  DEFINE(FREGS_FP,		offsetof(struct __arch_ftrace_regs, fp));
> +  DEFINE(FREGS_LR,		offsetof(struct __arch_ftrace_regs, lr));
> +  DEFINE(FREGS_SP,		offsetof(struct __arch_ftrace_regs, sp));
> +  DEFINE(FREGS_PC,		offsetof(struct __arch_ftrace_regs, pc));
>  #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> -  DEFINE(FREGS_DIRECT_TRAMP,	offsetof(struct ftrace_regs, direct_tramp));
> +  DEFINE(FREGS_DIRECT_TRAMP,	offsetof(struct __arch_ftrace_regs, direct_tramp));
>  #endif
> -  DEFINE(FREGS_SIZE,		sizeof(struct ftrace_regs));
> +  DEFINE(FREGS_SIZE,		sizeof(struct __arch_ftrace_regs));
>    BLANK();
>  #endif
>  #ifdef CONFIG_COMPAT
> diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
> index a650f5e11fc5..b2d947175cbe 100644
> --- a/arch/arm64/kernel/ftrace.c
> +++ b/arch/arm64/kernel/ftrace.c
> @@ -23,10 +23,10 @@ struct fregs_offset {
>  	int offset;
>  };
>  
> -#define FREGS_OFFSET(n, field)				\
> -{							\
> -	.name = n,					\
> -	.offset = offsetof(struct ftrace_regs, field),	\
> +#define FREGS_OFFSET(n, field)					\
> +{								\
> +	.name = n,						\
> +	.offset = offsetof(struct __arch_ftrace_regs, field),	\
>  }
>  
>  static const struct fregs_offset fregs_offsets[] = {
> @@ -481,7 +481,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent,
>  void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>  		       struct ftrace_ops *op, struct ftrace_regs *fregs)
>  {
> -	prepare_ftrace_return(ip, &fregs->lr, fregs->fp);
> +	prepare_ftrace_return(ip, &arch_ftrace_regs(fregs)->lr, arch_ftrace_regs(fregs)->fp);
>  }
>  #else
>  /*
> diff --git a/arch/loongarch/include/asm/ftrace.h b/arch/loongarch/include/asm/ftrace.h
> index c0a682808e07..0e15d36ce251 100644
> --- a/arch/loongarch/include/asm/ftrace.h
> +++ b/arch/loongarch/include/asm/ftrace.h
> @@ -43,38 +43,40 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent);
>  
>  #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
>  struct ftrace_ops;
> +struct ftrace_regs;
> +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs))
>  
> -struct ftrace_regs {
> +struct __arch_ftrace_regs {
>  	struct pt_regs regs;
>  };
>  
>  static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs)
>  {
> -	return &fregs->regs;
> +	return &arch_ftrace_regs(fregs)->regs;
>  }
>  
>  static __always_inline unsigned long
>  ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs)
>  {
> -	return instruction_pointer(&fregs->regs);
> +	return instruction_pointer(&arch_ftrace_regs(fregs)->regs);
>  }
>  
>  static __always_inline void
>  ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip)
>  {
> -	instruction_pointer_set(&fregs->regs, ip);
> +	instruction_pointer_set(&arch_ftrace_regs(fregs)->regs, ip);
>  }
>  
>  #define ftrace_regs_get_argument(fregs, n) \
> -	regs_get_kernel_argument(&(fregs)->regs, n)
> +	regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n)
>  #define ftrace_regs_get_stack_pointer(fregs) \
> -	kernel_stack_pointer(&(fregs)->regs)
> +	kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs)
>  #define ftrace_regs_return_value(fregs) \
> -	regs_return_value(&(fregs)->regs)
> +	regs_return_value(&arch_ftrace_regs(fregs)->regs)
>  #define ftrace_regs_set_return_value(fregs, ret) \
> -	regs_set_return_value(&(fregs)->regs, ret)
> +	regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret)
>  #define ftrace_override_function_with_return(fregs) \
> -	override_function_with_return(&(fregs)->regs)
> +	override_function_with_return(&arch_ftrace_regs(fregs)->regs)
>  #define ftrace_regs_query_register_offset(name) \
>  	regs_query_register_offset(name)
>  
> @@ -90,7 +92,7 @@ __arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
>  }
>  
>  #define arch_ftrace_set_direct_caller(fregs, addr) \
> -	__arch_ftrace_set_direct_caller(&(fregs)->regs, addr)
> +	__arch_ftrace_set_direct_caller(&arch_ftrace_regs(fregs)->regs, addr)
>  #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
>  
>  #endif
> diff --git a/arch/loongarch/kernel/ftrace_dyn.c b/arch/loongarch/kernel/ftrace_dyn.c
> index bff058317062..18056229e22e 100644
> --- a/arch/loongarch/kernel/ftrace_dyn.c
> +++ b/arch/loongarch/kernel/ftrace_dyn.c
> @@ -241,7 +241,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent)
>  void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>  		       struct ftrace_ops *op, struct ftrace_regs *fregs)
>  {
> -	struct pt_regs *regs = &fregs->regs;
> +	struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs;
>  	unsigned long *parent = (unsigned long *)&regs->regs[1];
>  
>  	prepare_ftrace_return(ip, (unsigned long *)parent);
> diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
> index 559560286e6d..e299fd47d201 100644
> --- a/arch/powerpc/include/asm/ftrace.h
> +++ b/arch/powerpc/include/asm/ftrace.h
> @@ -32,39 +32,42 @@ struct dyn_arch_ftrace {
>  int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec);
>  #define ftrace_init_nop ftrace_init_nop
>  
> -struct ftrace_regs {
> +struct ftrace_regs;
> +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs))
> +
> +struct __arch_ftrace_regs {
>  	struct pt_regs regs;
>  };
>  
>  static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs)
>  {
>  	/* We clear regs.msr in ftrace_call */
> -	return fregs->regs.msr ? &fregs->regs : NULL;
> +	return arch_ftrace_regs(fregs)->regs.msr ? &arch_ftrace_regs(fregs)->regs : NULL;
>  }
>  
>  static __always_inline void
>  ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
>  				    unsigned long ip)
>  {
> -	regs_set_return_ip(&fregs->regs, ip);
> +	regs_set_return_ip(&arch_ftrace_regs(fregs)->regs, ip);
>  }
>  
>  static __always_inline unsigned long
>  ftrace_regs_get_instruction_pointer(struct ftrace_regs *fregs)
>  {
> -	return instruction_pointer(&fregs->regs);
> +	return instruction_pointer(&arch_ftrace_regs(fregs)->regs);
>  }
>  
>  #define ftrace_regs_get_argument(fregs, n) \
> -	regs_get_kernel_argument(&(fregs)->regs, n)
> +	regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n)
>  #define ftrace_regs_get_stack_pointer(fregs) \
> -	kernel_stack_pointer(&(fregs)->regs)
> +	kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs)
>  #define ftrace_regs_return_value(fregs) \
> -	regs_return_value(&(fregs)->regs)
> +	regs_return_value(&arch_ftrace_regs(fregs)->regs)
>  #define ftrace_regs_set_return_value(fregs, ret) \
> -	regs_set_return_value(&(fregs)->regs, ret)
> +	regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret)
>  #define ftrace_override_function_with_return(fregs) \
> -	override_function_with_return(&(fregs)->regs)
> +	override_function_with_return(&arch_ftrace_regs(fregs)->regs)
>  #define ftrace_regs_query_register_offset(name) \
>  	regs_query_register_offset(name)
>  
> diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
> index d8d6b4fd9a14..df41f4a7c738 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -421,7 +421,7 @@ int __init ftrace_dyn_arch_init(void)
>  void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>  		       struct ftrace_ops *op, struct ftrace_regs *fregs)
>  {
> -	unsigned long sp = fregs->regs.gpr[1];
> +	unsigned long sp = arch_ftrace_regs(fregs)->regs.gpr[1];
>  	int bit;
>  
>  	if (unlikely(ftrace_graph_is_dead()))
> @@ -439,6 +439,6 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>  
>  	ftrace_test_recursion_unlock(bit);
>  out:
> -	fregs->regs.link = parent_ip;
> +	arch_ftrace_regs(fregs)->regs.link = parent_ip;
>  }
>  #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
> diff --git a/arch/powerpc/kernel/trace/ftrace_64_pg.c b/arch/powerpc/kernel/trace/ftrace_64_pg.c
> index 12fab1803bcf..d3c5552e4984 100644
> --- a/arch/powerpc/kernel/trace/ftrace_64_pg.c
> +++ b/arch/powerpc/kernel/trace/ftrace_64_pg.c
> @@ -829,7 +829,7 @@ __prepare_ftrace_return(unsigned long parent, unsigned long ip, unsigned long sp
>  void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>  		       struct ftrace_ops *op, struct ftrace_regs *fregs)
>  {
> -	fregs->regs.link = __prepare_ftrace_return(parent_ip, ip, fregs->regs.gpr[1]);
> +	arch_ftrace_regs(fregs)->regs.link = __prepare_ftrace_return(parent_ip, ip, arch_ftrace_regs(fregs)->regs.gpr[1]);
>  }
>  #else
>  unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip,
> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> index 2cddd79ff21b..c6bcdff105b5 100644
> --- a/arch/riscv/include/asm/ftrace.h
> +++ b/arch/riscv/include/asm/ftrace.h
> @@ -126,7 +126,10 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec);
>  #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
>  #define arch_ftrace_get_regs(regs) NULL
>  struct ftrace_ops;
> -struct ftrace_regs {
> +struct ftrace_regs;
> +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs))
> +
> +struct __arch_ftrace_regs {
>  	unsigned long epc;
>  	unsigned long ra;
>  	unsigned long sp;
> @@ -150,42 +153,42 @@ struct ftrace_regs {
>  static __always_inline unsigned long ftrace_regs_get_instruction_pointer(const struct ftrace_regs
>  									 *fregs)
>  {
> -	return fregs->epc;
> +	return arch_ftrace_regs(fregs)->epc;
>  }
>  
>  static __always_inline void ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
>  								unsigned long pc)
>  {
> -	fregs->epc = pc;
> +	arch_ftrace_regs(fregs)->epc = pc;
>  }
>  
>  static __always_inline unsigned long ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
>  {
> -	return fregs->sp;
> +	return arch_ftrace_regs(fregs)->sp;
>  }
>  
>  static __always_inline unsigned long ftrace_regs_get_argument(struct ftrace_regs *fregs,
>  							      unsigned int n)
>  {
>  	if (n < 8)
> -		return fregs->args[n];
> +		return arch_ftrace_regs(fregs)->args[n];
>  	return 0;
>  }
>  
>  static __always_inline unsigned long ftrace_regs_get_return_value(const struct ftrace_regs *fregs)
>  {
> -	return fregs->a0;
> +	return arch_ftrace_regs(fregs)->a0;
>  }
>  
>  static __always_inline void ftrace_regs_set_return_value(struct ftrace_regs *fregs,
>  							 unsigned long ret)
>  {
> -	fregs->a0 = ret;
> +	arch_ftrace_regs(fregs)->a0 = ret;
>  }
>  
>  static __always_inline void ftrace_override_function_with_return(struct ftrace_regs *fregs)
>  {
> -	fregs->epc = fregs->ra;
> +	arch_ftrace_regs(fregs)->epc = arch_ftrace_regs(fregs)->ra;
>  }
>  
>  int ftrace_regs_query_register_offset(const char *name);
> @@ -196,7 +199,7 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>  
>  static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsigned long addr)
>  {
> -	fregs->t1 = addr;
> +	arch_ftrace_regs(fregs)->t1 = addr;
>  }
>  #endif /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */
>  
> diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
> index e94180ba432f..f6f5a277ba9d 100644
> --- a/arch/riscv/kernel/asm-offsets.c
> +++ b/arch/riscv/kernel/asm-offsets.c
> @@ -498,19 +498,19 @@ void asm_offsets(void)
>  	OFFSET(STACKFRAME_RA, stackframe, ra);
>  
>  #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> -	DEFINE(FREGS_SIZE_ON_STACK, ALIGN(sizeof(struct ftrace_regs), STACK_ALIGN));
> -	DEFINE(FREGS_EPC,	    offsetof(struct ftrace_regs, epc));
> -	DEFINE(FREGS_RA,	    offsetof(struct ftrace_regs, ra));
> -	DEFINE(FREGS_SP,	    offsetof(struct ftrace_regs, sp));
> -	DEFINE(FREGS_S0,	    offsetof(struct ftrace_regs, s0));
> -	DEFINE(FREGS_T1,	    offsetof(struct ftrace_regs, t1));
> -	DEFINE(FREGS_A0,	    offsetof(struct ftrace_regs, a0));
> -	DEFINE(FREGS_A1,	    offsetof(struct ftrace_regs, a1));
> -	DEFINE(FREGS_A2,	    offsetof(struct ftrace_regs, a2));
> -	DEFINE(FREGS_A3,	    offsetof(struct ftrace_regs, a3));
> -	DEFINE(FREGS_A4,	    offsetof(struct ftrace_regs, a4));
> -	DEFINE(FREGS_A5,	    offsetof(struct ftrace_regs, a5));
> -	DEFINE(FREGS_A6,	    offsetof(struct ftrace_regs, a6));
> -	DEFINE(FREGS_A7,	    offsetof(struct ftrace_regs, a7));
> +	DEFINE(FREGS_SIZE_ON_STACK, ALIGN(sizeof(struct __arch_ftrace_regs), STACK_ALIGN));
> +	DEFINE(FREGS_EPC,	    offsetof(struct __arch_ftrace_regs, epc));
> +	DEFINE(FREGS_RA,	    offsetof(struct __arch_ftrace_regs, ra));
> +	DEFINE(FREGS_SP,	    offsetof(struct __arch_ftrace_regs, sp));
> +	DEFINE(FREGS_S0,	    offsetof(struct __arch_ftrace_regs, s0));
> +	DEFINE(FREGS_T1,	    offsetof(struct __arch_ftrace_regs, t1));
> +	DEFINE(FREGS_A0,	    offsetof(struct __arch_ftrace_regs, a0));
> +	DEFINE(FREGS_A1,	    offsetof(struct __arch_ftrace_regs, a1));
> +	DEFINE(FREGS_A2,	    offsetof(struct __arch_ftrace_regs, a2));
> +	DEFINE(FREGS_A3,	    offsetof(struct __arch_ftrace_regs, a3));
> +	DEFINE(FREGS_A4,	    offsetof(struct __arch_ftrace_regs, a4));
> +	DEFINE(FREGS_A5,	    offsetof(struct __arch_ftrace_regs, a5));
> +	DEFINE(FREGS_A6,	    offsetof(struct __arch_ftrace_regs, a6));
> +	DEFINE(FREGS_A7,	    offsetof(struct __arch_ftrace_regs, a7));
>  #endif
>  }
> diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
> index 4b95c574fd04..5081ad886841 100644
> --- a/arch/riscv/kernel/ftrace.c
> +++ b/arch/riscv/kernel/ftrace.c
> @@ -214,7 +214,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
>  void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>  		       struct ftrace_ops *op, struct ftrace_regs *fregs)
>  {
> -	prepare_ftrace_return(&fregs->ra, ip, fregs->s0);
> +	prepare_ftrace_return(&arch_ftrace_regs(fregs)->ra, ip, arch_ftrace_regs(fregs)->s0);
>  }
>  #else /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */
>  extern void ftrace_graph_call(void);
> diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
> index 406746666eb7..1498d0a9c762 100644
> --- a/arch/s390/include/asm/ftrace.h
> +++ b/arch/s390/include/asm/ftrace.h
> @@ -51,13 +51,16 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
>  	return addr;
>  }
>  
> -struct ftrace_regs {
> +struct ftrace_regs;
> +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs))
> +
> +struct __arch_ftrace_regs {
>  	struct pt_regs regs;
>  };
>  
>  static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs)
>  {
> -	struct pt_regs *regs = &fregs->regs;
> +	struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs;
>  
>  	if (test_pt_regs_flag(regs, PIF_FTRACE_FULL_REGS))
>  		return regs;
> @@ -84,26 +87,26 @@ static __always_inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph
>  static __always_inline unsigned long
>  ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
>  {
> -	return fregs->regs.psw.addr;
> +	return arch_ftrace_regs(fregs)->regs.psw.addr;
>  }
>  
>  static __always_inline void
>  ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
>  				    unsigned long ip)
>  {
> -	fregs->regs.psw.addr = ip;
> +	arch_ftrace_regs(fregs)->regs.psw.addr = ip;
>  }
>  
>  #define ftrace_regs_get_argument(fregs, n) \
> -	regs_get_kernel_argument(&(fregs)->regs, n)
> +	regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n)
>  #define ftrace_regs_get_stack_pointer(fregs) \
> -	kernel_stack_pointer(&(fregs)->regs)
> +	kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs)
>  #define ftrace_regs_return_value(fregs) \
> -	regs_return_value(&(fregs)->regs)
> +	regs_return_value(&arch_ftrace_regs(fregs)->regs)
>  #define ftrace_regs_set_return_value(fregs, ret) \
> -	regs_set_return_value(&(fregs)->regs, ret)
> +	regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret)
>  #define ftrace_override_function_with_return(fregs) \
> -	override_function_with_return(&(fregs)->regs)
> +	override_function_with_return(&arch_ftrace_regs(fregs)->regs)
>  #define ftrace_regs_query_register_offset(name) \
>  	regs_query_register_offset(name)
>  
> @@ -117,7 +120,7 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
>   */
>  static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsigned long addr)
>  {
> -	struct pt_regs *regs = &fregs->regs;
> +	struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs;
>  	regs->orig_gpr2 = addr;
>  }
>  #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
> diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c
> index 5529248d84fb..db9659980175 100644
> --- a/arch/s390/kernel/asm-offsets.c
> +++ b/arch/s390/kernel/asm-offsets.c
> @@ -184,8 +184,8 @@ int main(void)
>  	OFFSET(__FGRAPH_RET_FP, fgraph_ret_regs, fp);
>  	DEFINE(__FGRAPH_RET_SIZE, sizeof(struct fgraph_ret_regs));
>  #endif
> -	OFFSET(__FTRACE_REGS_PT_REGS, ftrace_regs, regs);
> -	DEFINE(__FTRACE_REGS_SIZE, sizeof(struct ftrace_regs));
> +	OFFSET(__FTRACE_REGS_PT_REGS, __arch_ftrace_regs, regs);
> +	DEFINE(__FTRACE_REGS_SIZE, sizeof(struct __arch_ftrace_regs));
>  
>  	OFFSET(__PCPU_FLAGS, pcpu, flags);
>  	return 0;
> diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
> index 0b6e62d1d8b8..51439a71e392 100644
> --- a/arch/s390/kernel/ftrace.c
> +++ b/arch/s390/kernel/ftrace.c
> @@ -318,7 +318,7 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
>  	if (bit < 0)
>  		return;
>  
> -	kmsan_unpoison_memory(fregs, sizeof(*fregs));
> +	kmsan_unpoison_memory(fregs, ftrace_regs_size());
>  	regs = ftrace_get_regs(fregs);
>  	p = get_kprobe((kprobe_opcode_t *)ip);
>  	if (!regs || unlikely(!p) || kprobe_disabled(p))
> diff --git a/arch/s390/lib/test_unwind.c b/arch/s390/lib/test_unwind.c
> index 8b7f981e6f34..6e42100875e7 100644
> --- a/arch/s390/lib/test_unwind.c
> +++ b/arch/s390/lib/test_unwind.c
> @@ -270,9 +270,9 @@ static void notrace __used test_unwind_ftrace_handler(unsigned long ip,
>  						      struct ftrace_ops *fops,
>  						      struct ftrace_regs *fregs)
>  {
> -	struct unwindme *u = (struct unwindme *)fregs->regs.gprs[2];
> +	struct unwindme *u = (struct unwindme *)arch_ftrace_regs(fregs)->regs.gprs[2];
>  
> -	u->ret = test_unwind(NULL, (u->flags & UWM_REGS) ? &fregs->regs : NULL,
> +	u->ret = test_unwind(NULL, (u->flags & UWM_REGS) ? &arch_ftrace_regs(fregs)->regs : NULL,
>  			     (u->flags & UWM_SP) ? u->sp : 0);
>  }
>  
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index 0152a81d9b4a..87943f7a299b 100644
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -33,7 +33,10 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
>  }
>  
>  #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
> -struct ftrace_regs {
> +struct ftrace_regs;
> +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs))
> +
> +struct __arch_ftrace_regs {
>  	struct pt_regs		regs;
>  };
>  
> @@ -41,27 +44,27 @@ static __always_inline struct pt_regs *
>  arch_ftrace_get_regs(struct ftrace_regs *fregs)
>  {
>  	/* Only when FL_SAVE_REGS is set, cs will be non zero */
> -	if (!fregs->regs.cs)
> +	if (!arch_ftrace_regs(fregs)->regs.cs)
>  		return NULL;
> -	return &fregs->regs;
> +	return &arch_ftrace_regs(fregs)->regs;
>  }
>  
>  #define ftrace_regs_set_instruction_pointer(fregs, _ip)	\
> -	do { (fregs)->regs.ip = (_ip); } while (0)
> +	do { arch_ftrace_regs(fregs)->regs.ip = (_ip); } while (0)
>  
>  #define ftrace_regs_get_instruction_pointer(fregs) \
> -	((fregs)->regs.ip)
> +	arch_ftrace_regs(fregs)->regs.ip)
>  
>  #define ftrace_regs_get_argument(fregs, n) \
> -	regs_get_kernel_argument(&(fregs)->regs, n)
> +	regs_get_kernel_argument(&arch_ftrace_regs(fregs)->regs, n)
>  #define ftrace_regs_get_stack_pointer(fregs) \
> -	kernel_stack_pointer(&(fregs)->regs)
> +	kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs)
>  #define ftrace_regs_return_value(fregs) \
> -	regs_return_value(&(fregs)->regs)
> +	regs_return_value(&arch_ftrace_regs(fregs)->regs)
>  #define ftrace_regs_set_return_value(fregs, ret) \
> -	regs_set_return_value(&(fregs)->regs, ret)
> +	regs_set_return_value(&arch_ftrace_regs(fregs)->regs, ret)
>  #define ftrace_override_function_with_return(fregs) \
> -	override_function_with_return(&(fregs)->regs)
> +	override_function_with_return(&arch_ftrace_regs(fregs)->regs)
>  #define ftrace_regs_query_register_offset(name) \
>  	regs_query_register_offset(name)
>  
> @@ -88,7 +91,7 @@ __arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
>  	regs->orig_ax = addr;
>  }
>  #define arch_ftrace_set_direct_caller(fregs, addr) \
> -	__arch_ftrace_set_direct_caller(&(fregs)->regs, addr)
> +	__arch_ftrace_set_direct_caller(&arch_ftrace_regs(fregs)->regs, addr)
>  #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
>  
>  #ifdef CONFIG_DYNAMIC_FTRACE
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 8da0e66ca22d..adb09f78edb2 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -647,7 +647,7 @@ void prepare_ftrace_return(unsigned long ip, unsigned long *parent,
>  void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>  		       struct ftrace_ops *op, struct ftrace_regs *fregs)
>  {
> -	struct pt_regs *regs = &fregs->regs;
> +	struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs;
>  	unsigned long *stack = (unsigned long *)kernel_stack_pointer(regs);
>  
>  	prepare_ftrace_return(ip, (unsigned long *)stack, 0);
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 2ac3b3b53cd0..f7d4f152f84d 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -115,8 +115,6 @@ static inline int ftrace_mod_get_kallsym(unsigned int symnum, unsigned long *val
>  
>  extern int ftrace_enabled;
>  
> -#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
> -
>  /**
>   * ftrace_regs - ftrace partial/optimal register set
>   *
> @@ -142,11 +140,28 @@ extern int ftrace_enabled;
>   *
>   * NOTE: user *must not* access regs directly, only do it via APIs, because
>   * the member can be changed according to the architecture.
> + * This is why the structure is empty here, so that nothing accesses
> + * the ftrace_regs directly.
>   */
>  struct ftrace_regs {
> +	/* Nothing to see here, use the accessor functions! */
> +};
> +
> +#define ftrace_regs_size()	sizeof(struct __arch_ftrace_regs)
> +
> +#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
> +
> +struct __arch_ftrace_regs {
>  	struct pt_regs		regs;
>  };
> -#define arch_ftrace_get_regs(fregs) (&(fregs)->regs)
> +
> +struct ftrace_regs;
> +#define arch_ftrace_regs(fregs) ((struct __arch_ftrace_regs *)(fregs))
> +
> +static inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs)
> +{
> +	return &arch_ftrace_regs(fregs)->regs;
> +}
>  
>  /*
>   * ftrace_regs_set_instruction_pointer() is to be defined by the architecture
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 5d87dac83b80..1e6251174530 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -7944,7 +7944,7 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
>  void arch_ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
>  			       struct ftrace_ops *op, struct ftrace_regs *fregs)
>  {
> -	kmsan_unpoison_memory(fregs, sizeof(*fregs));
> +	kmsan_unpoison_memory(fregs, ftrace_regs_size());
>  	__ftrace_ops_list_func(ip, parent_ip, NULL, fregs);
>  }
>  #else
> -- 
> 2.45.2
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux