[PATCH tip/master 0/3] kprobes blacklist enhancement

[PATCH tip/master 0/3] kprobes blacklist enhancement

[Masami Hiramatsu]

Hi,

Here is a series of patches of kprobe blacklist enhancement
which I sent 1 yrs ago (with some performance improvements).

I've decided to split these patches from performance improvement
patches because it is easy to review/merge.

These 3 patches add NOKPROBE_SYMBOL() support for modules and
show all the blacklisted symbols including arch-dependent
areas (which is previously aren't shown in debugfs/kprobes/blacklist).

Thank you,

---

Masami Hiramatsu (3):
      kprobes: Support blacklist functions in module
      kprobes: Use NOKPROBE_SYMBOL() in sample modules
      kprobes/x86: Use kprobe_blacklist for .kprobes.text and .entry.text


 Documentation/kprobes.txt           |    8 ++
 arch/x86/kernel/kprobes/core.c      |   11 +--
 include/linux/kprobes.h             |    1 
 include/linux/module.h              |    4 +
 kernel/kprobes.c                    |  122 ++++++++++++++++++++++++++++-------
 kernel/module.c                     |    6 ++
 samples/kprobes/jprobe_example.c    |    1 
 samples/kprobes/kprobe_example.c    |    3 +
 samples/kprobes/kretprobe_example.c |    2 +
 9 files changed, 123 insertions(+), 35 deletions(-)


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering 
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH tip/master 1/3] kprobes: Support blacklist functions in module

[Masami Hiramatsu]

To blacklist the functions in a module (e.g. user-defined
kprobe handler and the functions invoked from it), expand
blacklist support for modules.
With this change, users can use NOKPROBE_SYMBOL() macro in
their own modules.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Rob Landley <rob@landley.net>
Cc: Rusty Russell <rusty@rustcorp.com.au>
---
 Documentation/kprobes.txt |    8 ++++++
 include/linux/module.h    |    4 +++
 kernel/kprobes.c          |   64 ++++++++++++++++++++++++++++++++++++++-------
 kernel/module.c           |    6 ++++
 4 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt
index 1f9b3e2..b5a698f 100644
--- a/Documentation/kprobes.txt
+++ b/Documentation/kprobes.txt
@@ -513,6 +513,14 @@ int enable_jprobe(struct jprobe *jp);
 Enables *probe which has been disabled by disable_*probe(). You must specify
 the probe which has been registered.
 
+4.9 NOKPROBE_SYMBOL()
+
+#include <linux/kprobes.h>
+NOKPROBE_SYMBOL(FUNCTION);
+
+Protects given FUNCTION from other kprobes. This is useful for handler
+functions and functions called from the handlers.
+
 5. Kprobes Features and Limitations
 
 Kprobes allows multiple probes at the same address.  Currently,
diff --git a/include/linux/module.h b/include/linux/module.h
index d67b193..0827e34 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -352,6 +352,10 @@ struct module {
 	void __percpu *percpu;
 	unsigned int percpu_size;
 #endif
+#ifdef CONFIG_KPROBES
+	unsigned int num_kprobe_blacklist;
+	unsigned long  *kprobe_blacklist;
+#endif
 
 #ifdef CONFIG_TRACEPOINTS
 	unsigned int num_tracepoints;
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index c90e417..53951c3 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -88,6 +88,7 @@ static raw_spinlock_t *kretprobe_table_lock_ptr(unsigned long hash)
 
 /* Blacklist -- list of struct kprobe_blacklist_entry */
 static LIST_HEAD(kprobe_blacklist);
+static DEFINE_MUTEX(kprobe_blacklist_mutex);
 
 #ifdef __ARCH_WANT_KPROBES_INSN_SLOT
 /*
@@ -1332,22 +1333,27 @@ bool __weak arch_within_kprobe_blacklist(unsigned long addr)
 	       addr < (unsigned long)__kprobes_text_end;
 }
 
-static bool within_kprobe_blacklist(unsigned long addr)
+static struct kprobe_blacklist_entry *find_blacklist_entry(unsigned long addr)
 {
 	struct kprobe_blacklist_entry *ent;
 
+	list_for_each_entry(ent, &kprobe_blacklist, list) {
+		if (addr >= ent->start_addr && addr < ent->end_addr)
+			return ent;
+	}
+
+	return NULL;
+}
+
+static bool within_kprobe_blacklist(unsigned long addr)
+{
 	if (arch_within_kprobe_blacklist(addr))
 		return true;
 	/*
 	 * If there exists a kprobe_blacklist, verify and
 	 * fail any probe registration in the prohibited area
 	 */
-	list_for_each_entry(ent, &kprobe_blacklist, list) {
-		if (addr >= ent->start_addr && addr < ent->end_addr)
-			return true;
-	}
-
-	return false;
+	return !!find_blacklist_entry(addr);
 }
 
 /*
@@ -1437,6 +1443,8 @@ static int check_kprobe_address_safe(struct kprobe *p,
 	ret = arch_check_ftrace_location(p);
 	if (ret)
 		return ret;
+
+	mutex_lock(&kprobe_blacklist_mutex);
 	jump_label_lock();
 	preempt_disable();
 
@@ -1474,6 +1482,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
 out:
 	preempt_enable();
 	jump_label_unlock();
+	mutex_unlock(&kprobe_blacklist_mutex);
 
 	return ret;
 }
@@ -2054,13 +2063,13 @@ NOKPROBE_SYMBOL(dump_kprobe);
  * since a kprobe need not necessarily be at the beginning
  * of a function.
  */
-static int __init populate_kprobe_blacklist(unsigned long *start,
-					     unsigned long *end)
+static int populate_kprobe_blacklist(unsigned long *start, unsigned long *end)
 {
 	unsigned long *iter;
 	struct kprobe_blacklist_entry *ent;
 	unsigned long entry, offset = 0, size = 0;
 
+	mutex_lock(&kprobe_blacklist_mutex);
 	for (iter = start; iter < end; iter++) {
 		entry = arch_deref_entry_point((void *)*iter);
 
@@ -2079,9 +2088,28 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
 		INIT_LIST_HEAD(&ent->list);
 		list_add_tail(&ent->list, &kprobe_blacklist);
 	}
+	mutex_unlock(&kprobe_blacklist_mutex);
+
 	return 0;
 }
 
+/* Shrink the blacklist */
+static void shrink_kprobe_blacklist(unsigned long *start, unsigned long *end)
+{
+	struct kprobe_blacklist_entry *ent;
+	unsigned long *iter;
+
+	mutex_lock(&kprobe_blacklist_mutex);
+	for (iter = start; iter < end; iter++) {
+		ent = find_blacklist_entry(*iter);
+		if (!ent)
+			continue;
+		list_del(&ent->list);
+		kfree(ent);
+	}
+	mutex_unlock(&kprobe_blacklist_mutex);
+}
+
 /* Module notifier call back, checking kprobes on the module */
 static int kprobes_module_callback(struct notifier_block *nb,
 				   unsigned long val, void *data)
@@ -2092,6 +2120,16 @@ static int kprobes_module_callback(struct notifier_block *nb,
 	unsigned int i;
 	int checkcore = (val == MODULE_STATE_GOING);
 
+	/* Add/remove module blacklist */
+	if (val == MODULE_STATE_COMING)
+		populate_kprobe_blacklist(mod->kprobe_blacklist,
+					  mod->kprobe_blacklist +
+					  mod->num_kprobe_blacklist);
+	else if (val == MODULE_STATE_GOING)
+		shrink_kprobe_blacklist(mod->kprobe_blacklist,
+					mod->kprobe_blacklist +
+					mod->num_kprobe_blacklist);
+
 	if (val != MODULE_STATE_GOING && val != MODULE_STATE_LIVE)
 		return NOTIFY_DONE;
 
@@ -2278,6 +2316,7 @@ static const struct file_operations debugfs_kprobes_operations = {
 /* kprobes/blacklist -- shows which functions can not be probed */
 static void *kprobe_blacklist_seq_start(struct seq_file *m, loff_t *pos)
 {
+	mutex_lock(&kprobe_blacklist_mutex);
 	return seq_list_start(&kprobe_blacklist, *pos);
 }
 
@@ -2286,6 +2325,11 @@ static void *kprobe_blacklist_seq_next(struct seq_file *m, void *v, loff_t *pos)
 	return seq_list_next(v, &kprobe_blacklist, pos);
 }
 
+static void kprobe_blacklist_seq_stop(struct seq_file *m, void *v)
+{
+	mutex_unlock(&kprobe_blacklist_mutex);
+}
+
 static int kprobe_blacklist_seq_show(struct seq_file *m, void *v)
 {
 	struct kprobe_blacklist_entry *ent =
@@ -2299,7 +2343,7 @@ static int kprobe_blacklist_seq_show(struct seq_file *m, void *v)
 static const struct seq_operations kprobe_blacklist_seq_ops = {
 	.start = kprobe_blacklist_seq_start,
 	.next  = kprobe_blacklist_seq_next,
-	.stop  = kprobe_seq_stop,	/* Reuse void function */
+	.stop  = kprobe_blacklist_seq_stop,
 	.show  = kprobe_blacklist_seq_show,
 };
 
diff --git a/kernel/module.c b/kernel/module.c
index 4d2b82e..c3a593c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -57,6 +57,7 @@
 #include <linux/percpu.h>
 #include <linux/kmemleak.h>
 #include <linux/jump_label.h>
+#include <linux/kprobes.h>
 #include <linux/pfn.h>
 #include <linux/bsearch.h>
 #include <uapi/linux/module.h>
@@ -2937,6 +2938,11 @@ static int find_module_sections(struct module *mod, struct load_info *info)
 					     sizeof(*mod->ftrace_callsites),
 					     &mod->num_ftrace_callsites);
 #endif
+#ifdef CONFIG_KPROBES
+	mod->kprobe_blacklist = section_objs(info, "_kprobe_blacklist",
+					     sizeof(*mod->kprobe_blacklist),
+					     &mod->num_kprobe_blacklist);
+#endif
 
 	mod->extable = section_objs(info, "__ex_table",
 				    sizeof(*mod->extable), &mod->num_exentries);

Re: [PATCH tip/master 1/3] kprobes: Support blacklist functions in module

[Rusty Russell]

Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> writes:
> To blacklist the functions in a module (e.g. user-defined
> kprobe handler and the functions invoked from it), expand
> blacklist support for modules.
> With this change, users can use NOKPROBE_SYMBOL() macro in
> their own modules.

Looks great, thanks!

Acked-by: Rusty Russell <rusty@rustcorp.com.au>

> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Rob Landley <rob@landley.net>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> ---
>  Documentation/kprobes.txt |    8 ++++++
>  include/linux/module.h    |    4 +++
>  kernel/kprobes.c          |   64 ++++++++++++++++++++++++++++++++++++++-------
>  kernel/module.c           |    6 ++++
>  4 files changed, 72 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt
> index 1f9b3e2..b5a698f 100644
> --- a/Documentation/kprobes.txt
> +++ b/Documentation/kprobes.txt
> @@ -513,6 +513,14 @@ int enable_jprobe(struct jprobe *jp);
>  Enables *probe which has been disabled by disable_*probe(). You must specify
>  the probe which has been registered.
>  
> +4.9 NOKPROBE_SYMBOL()
> +
> +#include <linux/kprobes.h>
> +NOKPROBE_SYMBOL(FUNCTION);
> +
> +Protects given FUNCTION from other kprobes. This is useful for handler
> +functions and functions called from the handlers.
> +
>  5. Kprobes Features and Limitations
>  
>  Kprobes allows multiple probes at the same address.  Currently,
> diff --git a/include/linux/module.h b/include/linux/module.h
> index d67b193..0827e34 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -352,6 +352,10 @@ struct module {
>  	void __percpu *percpu;
>  	unsigned int percpu_size;
>  #endif
> +#ifdef CONFIG_KPROBES
> +	unsigned int num_kprobe_blacklist;
> +	unsigned long  *kprobe_blacklist;
> +#endif
>  
>  #ifdef CONFIG_TRACEPOINTS
>  	unsigned int num_tracepoints;
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index c90e417..53951c3 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -88,6 +88,7 @@ static raw_spinlock_t *kretprobe_table_lock_ptr(unsigned long hash)
>  
>  /* Blacklist -- list of struct kprobe_blacklist_entry */
>  static LIST_HEAD(kprobe_blacklist);
> +static DEFINE_MUTEX(kprobe_blacklist_mutex);
>  
>  #ifdef __ARCH_WANT_KPROBES_INSN_SLOT
>  /*
> @@ -1332,22 +1333,27 @@ bool __weak arch_within_kprobe_blacklist(unsigned long addr)
>  	       addr < (unsigned long)__kprobes_text_end;
>  }
>  
> -static bool within_kprobe_blacklist(unsigned long addr)
> +static struct kprobe_blacklist_entry *find_blacklist_entry(unsigned long addr)
>  {
>  	struct kprobe_blacklist_entry *ent;
>  
> +	list_for_each_entry(ent, &kprobe_blacklist, list) {
> +		if (addr >= ent->start_addr && addr < ent->end_addr)
> +			return ent;
> +	}
> +
> +	return NULL;
> +}
> +
> +static bool within_kprobe_blacklist(unsigned long addr)
> +{
>  	if (arch_within_kprobe_blacklist(addr))
>  		return true;
>  	/*
>  	 * If there exists a kprobe_blacklist, verify and
>  	 * fail any probe registration in the prohibited area
>  	 */
> -	list_for_each_entry(ent, &kprobe_blacklist, list) {
> -		if (addr >= ent->start_addr && addr < ent->end_addr)
> -			return true;
> -	}
> -
> -	return false;
> +	return !!find_blacklist_entry(addr);
>  }
>  
>  /*
> @@ -1437,6 +1443,8 @@ static int check_kprobe_address_safe(struct kprobe *p,
>  	ret = arch_check_ftrace_location(p);
>  	if (ret)
>  		return ret;
> +
> +	mutex_lock(&kprobe_blacklist_mutex);
>  	jump_label_lock();
>  	preempt_disable();
>  
> @@ -1474,6 +1482,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
>  out:
>  	preempt_enable();
>  	jump_label_unlock();
> +	mutex_unlock(&kprobe_blacklist_mutex);
>  
>  	return ret;
>  }
> @@ -2054,13 +2063,13 @@ NOKPROBE_SYMBOL(dump_kprobe);
>   * since a kprobe need not necessarily be at the beginning
>   * of a function.
>   */
> -static int __init populate_kprobe_blacklist(unsigned long *start,
> -					     unsigned long *end)
> +static int populate_kprobe_blacklist(unsigned long *start, unsigned long *end)
>  {
>  	unsigned long *iter;
>  	struct kprobe_blacklist_entry *ent;
>  	unsigned long entry, offset = 0, size = 0;
>  
> +	mutex_lock(&kprobe_blacklist_mutex);
>  	for (iter = start; iter < end; iter++) {
>  		entry = arch_deref_entry_point((void *)*iter);
>  
> @@ -2079,9 +2088,28 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
>  		INIT_LIST_HEAD(&ent->list);
>  		list_add_tail(&ent->list, &kprobe_blacklist);
>  	}
> +	mutex_unlock(&kprobe_blacklist_mutex);
> +
>  	return 0;
>  }
>  
> +/* Shrink the blacklist */
> +static void shrink_kprobe_blacklist(unsigned long *start, unsigned long *end)
> +{
> +	struct kprobe_blacklist_entry *ent;
> +	unsigned long *iter;
> +
> +	mutex_lock(&kprobe_blacklist_mutex);
> +	for (iter = start; iter < end; iter++) {
> +		ent = find_blacklist_entry(*iter);
> +		if (!ent)
> +			continue;
> +		list_del(&ent->list);
> +		kfree(ent);
> +	}
> +	mutex_unlock(&kprobe_blacklist_mutex);
> +}
> +
>  /* Module notifier call back, checking kprobes on the module */
>  static int kprobes_module_callback(struct notifier_block *nb,
>  				   unsigned long val, void *data)
> @@ -2092,6 +2120,16 @@ static int kprobes_module_callback(struct notifier_block *nb,
>  	unsigned int i;
>  	int checkcore = (val == MODULE_STATE_GOING);
>  
> +	/* Add/remove module blacklist */
> +	if (val == MODULE_STATE_COMING)
> +		populate_kprobe_blacklist(mod->kprobe_blacklist,
> +					  mod->kprobe_blacklist +
> +					  mod->num_kprobe_blacklist);
> +	else if (val == MODULE_STATE_GOING)
> +		shrink_kprobe_blacklist(mod->kprobe_blacklist,
> +					mod->kprobe_blacklist +
> +					mod->num_kprobe_blacklist);
> +
>  	if (val != MODULE_STATE_GOING && val != MODULE_STATE_LIVE)
>  		return NOTIFY_DONE;
>  
> @@ -2278,6 +2316,7 @@ static const struct file_operations debugfs_kprobes_operations = {
>  /* kprobes/blacklist -- shows which functions can not be probed */
>  static void *kprobe_blacklist_seq_start(struct seq_file *m, loff_t *pos)
>  {
> +	mutex_lock(&kprobe_blacklist_mutex);
>  	return seq_list_start(&kprobe_blacklist, *pos);
>  }
>  
> @@ -2286,6 +2325,11 @@ static void *kprobe_blacklist_seq_next(struct seq_file *m, void *v, loff_t *pos)
>  	return seq_list_next(v, &kprobe_blacklist, pos);
>  }
>  
> +static void kprobe_blacklist_seq_stop(struct seq_file *m, void *v)
> +{
> +	mutex_unlock(&kprobe_blacklist_mutex);
> +}
> +
>  static int kprobe_blacklist_seq_show(struct seq_file *m, void *v)
>  {
>  	struct kprobe_blacklist_entry *ent =
> @@ -2299,7 +2343,7 @@ static int kprobe_blacklist_seq_show(struct seq_file *m, void *v)
>  static const struct seq_operations kprobe_blacklist_seq_ops = {
>  	.start = kprobe_blacklist_seq_start,
>  	.next  = kprobe_blacklist_seq_next,
> -	.stop  = kprobe_seq_stop,	/* Reuse void function */
> +	.stop  = kprobe_blacklist_seq_stop,
>  	.show  = kprobe_blacklist_seq_show,
>  };
>  
> diff --git a/kernel/module.c b/kernel/module.c
> index 4d2b82e..c3a593c 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -57,6 +57,7 @@
>  #include <linux/percpu.h>
>  #include <linux/kmemleak.h>
>  #include <linux/jump_label.h>
> +#include <linux/kprobes.h>
>  #include <linux/pfn.h>
>  #include <linux/bsearch.h>
>  #include <uapi/linux/module.h>
> @@ -2937,6 +2938,11 @@ static int find_module_sections(struct module *mod, struct load_info *info)
>  					     sizeof(*mod->ftrace_callsites),
>  					     &mod->num_ftrace_callsites);
>  #endif
> +#ifdef CONFIG_KPROBES
> +	mod->kprobe_blacklist = section_objs(info, "_kprobe_blacklist",
> +					     sizeof(*mod->kprobe_blacklist),
> +					     &mod->num_kprobe_blacklist);
> +#endif
>  
>  	mod->extable = section_objs(info, "__ex_table",
>  				    sizeof(*mod->extable), &mod->num_exentries);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH tip/master 1/3] kprobes: Support blacklist functions in module

[Ingo Molnar]

* Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

> To blacklist the functions in a module (e.g. user-defined
> kprobe handler and the functions invoked from it), expand
> blacklist support for modules.
> With this change, users can use NOKPROBE_SYMBOL() macro in
> their own modules.

Btw., whatever happened with renaming '__kprobes' to '__nokprobe' and using that 
consistently to blacklist certain functions?

Also, shouldn't we convert such instances:

	static int notifier_call_chain(struct notifier_block **nl,
                               unsigned long val, void *v,
                               int nr_to_call, int *nr_calls)

	...

	NOKPROBE_SYMBOL(notifier_call_chain);

to:

	static int __nokprobe notifier_call_chain(struct notifier_block **nl,
                               unsigned long val, void *v,
                               int nr_to_call, int *nr_calls)

?

I.e. instead of extending it to modules we should eliminate NOKPROBE_SYMBOL() in 
favor of marking functions as __nokprobe which is the standard syntax for marking 
functions.

Thanks,

	Ingo

Re: [PATCH tip/master 1/3] kprobes: Support blacklist functions in module

[Masami Hiramatsu]

On 2015/07/17 21:10, Ingo Molnar wrote:
> 
> * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> 
>> To blacklist the functions in a module (e.g. user-defined
>> kprobe handler and the functions invoked from it), expand
>> blacklist support for modules.
>> With this change, users can use NOKPROBE_SYMBOL() macro in
>> their own modules.
> 
> Btw., whatever happened with renaming '__kprobes' to '__nokprobe' and using that 
> consistently to blacklist certain functions?

Yes, in this part, __kprobes marked functions placed in .kprobes.text section
are safely added to the blacklist :)

-----
+       if (err >= 0 && __kprobes_text_start != __kprobes_text_end) {
+               /* The __kprobes marked functions must not be probed */
+               err = kprobe_blacklist_add_range(
+                                       (unsigned long)__kprobes_text_start,
+                                       (unsigned long)__kprobes_text_end);
+       }
-----


> 
> Also, shouldn't we convert such instances:
> 
> 	static int notifier_call_chain(struct notifier_block **nl,
>                                unsigned long val, void *v,
>                                int nr_to_call, int *nr_calls)
> 
> 	...
> 
> 	NOKPROBE_SYMBOL(notifier_call_chain);
> 
> to:
> 
> 	static int __nokprobe notifier_call_chain(struct notifier_block **nl,
>                                unsigned long val, void *v,
>                                int nr_to_call, int *nr_calls)
> 
> ?

For some symbols we can do that. But it can conflict with other __section
attributes e.g. __sched, since a function must be placed in only one
section. So, IMHO, using section for expressing its attribute is not
a good idea, but I couldn't find another option in common function attribute.

https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes

Thus I've introduced NOKPROBE_SYMBOL macro which stores the target function
addresses (not the function itself) in the _kprobe_blacklist section.

Thank you,

> 
> I.e. instead of extending it to modules we should eliminate NOKPROBE_SYMBOL() in 
> favor of marking functions as __nokprobe which is the standard syntax for marking 
> functions.
> 
> Thanks,
> 
> 	Ingo
> 


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH tip/master 1/3] kprobes: Support blacklist functions in module

[Ingo Molnar]

* Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

> For some symbols we can do that. But it can conflict with other __section
> attributes e.g. __sched, since a function must be placed in only one
> section. [...]

The the scheduler is not modular, so __sched should not be a problem in itself.

> [...] So, IMHO, using section for expressing its attribute is not a good idea, 
> but I couldn't find another option in common function attribute.
> 
> https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes
> 
> Thus I've introduced NOKPROBE_SYMBOL macro which stores the target function
> addresses (not the function itself) in the _kprobe_blacklist section.

So the question is, in which cases do modules need this?

Thanks,

	Ingo

Re: Re: [PATCH tip/master 1/3] kprobes: Support blacklist functions in module

[Masami Hiramatsu]

On 2015/07/21 16:48, Ingo Molnar wrote:
> * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> 
>> For some symbols we can do that. But it can conflict with other __section
>> attributes e.g. __sched, since a function must be placed in only one
>> section. [...]
> 
> The the scheduler is not modular, so __sched should not be a problem in itself.

No, I meant why I chose this macro, itself should not be a section.
Or would we better use __nokprobe in module and NOKPROBE_SYMBOL in kernel? :(

>> [...] So, IMHO, using section for expressing its attribute is not a good idea, 
>> but I couldn't find another option in common function attribute.
>>
>> https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes
>>
>> Thus I've introduced NOKPROBE_SYMBOL macro which stores the target function
>> addresses (not the function itself) in the _kprobe_blacklist section.
> 
> So the question is, in which cases do modules need this?

The main reason for this is to put the kprobes handlers (and the functions called
from the kprobe handlers) on the blacklist. And also, there may be some cases which
NMI handlers can be in modules (as setting kconfig "m").

Thank you,

-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH tip/master 2/3] kprobes: Use NOKPROBE_SYMBOL() in sample modules

[Masami Hiramatsu]

Use NOKPROBE_SYMBOL() to protect handlers from kprobes
in sample modules.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Ananth N Mavinakayanahalli <ananth@in.ibm.com>
---
 samples/kprobes/jprobe_example.c    |    1 +
 samples/kprobes/kprobe_example.c    |    3 +++
 samples/kprobes/kretprobe_example.c |    2 ++
 3 files changed, 6 insertions(+)

diff --git a/samples/kprobes/jprobe_example.c b/samples/kprobes/jprobe_example.c
index 9119ac6..158d82c 100644
--- a/samples/kprobes/jprobe_example.c
+++ b/samples/kprobes/jprobe_example.c
@@ -34,6 +34,7 @@ static long jdo_fork(unsigned long clone_flags, unsigned long stack_start,
 	jprobe_return();
 	return 0;
 }
+NOKPROBE_SYMBOL(jdo_fork);
 
 static struct jprobe my_jprobe = {
 	.entry			= jdo_fork,
diff --git a/samples/kprobes/kprobe_example.c b/samples/kprobes/kprobe_example.c
index 366db1a..462d90f 100644
--- a/samples/kprobes/kprobe_example.c
+++ b/samples/kprobes/kprobe_example.c
@@ -46,6 +46,7 @@ static int handler_pre(struct kprobe *p, struct pt_regs *regs)
 	/* A dump_stack() here will give a stack backtrace */
 	return 0;
 }
+NOKPROBE_SYMBOL(handler_pre);
 
 /* kprobe post_handler: called after the probed instruction is executed */
 static void handler_post(struct kprobe *p, struct pt_regs *regs,
@@ -68,6 +69,7 @@ static void handler_post(struct kprobe *p, struct pt_regs *regs,
 		p->addr, regs->ex1);
 #endif
 }
+NOKPROBE_SYMBOL(handler_post);
 
 /*
  * fault_handler: this is called if an exception is generated for any
@@ -81,6 +83,7 @@ static int handler_fault(struct kprobe *p, struct pt_regs *regs, int trapnr)
 	/* Return 0 because we don't handle the fault. */
 	return 0;
 }
+NOKPROBE_SYMBOL(handler_fault);
 
 static int __init kprobe_init(void)
 {
diff --git a/samples/kprobes/kretprobe_example.c b/samples/kprobes/kretprobe_example.c
index 1041b67..d932c52 100644
--- a/samples/kprobes/kretprobe_example.c
+++ b/samples/kprobes/kretprobe_example.c
@@ -47,6 +47,7 @@ static int entry_handler(struct kretprobe_instance *ri, struct pt_regs *regs)
 	data->entry_stamp = ktime_get();
 	return 0;
 }
+NOKPROBE_SYMBOL(entry_handler);
 
 /*
  * Return-probe handler: Log the return value and duration. Duration may turn
@@ -66,6 +67,7 @@ static int ret_handler(struct kretprobe_instance *ri, struct pt_regs *regs)
 			func_name, retval, (long long)delta);
 	return 0;
 }
+NOKPROBE_SYMBOL(ret_handler);
 
 static struct kretprobe my_kretprobe = {
 	.handler		= ret_handler,

[PATCH tip/master 3/3] kprobes/x86: Use kprobe_blacklist for .kprobes.text and .entry.text

[Masami Hiramatsu]

Use kprobe_blackpoint for blacklisting .entry.text and .kprobees.text
instead of arch_within_kprobe_blacklist. This also makes them visible
via (debugfs)/kprobes/blacklist.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 arch/x86/kernel/kprobes/core.c |   11 +------
 include/linux/kprobes.h        |    1 +
 kernel/kprobes.c               |   64 ++++++++++++++++++++++++++++------------
 3 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 1deffe6..8496f84 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -1112,17 +1112,10 @@ int longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
 }
 NOKPROBE_SYMBOL(longjmp_break_handler);
 
-bool arch_within_kprobe_blacklist(unsigned long addr)
-{
-	return  (addr >= (unsigned long)__kprobes_text_start &&
-		 addr < (unsigned long)__kprobes_text_end) ||
-		(addr >= (unsigned long)__entry_text_start &&
-		 addr < (unsigned long)__entry_text_end);
-}
-
 int __init arch_init_kprobes(void)
 {
-	return 0;
+	return kprobe_blacklist_add_range((unsigned long)__entry_text_start,
+					  (unsigned long) __entry_text_end);
 }
 
 int arch_trampoline_kprobe(struct kprobe *p)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 1ab5475..a2eb53e 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -266,6 +266,7 @@ extern int arch_init_kprobes(void);
 extern void show_registers(struct pt_regs *regs);
 extern void kprobes_inc_nmissed_count(struct kprobe *p);
 extern bool arch_within_kprobe_blacklist(unsigned long addr);
+extern int kprobe_blacklist_add_range(unsigned long start, unsigned long end);
 
 struct kprobe_insn_cache {
 	struct mutex mutex;
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 53951c3..e164a04 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1326,13 +1326,6 @@ out:
 	return ret;
 }
 
-bool __weak arch_within_kprobe_blacklist(unsigned long addr)
-{
-	/* The __kprobes marked functions and entry code must not be probed */
-	return addr >= (unsigned long)__kprobes_text_start &&
-	       addr < (unsigned long)__kprobes_text_end;
-}
-
 static struct kprobe_blacklist_entry *find_blacklist_entry(unsigned long addr)
 {
 	struct kprobe_blacklist_entry *ent;
@@ -1347,8 +1340,6 @@ static struct kprobe_blacklist_entry *find_blacklist_entry(unsigned long addr)
 
 static bool within_kprobe_blacklist(unsigned long addr)
 {
-	if (arch_within_kprobe_blacklist(addr))
-		return true;
 	/*
 	 * If there exists a kprobe_blacklist, verify and
 	 * fail any probe registration in the prohibited area
@@ -2055,6 +2046,40 @@ void dump_kprobe(struct kprobe *kp)
 }
 NOKPROBE_SYMBOL(dump_kprobe);
 
+static int __kprobe_blacklist_add(unsigned long start, unsigned long end)
+{
+	struct kprobe_blacklist_entry *ent;
+
+	ent = kmalloc(sizeof(*ent), GFP_KERNEL);
+	if (!ent)
+		return -ENOMEM;
+
+	ent->start_addr = start;
+	ent->end_addr = end;
+	INIT_LIST_HEAD(&ent->list);
+	list_add_tail(&ent->list, &kprobe_blacklist);
+	return 0;
+}
+
+int kprobe_blacklist_add_range(unsigned long start, unsigned long end)
+{
+	unsigned long offset = 0, size = 0;
+	int err = 0;
+
+	mutex_lock(&kprobe_blacklist_mutex);
+	while (!err && start < end) {
+		if (!kallsyms_lookup_size_offset(start, &size, &offset) ||
+		    size == 0) {
+			err = -ENOENT;
+			break;
+		}
+		err = __kprobe_blacklist_add(start, start + size);
+		start += size;
+	}
+	mutex_unlock(&kprobe_blacklist_mutex);
+	return err;
+}
+
 /*
  * Lookup and populate the kprobe_blacklist.
  *
@@ -2066,8 +2091,8 @@ NOKPROBE_SYMBOL(dump_kprobe);
 static int populate_kprobe_blacklist(unsigned long *start, unsigned long *end)
 {
 	unsigned long *iter;
-	struct kprobe_blacklist_entry *ent;
 	unsigned long entry, offset = 0, size = 0;
+	int ret;
 
 	mutex_lock(&kprobe_blacklist_mutex);
 	for (iter = start; iter < end; iter++) {
@@ -2079,14 +2104,7 @@ static int populate_kprobe_blacklist(unsigned long *start, unsigned long *end)
 				(void *)entry);
 			continue;
 		}
-
-		ent = kmalloc(sizeof(*ent), GFP_KERNEL);
-		if (!ent)
-			return -ENOMEM;
-		ent->start_addr = entry;
-		ent->end_addr = entry + size;
-		INIT_LIST_HEAD(&ent->list);
-		list_add_tail(&ent->list, &kprobe_blacklist);
+		ret = __kprobe_blacklist_add(entry, entry - offset + size);
 	}
 	mutex_unlock(&kprobe_blacklist_mutex);
 
@@ -2181,7 +2199,15 @@ static int __init init_kprobes(void)
 
 	err = populate_kprobe_blacklist(__start_kprobe_blacklist,
 					__stop_kprobe_blacklist);
-	if (err) {
+
+	if (err >= 0 && __kprobes_text_start != __kprobes_text_end) {
+		/* The __kprobes marked functions must not be probed */
+		err = kprobe_blacklist_add_range(
+					(unsigned long)__kprobes_text_start,
+					(unsigned long)__kprobes_text_end);
+	}
+
+	if (err < 0) {
 		pr_err("kprobes: failed to populate blacklist: %d\n", err);
 		pr_err("Please take care of using kprobes.\n");
 	}