Re: [PATCH] kernel/kprobes.c: move kretprobe implementation to CONFIG_KRETPROBES area

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

(2014/02/05 12:36), Chen Gang wrote:
> When CONFIG_KRETPROBES disabled, kretprobe implementation are useless,
> so need move them to CONFIG_KPROBES area.
> 
>  - move all kretprobe* to CONFIG_KPROBES area and dummy outside.
>  - define kretprobe_flush_task() to let kprobe_flush_task() call.
>  - define init_kretprobes() to let init_kprobes() call.
> 
> 

Looks good to me ;)

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  kernel/kprobes.c | 323 +++++++++++++++++++++++++++++++------------------------
>  1 file changed, 181 insertions(+), 142 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index ceeadfc..0619536 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -69,7 +69,6 @@
>  
>  static int kprobes_initialized;
>  static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
> -static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
>  
>  /* NOTE: change this value only with kprobe_mutex held */
>  static bool kprobes_all_disarmed;
> @@ -77,14 +76,6 @@ static bool kprobes_all_disarmed;
>  /* This protects kprobe_table and optimizing_list */
>  static DEFINE_MUTEX(kprobe_mutex);
>  static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
> -static struct {
> -	raw_spinlock_t lock ____cacheline_aligned_in_smp;
> -} kretprobe_table_locks[KPROBE_TABLE_SIZE];
> -
> -static raw_spinlock_t *kretprobe_table_lock_ptr(unsigned long hash)
> -{
> -	return &(kretprobe_table_locks[hash].lock);
> -}
>  
>  /*
>   * Normally, functions that we'd want to prohibit kprobes in, are marked
> @@ -1079,125 +1070,6 @@ void __kprobes kprobes_inc_nmissed_count(struct kprobe *p)
>  	return;
>  }
>  
> -void __kprobes recycle_rp_inst(struct kretprobe_instance *ri,
> -				struct hlist_head *head)
> -{
> -	struct kretprobe *rp = ri->rp;
> -
> -	/* remove rp inst off the rprobe_inst_table */
> -	hlist_del(&ri->hlist);
> -	INIT_HLIST_NODE(&ri->hlist);
> -	if (likely(rp)) {
> -		raw_spin_lock(&rp->lock);
> -		hlist_add_head(&ri->hlist, &rp->free_instances);
> -		raw_spin_unlock(&rp->lock);
> -	} else
> -		/* Unregistering */
> -		hlist_add_head(&ri->hlist, head);
> -}
> -
> -void __kprobes kretprobe_hash_lock(struct task_struct *tsk,
> -			 struct hlist_head **head, unsigned long *flags)
> -__acquires(hlist_lock)
> -{
> -	unsigned long hash = hash_ptr(tsk, KPROBE_HASH_BITS);
> -	raw_spinlock_t *hlist_lock;
> -
> -	*head = &kretprobe_inst_table[hash];
> -	hlist_lock = kretprobe_table_lock_ptr(hash);
> -	raw_spin_lock_irqsave(hlist_lock, *flags);
> -}
> -
> -static void __kprobes kretprobe_table_lock(unsigned long hash,
> -	unsigned long *flags)
> -__acquires(hlist_lock)
> -{
> -	raw_spinlock_t *hlist_lock = kretprobe_table_lock_ptr(hash);
> -	raw_spin_lock_irqsave(hlist_lock, *flags);
> -}
> -
> -void __kprobes kretprobe_hash_unlock(struct task_struct *tsk,
> -	unsigned long *flags)
> -__releases(hlist_lock)
> -{
> -	unsigned long hash = hash_ptr(tsk, KPROBE_HASH_BITS);
> -	raw_spinlock_t *hlist_lock;
> -
> -	hlist_lock = kretprobe_table_lock_ptr(hash);
> -	raw_spin_unlock_irqrestore(hlist_lock, *flags);
> -}
> -
> -static void __kprobes kretprobe_table_unlock(unsigned long hash,
> -       unsigned long *flags)
> -__releases(hlist_lock)
> -{
> -	raw_spinlock_t *hlist_lock = kretprobe_table_lock_ptr(hash);
> -	raw_spin_unlock_irqrestore(hlist_lock, *flags);
> -}
> -
> -/*
> - * This function is called from finish_task_switch when task tk becomes dead,
> - * so that we can recycle any function-return probe instances associated
> - * with this task. These left over instances represent probed functions
> - * that have been called but will never return.
> - */
> -void __kprobes kprobe_flush_task(struct task_struct *tk)
> -{
> -	struct kretprobe_instance *ri;
> -	struct hlist_head *head, empty_rp;
> -	struct hlist_node *tmp;
> -	unsigned long hash, flags = 0;
> -
> -	if (unlikely(!kprobes_initialized))
> -		/* Early boot.  kretprobe_table_locks not yet initialized. */
> -		return;
> -
> -	INIT_HLIST_HEAD(&empty_rp);
> -	hash = hash_ptr(tk, KPROBE_HASH_BITS);
> -	head = &kretprobe_inst_table[hash];
> -	kretprobe_table_lock(hash, &flags);
> -	hlist_for_each_entry_safe(ri, tmp, head, hlist) {
> -		if (ri->task == tk)
> -			recycle_rp_inst(ri, &empty_rp);
> -	}
> -	kretprobe_table_unlock(hash, &flags);
> -	hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) {
> -		hlist_del(&ri->hlist);
> -		kfree(ri);
> -	}
> -}
> -
> -static inline void free_rp_inst(struct kretprobe *rp)
> -{
> -	struct kretprobe_instance *ri;
> -	struct hlist_node *next;
> -
> -	hlist_for_each_entry_safe(ri, next, &rp->free_instances, hlist) {
> -		hlist_del(&ri->hlist);
> -		kfree(ri);
> -	}
> -}
> -
> -static void __kprobes cleanup_rp_inst(struct kretprobe *rp)
> -{
> -	unsigned long flags, hash;
> -	struct kretprobe_instance *ri;
> -	struct hlist_node *next;
> -	struct hlist_head *head;
> -
> -	/* No race here */
> -	for (hash = 0; hash < KPROBE_TABLE_SIZE; hash++) {
> -		kretprobe_table_lock(hash, &flags);
> -		head = &kretprobe_inst_table[hash];
> -		hlist_for_each_entry_safe(ri, next, head, hlist) {
> -			if (ri->rp == rp)
> -				ri->rp = NULL;
> -		}
> -		kretprobe_table_unlock(hash, &flags);
> -	}
> -	free_rp_inst(rp);
> -}
> -
>  /*
>  * Add the new probe to ap->list. Fail if this is the
>  * second jprobe at the address - two jprobes can't coexist
> @@ -1764,6 +1636,55 @@ void __kprobes unregister_jprobes(struct jprobe **jps, int num)
>  EXPORT_SYMBOL_GPL(unregister_jprobes);
>  
>  #ifdef CONFIG_KRETPROBES
> +static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
> +static struct {
> +	raw_spinlock_t lock ____cacheline_aligned_in_smp;
> +} kretprobe_table_locks[KPROBE_TABLE_SIZE];
> +
> +static raw_spinlock_t *kretprobe_table_lock_ptr(unsigned long hash)
> +{
> +	return &(kretprobe_table_locks[hash].lock);
> +}
> +
> +void __kprobes kretprobe_hash_lock(struct task_struct *tsk,
> +			 struct hlist_head **head, unsigned long *flags)
> +__acquires(hlist_lock)
> +{
> +	unsigned long hash = hash_ptr(tsk, KPROBE_HASH_BITS);
> +	raw_spinlock_t *hlist_lock;
> +
> +	*head = &kretprobe_inst_table[hash];
> +	hlist_lock = kretprobe_table_lock_ptr(hash);
> +	raw_spin_lock_irqsave(hlist_lock, *flags);
> +}
> +
> +void __kprobes kretprobe_hash_unlock(struct task_struct *tsk,
> +	unsigned long *flags)
> +__releases(hlist_lock)
> +{
> +	unsigned long hash = hash_ptr(tsk, KPROBE_HASH_BITS);
> +	raw_spinlock_t *hlist_lock;
> +
> +	hlist_lock = kretprobe_table_lock_ptr(hash);
> +	raw_spin_unlock_irqrestore(hlist_lock, *flags);
> +}
> +
> +static void __kprobes kretprobe_table_lock(unsigned long hash,
> +	unsigned long *flags)
> +__acquires(hlist_lock)
> +{
> +	raw_spinlock_t *hlist_lock = kretprobe_table_lock_ptr(hash);
> +	raw_spin_lock_irqsave(hlist_lock, *flags);
> +}
> +
> +static void __kprobes kretprobe_table_unlock(unsigned long hash,
> +	unsigned long *flags)
> +__releases(hlist_lock)
> +{
> +	raw_spinlock_t *hlist_lock = kretprobe_table_lock_ptr(hash);
> +	raw_spin_unlock_irqrestore(hlist_lock, *flags);
> +}
> +
>  /*
>   * This kprobe pre_handler is registered with every kretprobe. When probe
>   * hits it will set up the return probe.
> @@ -1808,6 +1729,17 @@ static int __kprobes pre_handler_kretprobe(struct kprobe *p,
>  	return 0;
>  }
>  
> +static inline void free_rp_inst(struct kretprobe *rp)
> +{
> +	struct kretprobe_instance *ri;
> +	struct hlist_node *next;
> +
> +	hlist_for_each_entry_safe(ri, next, &rp->free_instances, hlist) {
> +		hlist_del(&ri->hlist);
> +		kfree(ri);
> +	}
> +}
> +
>  int __kprobes register_kretprobe(struct kretprobe *rp)
>  {
>  	int ret = 0;
> @@ -1885,6 +1817,26 @@ void __kprobes unregister_kretprobe(struct kretprobe *rp)
>  }
>  EXPORT_SYMBOL_GPL(unregister_kretprobe);
>  
> +static void __kprobes cleanup_rp_inst(struct kretprobe *rp)
> +{
> +	unsigned long flags, hash;
> +	struct kretprobe_instance *ri;
> +	struct hlist_node *next;
> +	struct hlist_head *head;
> +
> +	/* No race here */
> +	for (hash = 0; hash < KPROBE_TABLE_SIZE; hash++) {
> +		kretprobe_table_lock(hash, &flags);
> +		head = &kretprobe_inst_table[hash];
> +		hlist_for_each_entry_safe(ri, next, head, hlist) {
> +			if (ri->rp == rp)
> +				ri->rp = NULL;
> +		}
> +		kretprobe_table_unlock(hash, &flags);
> +	}
> +	free_rp_inst(rp);
> +}
> +
>  void __kprobes unregister_kretprobes(struct kretprobe **rps, int num)
>  {
>  	int i;
> @@ -1907,7 +1859,78 @@ void __kprobes unregister_kretprobes(struct kretprobe **rps, int num)
>  }
>  EXPORT_SYMBOL_GPL(unregister_kretprobes);
>  
> +void __kprobes recycle_rp_inst(struct kretprobe_instance *ri,
> +				struct hlist_head *head)
> +{
> +	struct kretprobe *rp = ri->rp;
> +
> +	/* remove rp inst off the rprobe_inst_table */
> +	hlist_del(&ri->hlist);
> +	INIT_HLIST_NODE(&ri->hlist);
> +	if (likely(rp)) {
> +		raw_spin_lock(&rp->lock);
> +		hlist_add_head(&ri->hlist, &rp->free_instances);
> +		raw_spin_unlock(&rp->lock);
> +	} else
> +		/* Unregistering */
> +		hlist_add_head(&ri->hlist, head);
> +}
> +
> +static void __kprobes kretprobe_flush_task(struct task_struct *tk)
> +{
> +	struct kretprobe_instance *ri;
> +	struct hlist_head *head, empty_rp;
> +	struct hlist_node *tmp;
> +	unsigned long hash, flags = 0;
> +
> +	if (unlikely(!kprobes_initialized))
> +		/* Early boot.  kretprobe_table_locks not yet initialized. */
> +		return;
> +
> +	INIT_HLIST_HEAD(&empty_rp);
> +	hash = hash_ptr(tk, KPROBE_HASH_BITS);
> +	head = &kretprobe_inst_table[hash];
> +	kretprobe_table_lock(hash, &flags);
> +	hlist_for_each_entry_safe(ri, tmp, head, hlist) {
> +		if (ri->task == tk)
> +			recycle_rp_inst(ri, &empty_rp);
> +	}
> +	kretprobe_table_unlock(hash, &flags);
> +	hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) {
> +		hlist_del(&ri->hlist);
> +		kfree(ri);
> +	}
> +}
> +
> +static void __init init_kretprobes(void)
> +{
> +	int i;
> +
> +	/* FIXME allocate the probe table, currently defined statically */
> +	/* initialize all list heads */
> +	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
> +		INIT_HLIST_HEAD(&kretprobe_inst_table[i]);
> +		raw_spin_lock_init(&(kretprobe_table_locks[i].lock));
> +	}
> +
> +	if (kretprobe_blacklist_size) {
> +		/* lookup the function address from its name */
> +		for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
> +			kprobe_lookup_name(kretprobe_blacklist[i].name,
> +					   kretprobe_blacklist[i].addr);
> +			if (!kretprobe_blacklist[i].addr)
> +				printk(KERN_WARNING
> +					"kretprobe: lookup failed: %s\n",
> +					kretprobe_blacklist[i].name);
> +		}
> +	}
> +}
> +
>  #else /* CONFIG_KRETPROBES */
> +
> +#define kretprobe_flush_task(p)			do {} while (0)
> +#define init_kretprobes()			do {} while (0)
> +
>  int __kprobes register_kretprobe(struct kretprobe *rp)
>  {
>  	return -ENOSYS;
> @@ -1936,8 +1959,35 @@ static int __kprobes pre_handler_kretprobe(struct kprobe *p,
>  	return 0;
>  }
>  
> +void __kprobes recycle_rp_inst(struct kretprobe_instance *ri,
> +				struct hlist_head *head)
> +{
> +}
> +
> +void __kprobes kretprobe_hash_lock(struct task_struct *tsk,
> +			 struct hlist_head **head, unsigned long *flags)
> +__acquires(hlist_lock)
> +{
> +}
> +
> +void __kprobes kretprobe_hash_unlock(struct task_struct *tsk,
> +	unsigned long *flags)
> +__releases(hlist_lock)
> +{
> +}
>  #endif /* CONFIG_KRETPROBES */
>  
> +/*
> + * This function is called from finish_task_switch when task tk becomes dead,
> + * so that we can recycle any function-return probe instances associated
> + * with this task. These left over instances represent probed functions
> + * that have been called but will never return.
> + */
> +void __kprobes kprobe_flush_task(struct task_struct *tk)
> +{
> +	kretprobe_flush_task(tk);
> +}
> +
>  /* Set the kprobe gone and remove its instruction buffer. */
>  static void __kprobes kill_kprobe(struct kprobe *p)
>  {
> @@ -2073,11 +2123,8 @@ static int __init init_kprobes(void)
>  
>  	/* FIXME allocate the probe table, currently defined statically */
>  	/* initialize all list heads */
> -	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
> +	for (i = 0; i < KPROBE_TABLE_SIZE; i++)
>  		INIT_HLIST_HEAD(&kprobe_table[i]);
> -		INIT_HLIST_HEAD(&kretprobe_inst_table[i]);
> -		raw_spin_lock_init(&(kretprobe_table_locks[i].lock));
> -	}
>  
>  	/*
>  	 * Lookup and populate the kprobe_blacklist.
> @@ -2101,16 +2148,8 @@ static int __init init_kprobes(void)
>  			kb->range = size;
>  	}
>  
> -	if (kretprobe_blacklist_size) {
> -		/* lookup the function address from its name */
> -		for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
> -			kprobe_lookup_name(kretprobe_blacklist[i].name,
> -					   kretprobe_blacklist[i].addr);
> -			if (!kretprobe_blacklist[i].addr)
> -				printk("kretprobe: lookup failed: %s\n",
> -				       kretprobe_blacklist[i].name);
> -		}
> -	}
> +	/* Initialize kretprobes */
> +	init_kretprobes();
>  
>  #if defined(CONFIG_OPTPROBES)
>  #if defined(__ARCH_WANT_KPROBES_INSN_SLOT)
> 


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