Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1546198 > unrolled thread

Detecting kprobes generated code addresses

Back to article view | Back to linux.kernel

Contents

  Detecting kprobes generated code addresses Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-22 07:50 +0100
    Re: Detecting kprobes generated code addresses Masami Hiramatsu <mhiramat@kernel.org> - 2016-12-25 04:20 +0100
      Re: Detecting kprobes generated code addresses Masami Hiramatsu <mhiramat@kernel.org> - 2016-12-25 07:20 +0100
        Re: Detecting kprobes generated code addresses Masami Hiramatsu <mhiramat@kernel.org> - 2016-12-26 05:40 +0100
          [PATCH tip/master] kprobes: extable: Identify kprobes' insn-slots as kernel text area Masami Hiramatsu <mhiramat@kernel.org> - 2016-12-26 16:00 +0100
          [PATCH tip/master v2] kprobes: extable: Identify kprobes' insn-slots as kernel text area Masami Hiramatsu <mhiramat@kernel.org> - 2016-12-26 16:40 +0100
            Re: [PATCH tip/master v2] kprobes: extable: Identify kprobes'  insn-slots as kernel text area kbuild test robot <lkp@intel.com> - 2016-12-26 18:30 +0100
            Re: [PATCH tip/master v2] kprobes: extable: Identify kprobes'  insn-slots as kernel text area kbuild test robot <lkp@intel.com> - 2016-12-26 18:50 +0100
            Re: [PATCH tip/master v2] kprobes: extable: Identify kprobes'  insn-slots as kernel text area Masami Hiramatsu <mhiramat@kernel.org> - 2016-12-27 07:30 +0100
          [PATCH tip/master v3] kprobes: extable: Identify kprobes' insn-slots as kernel text area Masami Hiramatsu <mhiramat@kernel.org> - 2016-12-27 07:30 +0100
            Re: [PATCH tip/master v3] kprobes: extable: Identify kprobes'  insn-slots as kernel text area Peter Zijlstra <peterz@infradead.org> - 2017-01-03 12:00 +0100

#1546198 — Detecting kprobes generated code addresses

Hi Masami,

I would like to make __kernel_text_address() be able to detect whether
an address belongs to code which was generated by kprobes.  As far as I
can tell, that information seems to be in the 'pages' lists of
kprobe_insn_slots and kprobe_optinsn_slots.  But they seem to be
protected by mutexes.  Do you know if there's a sleep-free way to access
that information?

-- 
Josh

[toc] | [next] | [standalone]

#1547127

On Thu, 22 Dec 2016 00:42:19 -0600
Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> Hi Masami,
> 
> I would like to make __kernel_text_address() be able to detect whether
> an address belongs to code which was generated by kprobes.  As far as I
> can tell, that information seems to be in the 'pages' lists of
> kprobe_insn_slots and kprobe_optinsn_slots.  But they seem to be
> protected by mutexes.

Right. It is currently under mutex because it may kick
page allocation. But I think it is easy to fix that :)

>  Do you know if there's a sleep-free way to access
> that information?

Hmm, no, I couldn't find that yet.

Thanks,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1547132

On Sun, 25 Dec 2016 12:13:20 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Thu, 22 Dec 2016 00:42:19 -0600
> Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 
> > Hi Masami,
> > 
> > I would like to make __kernel_text_address() be able to detect whether
> > an address belongs to code which was generated by kprobes.  As far as I
> > can tell, that information seems to be in the 'pages' lists of
> > kprobe_insn_slots and kprobe_optinsn_slots.  But they seem to be
> > protected by mutexes.
> 
> Right. It is currently under mutex because it may kick
> page allocation. But I think it is easy to fix that :)

Hmm, IMHO, it seems that we should add a dummy (auto-generated)
symbol for optprobe trampoline code to kallsyms so that
__kernel_text_address() automatically returns true on it.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1547252

On Sun, 25 Dec 2016 15:16:00 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Sun, 25 Dec 2016 12:13:20 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > On Thu, 22 Dec 2016 00:42:19 -0600
> > Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > 
> > > Hi Masami,
> > > 
> > > I would like to make __kernel_text_address() be able to detect whether
> > > an address belongs to code which was generated by kprobes.  As far as I
> > > can tell, that information seems to be in the 'pages' lists of
> > > kprobe_insn_slots and kprobe_optinsn_slots.  But they seem to be
> > > protected by mutexes.
> > 
> > Right. It is currently under mutex because it may kick
> > page allocation. But I think it is easy to fix that :)
> 
> Hmm, IMHO, it seems that we should add a dummy (auto-generated)
> symbol for optprobe trampoline code to kallsyms so that
> __kernel_text_address() automatically returns true on it.

Sorry, I reconsidered this idea and conclude it was overkill.
As same as ftrace does, maybe it is enough to add a check
routine to __kernel_text_address().

Thanks,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1547330 — [PATCH tip/master] kprobes: extable: Identify kprobes' insn-slots as kernel text area

Make __kernel_text_address()/kernel_text_address() returns
true if the given address is on a kprobe's instruction slot,
which is generated by kprobes as a trampoline code.
This can help stacktraces to determine the address is on a
text area or not.

To implement this without any sleep in is_kprobe_*_slot(),
this also modify insn_cache page list as a rcu list. It may
increase processing deley (not processing time) for garbage
slot collection, because it requires to wait an additional
rcu grance period when freeing a page from the list.
However, since it is not a hot path, we may not take care of it.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

---
 Hi Josh, could check this patch fixes your issue? It will
 enable unwinder code to validate return address by using
 __kernel_text_address() again.
---
 include/linux/kprobes.h |   14 +++++++++
 kernel/extable.c        |    9 +++++-
 kernel/kprobes.c        |   70 ++++++++++++++++++++++++++++++++++++-----------
 3 files changed, 74 insertions(+), 19 deletions(-)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 8f68490..400c5c1 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -281,6 +281,9 @@ struct kprobe_insn_cache {
 extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c);
 extern void __free_insn_slot(struct kprobe_insn_cache *c,
 			     kprobe_opcode_t *slot, int dirty);
+/* sleep-less address checking routine  */
+extern bool __is_insn_slot_addr(struct kprobe_insn_cache *c,
+				unsigned long addr);
 
 #define DEFINE_INSN_CACHE_OPS(__name)					\
 extern struct kprobe_insn_cache kprobe_##__name##_slots;		\
@@ -294,6 +297,11 @@ static inline void free_##__name##_slot(kprobe_opcode_t *slot, int dirty)\
 {									\
 	__free_insn_slot(&kprobe_##__name##_slots, slot, dirty);	\
 }									\
+									\
+static inline bool is_kprobe_##__name##_slot(unsigned long addr)	\
+{									\
+	return __is_insn_slot_addr(&kprobe_##__name##_slots, addr);	\
+}
 
 DEFINE_INSN_CACHE_OPS(insn);
 
@@ -330,7 +338,11 @@ extern int proc_kprobes_optimization_handler(struct ctl_table *table,
 					     int write, void __user *buffer,
 					     size_t *length, loff_t *ppos);
 #endif
-
+#else	/* CONFIG_OPTPROBES */
+static inline bool is_kprobe_optinsn_slot(unsigned long addr)
+{
+	return false;
+}
 #endif /* CONFIG_OPTPROBES */
 #ifdef CONFIG_KPROBES_ON_FTRACE
 extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
diff --git a/kernel/extable.c b/kernel/extable.c
index e820cce..81c9633 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/init.h>
+#include <linux/kprobes.h>
 
 #include <asm/sections.h>
 #include <asm/uaccess.h>
@@ -104,6 +105,8 @@ int __kernel_text_address(unsigned long addr)
 		return 1;
 	if (is_ftrace_trampoline(addr))
 		return 1;
+	if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr))
+		return 1;
 	/*
 	 * There might be init symbols in saved stacktraces.
 	 * Give those symbols a chance to be printed in
@@ -123,7 +126,11 @@ int kernel_text_address(unsigned long addr)
 		return 1;
 	if (is_module_text_address(addr))
 		return 1;
-	return is_ftrace_trampoline(addr);
+	if (is_ftrace_trampoline(addr))
+		return 1;
+	if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr))
+		return 1;
+	return 0;
 }
 
 /*
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index d630954..1bd1c17 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -149,9 +149,11 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
 	struct kprobe_insn_page *kip;
 	kprobe_opcode_t *slot = NULL;
 
+	/* Since the slot array is not protected by rcu, we need a mutex */
 	mutex_lock(&c->mutex);
  retry:
-	list_for_each_entry(kip, &c->pages, list) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(kip, &c->pages, list) {
 		if (kip->nused < slots_per_page(c)) {
 			int i;
 			for (i = 0; i < slots_per_page(c); i++) {
@@ -167,6 +169,7 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
 			WARN_ON(1);
 		}
 	}
+	rcu_read_unlock();
 
 	/* If there are any garbage slots, collect it and try again. */
 	if (c->nr_garbage && collect_garbage_slots(c) == 0)
@@ -193,13 +196,15 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
 	kip->nused = 1;
 	kip->ngarbage = 0;
 	kip->cache = c;
-	list_add(&kip->list, &c->pages);
+	list_add_rcu(&kip->list, &c->pages);
 	slot = kip->insns;
 out:
 	mutex_unlock(&c->mutex);
 	return slot;
 }
 
+
+
 /* Return 1 if all garbages are collected, otherwise 0. */
 static int collect_one_slot(struct kprobe_insn_page *kip, int idx)
 {
@@ -213,7 +218,8 @@ static int collect_one_slot(struct kprobe_insn_page *kip, int idx)
 		 * next time somebody inserts a probe.
 		 */
 		if (!list_is_singular(&kip->list)) {
-			list_del(&kip->list);
+			list_del_rcu(&kip->list);
+			synchronize_rcu();
 			kip->cache->free(kip->insns);
 			kfree(kip);
 		}
@@ -248,29 +254,59 @@ void __free_insn_slot(struct kprobe_insn_cache *c,
 		      kprobe_opcode_t *slot, int dirty)
 {
 	struct kprobe_insn_page *kip;
+	long idx;
 
 	mutex_lock(&c->mutex);
-	list_for_each_entry(kip, &c->pages, list) {
-		long idx = ((long)slot - (long)kip->insns) /
-				(c->insn_size * sizeof(kprobe_opcode_t));
-		if (idx >= 0 && idx < slots_per_page(c)) {
-			WARN_ON(kip->slot_used[idx] != SLOT_USED);
-			if (dirty) {
-				kip->slot_used[idx] = SLOT_DIRTY;
-				kip->ngarbage++;
-				if (++c->nr_garbage > slots_per_page(c))
-					collect_garbage_slots(c);
-			} else
-				collect_one_slot(kip, idx);
+	rcu_read_lock();
+	list_for_each_entry_rcu(kip, &c->pages, list) {
+		idx = ((long)slot - (long)kip->insns) /
+			(c->insn_size * sizeof(kprobe_opcode_t));
+		if (idx >= 0 && idx < slots_per_page(c))
 			goto out;
-		}
 	}
-	/* Could not free this slot. */
+	/* Could not find this slot. */
 	WARN_ON(1);
+	kip = NULL;
 out:
+	rcu_read_unlock();
+	/* Mark and sweep: this may sleep */
+	if (kip) {
+		/* Check double free */
+		WARN_ON(kip->slot_used[idx] != SLOT_USED);
+		if (dirty) {
+			kip->slot_used[idx] = SLOT_DIRTY;
+			kip->ngarbage++;
+			if (++c->nr_garbage > slots_per_page(c))
+				collect_garbage_slots(c);
+		} else
+			collect_one_slot(kip, idx);
+	}
 	mutex_unlock(&c->mutex);
 }
 
+/*
+ * Check given address is on the page of kprobe instruction slots.
+ * This will be used for checking whether the address on a stack
+ * is on a text area or not.
+ */
+bool __is_insn_slot_addr(struct kprobe_insn_cache *c, unsigned long addr)
+{
+	struct kprobe_insn_page *kip;
+	bool ret = false;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(kip, &c->pages, list) {
+		if (addr >= (unsigned long)kip->insns &&
+		    addr < (unsigned long)kip->insns + PAGE_SIZE) {
+			ret = true;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return ret;
+}
+
 #ifdef CONFIG_OPTPROBES
 /* For optimized_kprobe buffer */
 struct kprobe_insn_cache kprobe_optinsn_slots = {

[toc] | [prev] | [next] | [standalone]

#1547334 — [PATCH tip/master v2] kprobes: extable: Identify kprobes' insn-slots as kernel text area

Make __kernel_text_address()/kernel_text_address() returns
true if the given address is on a kprobe's instruction slot,
which is generated by kprobes as a trampoline code.
This can help stacktraces to determine the address is on a
text area or not.

To implement this without any sleep in is_kprobe_*_slot(),
this also modify insn_cache page list as a rcu list. It may
increase processing deley (not processing time) for garbage
slot collection, because it requires to wait an additional
rcu grance period when freeing a page from the list.
However, since it is not a hot path, we may not take care of it.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

---
 V2: Fix build error when CONFIG_KPROBES=n

 Hi Josh, could check this patch fixes your issue? It will
 enable unwinder code to validate return address by using
 __kernel_text_address() again.
---
 include/linux/kprobes.h |   22 ++++++++++++++-
 kernel/extable.c        |    9 +++++-
 kernel/kprobes.c        |   70 ++++++++++++++++++++++++++++++++++++-----------
 3 files changed, 82 insertions(+), 19 deletions(-)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 8f68490..f0496b0 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -281,6 +281,9 @@ struct kprobe_insn_cache {
 extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c);
 extern void __free_insn_slot(struct kprobe_insn_cache *c,
 			     kprobe_opcode_t *slot, int dirty);
+/* sleep-less address checking routine  */
+extern bool __is_insn_slot_addr(struct kprobe_insn_cache *c,
+				unsigned long addr);
 
 #define DEFINE_INSN_CACHE_OPS(__name)					\
 extern struct kprobe_insn_cache kprobe_##__name##_slots;		\
@@ -294,6 +297,11 @@ static inline void free_##__name##_slot(kprobe_opcode_t *slot, int dirty)\
 {									\
 	__free_insn_slot(&kprobe_##__name##_slots, slot, dirty);	\
 }									\
+									\
+static inline bool is_kprobe_##__name##_slot(unsigned long addr)	\
+{									\
+	return __is_insn_slot_addr(&kprobe_##__name##_slots, addr);	\
+}
 
 DEFINE_INSN_CACHE_OPS(insn);
 
@@ -330,7 +338,6 @@ extern int proc_kprobes_optimization_handler(struct ctl_table *table,
 					     int write, void __user *buffer,
 					     size_t *length, loff_t *ppos);
 #endif
-
 #endif /* CONFIG_OPTPROBES */
 #ifdef CONFIG_KPROBES_ON_FTRACE
 extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
@@ -481,6 +488,19 @@ static inline int enable_jprobe(struct jprobe *jp)
 	return enable_kprobe(&jp->kp);
 }
 
+#ifndef CONFIG_KPROBES
+static inline bool is_kprobe_insn_slot(unsigned long addr)
+{
+	return false;
+}
+#endif
+#ifndef CONFIG_OPTPROBES
+static inline bool is_kprobe_optinsn_slot(unsigned long addr)
+{
+	return false;
+}
+#endif
+
 #ifdef CONFIG_KPROBES
 /*
  * Blacklist ganerating macro. Specify functions which is not probed
diff --git a/kernel/extable.c b/kernel/extable.c
index e820cce..81c9633 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/init.h>
+#include <linux/kprobes.h>
 
 #include <asm/sections.h>
 #include <asm/uaccess.h>
@@ -104,6 +105,8 @@ int __kernel_text_address(unsigned long addr)
 		return 1;
 	if (is_ftrace_trampoline(addr))
 		return 1;
+	if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr))
+		return 1;
 	/*
 	 * There might be init symbols in saved stacktraces.
 	 * Give those symbols a chance to be printed in
@@ -123,7 +126,11 @@ int kernel_text_address(unsigned long addr)
 		return 1;
 	if (is_module_text_address(addr))
 		return 1;
-	return is_ftrace_trampoline(addr);
+	if (is_ftrace_trampoline(addr))
+		return 1;
+	if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr))
+		return 1;
+	return 0;
 }
 
 /*
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index d630954..1bd1c17 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -149,9 +149,11 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
 	struct kprobe_insn_page *kip;
 	kprobe_opcode_t *slot = NULL;
 
+	/* Since the slot array is not protected by rcu, we need a mutex */
 	mutex_lock(&c->mutex);
  retry:
-	list_for_each_entry(kip, &c->pages, list) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(kip, &c->pages, list) {
 		if (kip->nused < slots_per_page(c)) {
 			int i;
 			for (i = 0; i < slots_per_page(c); i++) {
@@ -167,6 +169,7 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
 			WARN_ON(1);
 		}
 	}
+	rcu_read_unlock();
 
 	/* If there are any garbage slots, collect it and try again. */
 	if (c->nr_garbage && collect_garbage_slots(c) == 0)
@@ -193,13 +196,15 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
 	kip->nused = 1;
 	kip->ngarbage = 0;
 	kip->cache = c;
-	list_add(&kip->list, &c->pages);
+	list_add_rcu(&kip->list, &c->pages);
 	slot = kip->insns;
 out:
 	mutex_unlock(&c->mutex);
 	return slot;
 }
 
+
+
 /* Return 1 if all garbages are collected, otherwise 0. */
 static int collect_one_slot(struct kprobe_insn_page *kip, int idx)
 {
@@ -213,7 +218,8 @@ static int collect_one_slot(struct kprobe_insn_page *kip, int idx)
 		 * next time somebody inserts a probe.
 		 */
 		if (!list_is_singular(&kip->list)) {
-			list_del(&kip->list);
+			list_del_rcu(&kip->list);
+			synchronize_rcu();
 			kip->cache->free(kip->insns);
 			kfree(kip);
 		}
@@ -248,29 +254,59 @@ void __free_insn_slot(struct kprobe_insn_cache *c,
 		      kprobe_opcode_t *slot, int dirty)
 {
 	struct kprobe_insn_page *kip;
+	long idx;
 
 	mutex_lock(&c->mutex);
-	list_for_each_entry(kip, &c->pages, list) {
-		long idx = ((long)slot - (long)kip->insns) /
-				(c->insn_size * sizeof(kprobe_opcode_t));
-		if (idx >= 0 && idx < slots_per_page(c)) {
-			WARN_ON(kip->slot_used[idx] != SLOT_USED);
-			if (dirty) {
-				kip->slot_used[idx] = SLOT_DIRTY;
-				kip->ngarbage++;
-				if (++c->nr_garbage > slots_per_page(c))
-					collect_garbage_slots(c);
-			} else
-				collect_one_slot(kip, idx);
+	rcu_read_lock();
+	list_for_each_entry_rcu(kip, &c->pages, list) {
+		idx = ((long)slot - (long)kip->insns) /
+			(c->insn_size * sizeof(kprobe_opcode_t));
+		if (idx >= 0 && idx < slots_per_page(c))
 			goto out;
-		}
 	}
-	/* Could not free this slot. */
+	/* Could not find this slot. */
 	WARN_ON(1);
+	kip = NULL;
 out:
+	rcu_read_unlock();
+	/* Mark and sweep: this may sleep */
+	if (kip) {
+		/* Check double free */
+		WARN_ON(kip->slot_used[idx] != SLOT_USED);
+		if (dirty) {
+			kip->slot_used[idx] = SLOT_DIRTY;
+			kip->ngarbage++;
+			if (++c->nr_garbage > slots_per_page(c))
+				collect_garbage_slots(c);
+		} else
+			collect_one_slot(kip, idx);
+	}
 	mutex_unlock(&c->mutex);
 }
 
+/*
+ * Check given address is on the page of kprobe instruction slots.
+ * This will be used for checking whether the address on a stack
+ * is on a text area or not.
+ */
+bool __is_insn_slot_addr(struct kprobe_insn_cache *c, unsigned long addr)
+{
+	struct kprobe_insn_page *kip;
+	bool ret = false;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(kip, &c->pages, list) {
+		if (addr >= (unsigned long)kip->insns &&
+		    addr < (unsigned long)kip->insns + PAGE_SIZE) {
+			ret = true;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return ret;
+}
+
 #ifdef CONFIG_OPTPROBES
 /* For optimized_kprobe buffer */
 struct kprobe_insn_cache kprobe_optinsn_slots = {

[toc] | [prev] | [next] | [standalone]

#1547350 — Re: [PATCH tip/master v2] kprobes: extable: Identify kprobes' insn-slots as kernel text area

[Multipart message — attachments visible in raw view] — view raw

Hi Masami,

[auto build test ERROR on tip/master]

url:    https://github.com/0day-ci/linux/commits/Masami-Hiramatsu/kprobes-extable-Identify-kprobes-insn-slots-as-kernel-text-area/20161226-233830
config: sparc64-defconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   kernel/built-in.o: In function `__kernel_text_address':
>> (.text+0x202b0): undefined reference to `kprobe_insn_slots'
   kernel/built-in.o: In function `__kernel_text_address':
>> (.text+0x202b8): undefined reference to `__is_insn_slot_addr'
   kernel/built-in.o: In function `__kernel_text_address':
   (.text+0x202bc): undefined reference to `kprobe_insn_slots'
   kernel/built-in.o: In function `kernel_text_address':
   (.text+0x20374): undefined reference to `kprobe_insn_slots'
   kernel/built-in.o: In function `kernel_text_address':
   (.text+0x2037c): undefined reference to `__is_insn_slot_addr'
   kernel/built-in.o: In function `kernel_text_address':
   (.text+0x20380): undefined reference to `kprobe_insn_slots'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1547360 — Re: [PATCH tip/master v2] kprobes: extable: Identify kprobes' insn-slots as kernel text area

[Multipart message — attachments visible in raw view] — view raw

Hi Masami,

[auto build test ERROR on tip/master]

url:    https://github.com/0day-ci/linux/commits/Masami-Hiramatsu/kprobes-extable-Identify-kprobes-insn-slots-as-kernel-text-area/20161226-233830
config: sh-r7785rp_defconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sh 

All errors (new ones prefixed by >>):

   kernel/built-in.o: In function `init_kernel_text':
>> kernel/extable.c:64: undefined reference to `__is_insn_slot_addr'
>> kernel/extable.c:64: undefined reference to `kprobe_insn_slots'
   kernel/built-in.o: In function `kernel_text_address':
   kernel/extable.c:134: undefined reference to `__is_insn_slot_addr'
   kernel/extable.c:134: undefined reference to `kprobe_insn_slots'

vim +64 kernel/extable.c

^1da177e Linus Torvalds 2005-04-16  58  		e = search_module_extables(addr);
^1da177e Linus Torvalds 2005-04-16  59  	return e;
^1da177e Linus Torvalds 2005-04-16  60  }
^1da177e Linus Torvalds 2005-04-16  61  
4a44bac1 Ingo Molnar    2009-03-19  62  static inline int init_kernel_text(unsigned long addr)
4a44bac1 Ingo Molnar    2009-03-19  63  {
4a44bac1 Ingo Molnar    2009-03-19 @64  	if (addr >= (unsigned long)_sinittext &&
5ecbe3c3 Helge Deller   2013-11-28  65  	    addr < (unsigned long)_einittext)
4a44bac1 Ingo Molnar    2009-03-19  66  		return 1;
4a44bac1 Ingo Molnar    2009-03-19  67  	return 0;

:::::: The code at line 64 was first introduced by commit
:::::: 4a44bac1f98223ed77e47bf3b42fcfd10cddd85f symbols, stacktrace: look up init symbols after module symbols

:::::: TO: Ingo Molnar <mingo@elte.hu>
:::::: CC: Ingo Molnar <mingo@elte.hu>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1547502 — Re: [PATCH tip/master v2] kprobes: extable: Identify kprobes' insn-slots as kernel text area

Oops, I found I missed an rcu_read_unlock on the fast path...
I'll send v3.

Thanks,


On Tue, 27 Dec 2016 00:34:20 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Make __kernel_text_address()/kernel_text_address() returns
> true if the given address is on a kprobe's instruction slot,
> which is generated by kprobes as a trampoline code.
> This can help stacktraces to determine the address is on a
> text area or not.
> 
> To implement this without any sleep in is_kprobe_*_slot(),
> this also modify insn_cache page list as a rcu list. It may
> increase processing deley (not processing time) for garbage
> slot collection, because it requires to wait an additional
> rcu grance period when freeing a page from the list.
> However, since it is not a hot path, we may not take care of it.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> 
> ---
>  V2: Fix build error when CONFIG_KPROBES=n
> 
>  Hi Josh, could check this patch fixes your issue? It will
>  enable unwinder code to validate return address by using
>  __kernel_text_address() again.
> ---
>  include/linux/kprobes.h |   22 ++++++++++++++-
>  kernel/extable.c        |    9 +++++-
>  kernel/kprobes.c        |   70 ++++++++++++++++++++++++++++++++++++-----------
>  3 files changed, 82 insertions(+), 19 deletions(-)
> 
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 8f68490..f0496b0 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -281,6 +281,9 @@ struct kprobe_insn_cache {
>  extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c);
>  extern void __free_insn_slot(struct kprobe_insn_cache *c,
>  			     kprobe_opcode_t *slot, int dirty);
> +/* sleep-less address checking routine  */
> +extern bool __is_insn_slot_addr(struct kprobe_insn_cache *c,
> +				unsigned long addr);
>  
>  #define DEFINE_INSN_CACHE_OPS(__name)					\
>  extern struct kprobe_insn_cache kprobe_##__name##_slots;		\
> @@ -294,6 +297,11 @@ static inline void free_##__name##_slot(kprobe_opcode_t *slot, int dirty)\
>  {									\
>  	__free_insn_slot(&kprobe_##__name##_slots, slot, dirty);	\
>  }									\
> +									\
> +static inline bool is_kprobe_##__name##_slot(unsigned long addr)	\
> +{									\
> +	return __is_insn_slot_addr(&kprobe_##__name##_slots, addr);	\
> +}
>  
>  DEFINE_INSN_CACHE_OPS(insn);
>  
> @@ -330,7 +338,6 @@ extern int proc_kprobes_optimization_handler(struct ctl_table *table,
>  					     int write, void __user *buffer,
>  					     size_t *length, loff_t *ppos);
>  #endif
> -
>  #endif /* CONFIG_OPTPROBES */
>  #ifdef CONFIG_KPROBES_ON_FTRACE
>  extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> @@ -481,6 +488,19 @@ static inline int enable_jprobe(struct jprobe *jp)
>  	return enable_kprobe(&jp->kp);
>  }
>  
> +#ifndef CONFIG_KPROBES
> +static inline bool is_kprobe_insn_slot(unsigned long addr)
> +{
> +	return false;
> +}
> +#endif
> +#ifndef CONFIG_OPTPROBES
> +static inline bool is_kprobe_optinsn_slot(unsigned long addr)
> +{
> +	return false;
> +}
> +#endif
> +
>  #ifdef CONFIG_KPROBES
>  /*
>   * Blacklist ganerating macro. Specify functions which is not probed
> diff --git a/kernel/extable.c b/kernel/extable.c
> index e820cce..81c9633 100644
> --- a/kernel/extable.c
> +++ b/kernel/extable.c
> @@ -20,6 +20,7 @@
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/init.h>
> +#include <linux/kprobes.h>
>  
>  #include <asm/sections.h>
>  #include <asm/uaccess.h>
> @@ -104,6 +105,8 @@ int __kernel_text_address(unsigned long addr)
>  		return 1;
>  	if (is_ftrace_trampoline(addr))
>  		return 1;
> +	if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr))
> +		return 1;
>  	/*
>  	 * There might be init symbols in saved stacktraces.
>  	 * Give those symbols a chance to be printed in
> @@ -123,7 +126,11 @@ int kernel_text_address(unsigned long addr)
>  		return 1;
>  	if (is_module_text_address(addr))
>  		return 1;
> -	return is_ftrace_trampoline(addr);
> +	if (is_ftrace_trampoline(addr))
> +		return 1;
> +	if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr))
> +		return 1;
> +	return 0;
>  }
>  
>  /*
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index d630954..1bd1c17 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -149,9 +149,11 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
>  	struct kprobe_insn_page *kip;
>  	kprobe_opcode_t *slot = NULL;
>  
> +	/* Since the slot array is not protected by rcu, we need a mutex */
>  	mutex_lock(&c->mutex);
>   retry:
> -	list_for_each_entry(kip, &c->pages, list) {
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(kip, &c->pages, list) {
>  		if (kip->nused < slots_per_page(c)) {
>  			int i;
>  			for (i = 0; i < slots_per_page(c); i++) {
> @@ -167,6 +169,7 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
>  			WARN_ON(1);
>  		}
>  	}
> +	rcu_read_unlock();
>  
>  	/* If there are any garbage slots, collect it and try again. */
>  	if (c->nr_garbage && collect_garbage_slots(c) == 0)
> @@ -193,13 +196,15 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
>  	kip->nused = 1;
>  	kip->ngarbage = 0;
>  	kip->cache = c;
> -	list_add(&kip->list, &c->pages);
> +	list_add_rcu(&kip->list, &c->pages);
>  	slot = kip->insns;
>  out:
>  	mutex_unlock(&c->mutex);
>  	return slot;
>  }
>  
> +
> +
>  /* Return 1 if all garbages are collected, otherwise 0. */
>  static int collect_one_slot(struct kprobe_insn_page *kip, int idx)
>  {
> @@ -213,7 +218,8 @@ static int collect_one_slot(struct kprobe_insn_page *kip, int idx)
>  		 * next time somebody inserts a probe.
>  		 */
>  		if (!list_is_singular(&kip->list)) {
> -			list_del(&kip->list);
> +			list_del_rcu(&kip->list);
> +			synchronize_rcu();
>  			kip->cache->free(kip->insns);
>  			kfree(kip);
>  		}
> @@ -248,29 +254,59 @@ void __free_insn_slot(struct kprobe_insn_cache *c,
>  		      kprobe_opcode_t *slot, int dirty)
>  {
>  	struct kprobe_insn_page *kip;
> +	long idx;
>  
>  	mutex_lock(&c->mutex);
> -	list_for_each_entry(kip, &c->pages, list) {
> -		long idx = ((long)slot - (long)kip->insns) /
> -				(c->insn_size * sizeof(kprobe_opcode_t));
> -		if (idx >= 0 && idx < slots_per_page(c)) {
> -			WARN_ON(kip->slot_used[idx] != SLOT_USED);
> -			if (dirty) {
> -				kip->slot_used[idx] = SLOT_DIRTY;
> -				kip->ngarbage++;
> -				if (++c->nr_garbage > slots_per_page(c))
> -					collect_garbage_slots(c);
> -			} else
> -				collect_one_slot(kip, idx);
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(kip, &c->pages, list) {
> +		idx = ((long)slot - (long)kip->insns) /
> +			(c->insn_size * sizeof(kprobe_opcode_t));
> +		if (idx >= 0 && idx < slots_per_page(c))
>  			goto out;
> -		}
>  	}
> -	/* Could not free this slot. */
> +	/* Could not find this slot. */
>  	WARN_ON(1);
> +	kip = NULL;
>  out:
> +	rcu_read_unlock();
> +	/* Mark and sweep: this may sleep */
> +	if (kip) {
> +		/* Check double free */
> +		WARN_ON(kip->slot_used[idx] != SLOT_USED);
> +		if (dirty) {
> +			kip->slot_used[idx] = SLOT_DIRTY;
> +			kip->ngarbage++;
> +			if (++c->nr_garbage > slots_per_page(c))
> +				collect_garbage_slots(c);
> +		} else
> +			collect_one_slot(kip, idx);
> +	}
>  	mutex_unlock(&c->mutex);
>  }
>  
> +/*
> + * Check given address is on the page of kprobe instruction slots.
> + * This will be used for checking whether the address on a stack
> + * is on a text area or not.
> + */
> +bool __is_insn_slot_addr(struct kprobe_insn_cache *c, unsigned long addr)
> +{
> +	struct kprobe_insn_page *kip;
> +	bool ret = false;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(kip, &c->pages, list) {
> +		if (addr >= (unsigned long)kip->insns &&
> +		    addr < (unsigned long)kip->insns + PAGE_SIZE) {
> +			ret = true;
> +			break;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return ret;
> +}
> +
>  #ifdef CONFIG_OPTPROBES
>  /* For optimized_kprobe buffer */
>  struct kprobe_insn_cache kprobe_optinsn_slots = {
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1547501 — [PATCH tip/master v3] kprobes: extable: Identify kprobes' insn-slots as kernel text area

Make __kernel_text_address()/kernel_text_address() returns
true if the given address is on a kprobe's instruction slot,
which is generated by kprobes as a trampoline code.
This can help stacktraces to determine the address is on a
text area or not.

To implement this without any sleep in is_kprobe_*_slot(),
this also modify insn_cache page list as a rcu list. It may
increase processing deley (not processing time) for garbage
slot collection, because it requires to wait an additional
rcu grance period when freeing a page from the list.
However, since it is not a hot path, we may not take care of it.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

---
 V3:
  - Fix build error on archs which don't need insn_slot
     (e.g. sh, sparc).
  - Fix a missed rcu_read_unlock() in fast path of __get_insn_slot.
---
 include/linux/kprobes.h |   30 ++++++++++++++++++++
 kernel/extable.c        |    9 +++++-
 kernel/kprobes.c        |   69 +++++++++++++++++++++++++++++++++++------------
 3 files changed, 89 insertions(+), 19 deletions(-)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 8f68490..16ddfb8 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -278,9 +278,13 @@ struct kprobe_insn_cache {
 	int nr_garbage;
 };
 
+#ifdef __ARCH_WANT_KPROBES_INSN_SLOT
 extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c);
 extern void __free_insn_slot(struct kprobe_insn_cache *c,
 			     kprobe_opcode_t *slot, int dirty);
+/* sleep-less address checking routine  */
+extern bool __is_insn_slot_addr(struct kprobe_insn_cache *c,
+				unsigned long addr);
 
 #define DEFINE_INSN_CACHE_OPS(__name)					\
 extern struct kprobe_insn_cache kprobe_##__name##_slots;		\
@@ -294,6 +298,18 @@ static inline void free_##__name##_slot(kprobe_opcode_t *slot, int dirty)\
 {									\
 	__free_insn_slot(&kprobe_##__name##_slots, slot, dirty);	\
 }									\
+									\
+static inline bool is_kprobe_##__name##_slot(unsigned long addr)	\
+{									\
+	return __is_insn_slot_addr(&kprobe_##__name##_slots, addr);	\
+}
+#else /* __ARCH_WANT_KPROBES_INSN_SLOT */
+#define DEFINE_INSN_CACHE_OPS(__name)					\
+static inline bool is_kprobe_##__name##_slot(unsigned long addr)	\
+{									\
+	return 0;							\
+}
+#endif
 
 DEFINE_INSN_CACHE_OPS(insn);
 
@@ -330,7 +346,6 @@ extern int proc_kprobes_optimization_handler(struct ctl_table *table,
 					     int write, void __user *buffer,
 					     size_t *length, loff_t *ppos);
 #endif
-
 #endif /* CONFIG_OPTPROBES */
 #ifdef CONFIG_KPROBES_ON_FTRACE
 extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
@@ -481,6 +496,19 @@ static inline int enable_jprobe(struct jprobe *jp)
 	return enable_kprobe(&jp->kp);
 }
 
+#ifndef CONFIG_KPROBES
+static inline bool is_kprobe_insn_slot(unsigned long addr)
+{
+	return false;
+}
+#endif
+#ifndef CONFIG_OPTPROBES
+static inline bool is_kprobe_optinsn_slot(unsigned long addr)
+{
+	return false;
+}
+#endif
+
 #ifdef CONFIG_KPROBES
 /*
  * Blacklist ganerating macro. Specify functions which is not probed
diff --git a/kernel/extable.c b/kernel/extable.c
index e820cce..81c9633 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/init.h>
+#include <linux/kprobes.h>
 
 #include <asm/sections.h>
 #include <asm/uaccess.h>
@@ -104,6 +105,8 @@ int __kernel_text_address(unsigned long addr)
 		return 1;
 	if (is_ftrace_trampoline(addr))
 		return 1;
+	if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr))
+		return 1;
 	/*
 	 * There might be init symbols in saved stacktraces.
 	 * Give those symbols a chance to be printed in
@@ -123,7 +126,11 @@ int kernel_text_address(unsigned long addr)
 		return 1;
 	if (is_module_text_address(addr))
 		return 1;
-	return is_ftrace_trampoline(addr);
+	if (is_ftrace_trampoline(addr))
+		return 1;
+	if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr))
+		return 1;
+	return 0;
 }
 
 /*
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index d630954..be41f6d 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -149,9 +149,11 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
 	struct kprobe_insn_page *kip;
 	kprobe_opcode_t *slot = NULL;
 
+	/* Since the slot array is not protected by rcu, we need a mutex */
 	mutex_lock(&c->mutex);
  retry:
-	list_for_each_entry(kip, &c->pages, list) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(kip, &c->pages, list) {
 		if (kip->nused < slots_per_page(c)) {
 			int i;
 			for (i = 0; i < slots_per_page(c); i++) {
@@ -159,6 +161,7 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
 					kip->slot_used[i] = SLOT_USED;
 					kip->nused++;
 					slot = kip->insns + (i * c->insn_size);
+					rcu_read_unlock();
 					goto out;
 				}
 			}
@@ -167,6 +170,7 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
 			WARN_ON(1);
 		}
 	}
+	rcu_read_unlock();
 
 	/* If there are any garbage slots, collect it and try again. */
 	if (c->nr_garbage && collect_garbage_slots(c) == 0)
@@ -193,7 +197,7 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c)
 	kip->nused = 1;
 	kip->ngarbage = 0;
 	kip->cache = c;
-	list_add(&kip->list, &c->pages);
+	list_add_rcu(&kip->list, &c->pages);
 	slot = kip->insns;
 out:
 	mutex_unlock(&c->mutex);
@@ -213,7 +217,8 @@ static int collect_one_slot(struct kprobe_insn_page *kip, int idx)
 		 * next time somebody inserts a probe.
 		 */
 		if (!list_is_singular(&kip->list)) {
-			list_del(&kip->list);
+			list_del_rcu(&kip->list);
+			synchronize_rcu();
 			kip->cache->free(kip->insns);
 			kfree(kip);
 		}
@@ -248,29 +253,59 @@ void __free_insn_slot(struct kprobe_insn_cache *c,
 		      kprobe_opcode_t *slot, int dirty)
 {
 	struct kprobe_insn_page *kip;
+	long idx;
 
 	mutex_lock(&c->mutex);
-	list_for_each_entry(kip, &c->pages, list) {
-		long idx = ((long)slot - (long)kip->insns) /
-				(c->insn_size * sizeof(kprobe_opcode_t));
-		if (idx >= 0 && idx < slots_per_page(c)) {
-			WARN_ON(kip->slot_used[idx] != SLOT_USED);
-			if (dirty) {
-				kip->slot_used[idx] = SLOT_DIRTY;
-				kip->ngarbage++;
-				if (++c->nr_garbage > slots_per_page(c))
-					collect_garbage_slots(c);
-			} else
-				collect_one_slot(kip, idx);
+	rcu_read_lock();
+	list_for_each_entry_rcu(kip, &c->pages, list) {
+		idx = ((long)slot - (long)kip->insns) /
+			(c->insn_size * sizeof(kprobe_opcode_t));
+		if (idx >= 0 && idx < slots_per_page(c))
 			goto out;
-		}
 	}
-	/* Could not free this slot. */
+	/* Could not find this slot. */
 	WARN_ON(1);
+	kip = NULL;
 out:
+	rcu_read_unlock();
+	/* Mark and sweep: this may sleep */
+	if (kip) {
+		/* Check double free */
+		WARN_ON(kip->slot_used[idx] != SLOT_USED);
+		if (dirty) {
+			kip->slot_used[idx] = SLOT_DIRTY;
+			kip->ngarbage++;
+			if (++c->nr_garbage > slots_per_page(c))
+				collect_garbage_slots(c);
+		} else
+			collect_one_slot(kip, idx);
+	}
 	mutex_unlock(&c->mutex);
 }
 
+/*
+ * Check given address is on the page of kprobe instruction slots.
+ * This will be used for checking whether the address on a stack
+ * is on a text area or not.
+ */
+bool __is_insn_slot_addr(struct kprobe_insn_cache *c, unsigned long addr)
+{
+	struct kprobe_insn_page *kip;
+	bool ret = false;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(kip, &c->pages, list) {
+		if (addr >= (unsigned long)kip->insns &&
+		    addr < (unsigned long)kip->insns + PAGE_SIZE) {
+			ret = true;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return ret;
+}
+
 #ifdef CONFIG_OPTPROBES
 /* For optimized_kprobe buffer */
 struct kprobe_insn_cache kprobe_optinsn_slots = {

[toc] | [prev] | [next] | [standalone]

#1549694 — Re: [PATCH tip/master v3] kprobes: extable: Identify kprobes' insn-slots as kernel text area

On Tue, Dec 27, 2016 at 03:14:10PM +0900, Masami Hiramatsu wrote:

> diff --git a/kernel/extable.c b/kernel/extable.c
> index e820cce..81c9633 100644
> --- a/kernel/extable.c
> +++ b/kernel/extable.c

> @@ -123,7 +126,11 @@ int kernel_text_address(unsigned long addr)
>  		return 1;
>  	if (is_module_text_address(addr))
>  		return 1;
> -	return is_ftrace_trampoline(addr);
> +	if (is_ftrace_trampoline(addr))
> +		return 1;
> +	if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr))
> +		return 1;
> +	return 0;
>  }

> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index d630954..be41f6d 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c

> +/*
> + * Check given address is on the page of kprobe instruction slots.
> + * This will be used for checking whether the address on a stack
> + * is on a text area or not.
> + */
> +bool __is_insn_slot_addr(struct kprobe_insn_cache *c, unsigned long addr)
> +{
> +	struct kprobe_insn_page *kip;
> +	bool ret = false;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(kip, &c->pages, list) {
> +		if (addr >= (unsigned long)kip->insns &&
> +		    addr < (unsigned long)kip->insns + PAGE_SIZE) {
> +			ret = true;
> +			break;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return ret;
> +}

How many entries should one expect on that list? I spend quite a bit of
time reducing the cost of is_module_text_address() a while back and see
that both ftrace (which actually needs this to be fast) and now
kprobes have linear list walks in here.

I'm assuming the ftrace thing to be mostly empty, since I never saw it
on my benchmarks back then, but it is something Steve should look at I
suppose.

Similarly, the changelog here should include some talk about worst case
costs.

FWIW, see commit: 93c2e105f6bc ("module: Optimize __module_address() using a latched RB-tree")

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web