kprobes blacklist enhancement

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 <[email protected]>

Cc Ananth N Mavinakayanahalli <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc Rob Landley <[email protected]>

Cc Rusty Russell <[email protected]>

---
 Documentation/kprobes.txt |    8 ++++++
 include/linux/module.h    |    4 +++
 kernel/kprobes.c          |   64 ++++++++++++++++++++++++++++++++++++++-------
 kernel/module.c           |    6 ++++
 4 files changed, 72 insertions(+), 10 deletions(-)

Documentation/kprobes.txt +8 -0

index 1f9b3e2..b5a698f 100644
@@ -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,

include/linux/module.h +4 -0

index d67b193..0827e34 100644
@@ -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;

kernel/kprobes.c +54 -10

index c90e417..53951c3 100644
@@ -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,
 };
 

kernel/module.c +6 -1

index 4d2b82e..c3a593c 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

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.

Quoted 5 lines · 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.

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

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.

Quoted 3 lines · Masami Hiramatsu

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.

Quoted 7 lines

[...] 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?