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

2014-02-05 Thread Chen Gang 
On 02/05/2014 03:51 PM, Masami Hiramatsu wrote:
> (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,
> 

OK, I can understand, thanks. :-)


>>
>>
>> Signed-off-by: Chen Gang 
>> ---
>>  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)
>>

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

2014-02-04 Thread Masami Hiramatsu 
(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 
> ---
>  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 in

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

2014-02-04 Thread Chen Gang 
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.


Signed-off-by: Chen Gang 
---
 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 -E