Re: [PATCH] include/linux/kprobes.h: move all functions to their matched area

[Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>]

(2014/02/05 14:27), Chen Gang wrote:
> For dummy functions, it is not a good idea to still use the input
> parameters (not a good idea to assume they are still effect).
> 
>  - let kprobe* static inline functions in CONFIG_KPROBES, dummy outside.
> 
>  - let (en/dis)able_jprobe() in CONFIG_KPROBES, dummy outside.
> 
>  - for kretprobe:
> 
>    - let kretprobe_assert() only in CONFIG_KRETPROBES (internal use).
> 
>    - remove kretprobe_inst_table_head() (cannot grep it in kernel wide).
> 
>    - for (en/dis)able_kretprobe():
> 
>        if CONFIG_KRETPROBES enabled, they use (en/dis)able_kprobe().
>        else if CONFIG_KPROBES enabled, return -EINVAL (not registered).
>        else, return -ENOSYS, just like (en/dis)able_kprobe() have done.

NAK. I don't increase complexity in this header file anymore,
unless we have any actual issue with sane usage.

Thank you,

> 
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  include/linux/kprobes.h | 136 ++++++++++++++++++++++++++++++++----------------
>  1 file changed, 92 insertions(+), 44 deletions(-)
> 
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 925eaf2..860313d 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -129,30 +129,6 @@ struct kprobe {
>  				   */
>  #define KPROBE_FLAG_FTRACE	8 /* probe is using ftrace */
>  
> -/* Has this kprobe gone ? */
> -static inline int kprobe_gone(struct kprobe *p)
> -{
> -	return p->flags & KPROBE_FLAG_GONE;
> -}
> -
> -/* Is this kprobe disabled ? */
> -static inline int kprobe_disabled(struct kprobe *p)
> -{
> -	return p->flags & (KPROBE_FLAG_DISABLED | KPROBE_FLAG_GONE);
> -}
> -
> -/* Is this kprobe really running optimized path ? */
> -static inline int kprobe_optimized(struct kprobe *p)
> -{
> -	return p->flags & KPROBE_FLAG_OPTIMIZED;
> -}
> -
> -/* Is this kprobe uses ftrace ? */
> -static inline int kprobe_ftrace(struct kprobe *p)
> -{
> -	return p->flags & KPROBE_FLAG_FTRACE;
> -}
> -
>  /*
>   * Special probe type that uses setjmp-longjmp type tricks to resume
>   * execution at a specified entry with a matching prototype corresponding
> @@ -223,7 +199,42 @@ static inline int kprobes_built_in(void)
>  	return 1;
>  }
>  
> +/* Has this kprobe gone ? */
> +static inline int kprobe_gone(struct kprobe *p)
> +{
> +	return p->flags & KPROBE_FLAG_GONE;
> +}
> +
> +/* Is this kprobe disabled ? */
> +static inline int kprobe_disabled(struct kprobe *p)
> +{
> +	return p->flags & (KPROBE_FLAG_DISABLED | KPROBE_FLAG_GONE);
> +}
> +
> +/* Is this kprobe really running optimized path ? */
> +static inline int kprobe_optimized(struct kprobe *p)
> +{
> +	return p->flags & KPROBE_FLAG_OPTIMIZED;
> +}
> +
> +/* Is this kprobe uses ftrace ? */
> +static inline int kprobe_ftrace(struct kprobe *p)
> +{
> +	return p->flags & KPROBE_FLAG_FTRACE;
> +}
> +
>  #ifdef CONFIG_KRETPROBES
> +static inline void kretprobe_assert(struct kretprobe_instance *ri,
> +	unsigned long orig_ret_address, unsigned long trampoline_address)
> +{
> +	if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
> +		printk(KERN_ERR
> +			"kretprobe BUG!: Processing kretprobe %p @ %p\n",
> +			ri->rp, ri->rp->kp.addr);
> +		BUG();
> +	}
> +}
> +
>  extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
>  				   struct pt_regs *regs);
>  extern int arch_trampoline_kprobe(struct kprobe *p);
> @@ -240,16 +251,6 @@ static inline int arch_trampoline_kprobe(struct kprobe *p)
>  
>  extern struct kretprobe_blackpoint kretprobe_blacklist[];
>  
> -static inline void kretprobe_assert(struct kretprobe_instance *ri,
> -	unsigned long orig_ret_address, unsigned long trampoline_address)
> -{
> -	if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
> -		printk("kretprobe BUG!: Processing kretprobe %p @ %p\n",
> -				ri->rp, ri->rp->kp.addr);
> -		BUG();
> -	}
> -}
> -
>  #ifdef CONFIG_KPROBES_SANITY_TEST
>  extern int init_test_probes(void);
>  #else
> @@ -340,7 +341,6 @@ struct kprobe *get_kprobe(void *addr);
>  void kretprobe_hash_lock(struct task_struct *tsk,
>  			 struct hlist_head **head, unsigned long *flags);
>  void kretprobe_hash_unlock(struct task_struct *tsk, unsigned long *flags);
> -struct hlist_head * kretprobe_inst_table_head(struct task_struct *tsk);
>  
>  /* kprobe_running() will just return the current_kprobe on this CPU */
>  static inline struct kprobe *kprobe_running(void)
> @@ -384,12 +384,60 @@ int enable_kprobe(struct kprobe *kp);
>  
>  void dump_kprobe(struct kprobe *kp);
>  
> +static inline int disable_jprobe(struct jprobe *jp)
> +{
> +	return disable_kprobe(&jp->kp);
> +}
> +
> +static inline int enable_jprobe(struct jprobe *jp)
> +{
> +	return enable_kprobe(&jp->kp);
> +}
> +
> +#ifdef CONFIG_KRETPROBES
> +static inline int disable_kretprobe(struct kretprobe *rp)
> +{
> +	return disable_kprobe(&rp->kp);
> +}
> +
> +static inline int enable_kretprobe(struct kretprobe *rp)
> +{
> +	return enable_kprobe(&rp->kp);
> +}
> +
> +#else /* CONFIG_KRETPROBES */
> +static inline int disable_kretprobe(struct kretprobe *rp)
> +{
> +	return -EINVAL;
> +}
> +static inline int enable_kretprobe(struct kretprobe *rp)
> +{
> +	return -EINVAL;
> +}
> +#endif /* CONFIG_KRETPROBES */
> +
>  #else /* !CONFIG_KPROBES: */
>  
>  static inline int kprobes_built_in(void)
>  {
>  	return 0;
>  }
> +static inline int kprobe_gone(struct kprobe *p)
> +{
> +	return 1;
> +}
> +static inline int kprobe_disabled(struct kprobe *p)
> +{
> +	return 1;
> +}
> +static inline int kprobe_optimized(struct kprobe *p)
> +{
> +	return 0;
> +}
> +static inline int kprobe_ftrace(struct kprobe *p)
> +{
> +	return 0;
> +}
>  static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
>  {
>  	return 0;
> @@ -458,22 +506,22 @@ static inline int enable_kprobe(struct kprobe *kp)
>  {
>  	return -ENOSYS;
>  }
> -#endif /* CONFIG_KPROBES */
> -static inline int disable_kretprobe(struct kretprobe *rp)
> +static inline int disable_jprobe(struct jprobe *jp)
>  {
> -	return disable_kprobe(&rp->kp);
> +	return -ENOSYS;
>  }
> -static inline int enable_kretprobe(struct kretprobe *rp)
> +static inline int enable_jprobe(struct jprobe *jp)
>  {
> -	return enable_kprobe(&rp->kp);
> +	return -ENOSYS;
>  }
> -static inline int disable_jprobe(struct jprobe *jp)
> +static inline int disable_kretprobe(struct kretprobe *rp)
>  {
> -	return disable_kprobe(&jp->kp);
> +	return -ENOSYS;
>  }
> -static inline int enable_jprobe(struct jprobe *jp)
> +static inline int enable_kretprobe(struct kretprobe *rp)
>  {
> -	return enable_kprobe(&jp->kp);
> +	return -ENOSYS;
>  }
> +#endif /* CONFIG_KPROBES */
>  
>  #endif /* _LINUX_KPROBES_H */
> 


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com