diff mbox

[RESEND,v5,BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64

Message ID 20140717071121.3130.86341.stgit@kbuild-fedora.novalocal (mailing list archive)
State Not Applicable
Headers show

Commit Message

Masami Hiramatsu July 17, 2014, 7:11 a.m. UTC 
On ia64 and ppc64, the function pointer does not point the
entry address of the function, but the address of function
discriptor (which contains the entry address and misc
data.) Since the kprobes passes the function pointer stored
by NOKPROBE_SYMBOL() to kallsyms_lookup_size_offset() for
initalizing its blacklist, it fails and reports many errors
as below.

  Failed to find blacklist 0001013168300000
  Failed to find blacklist 0001013000f0a000
  Failed to find blacklist 000101315f70a000
  Failed to find blacklist 000101324c80a000
  Failed to find blacklist 0001013063f0a000
  Failed to find blacklist 000101327800a000
  Failed to find blacklist 0001013277f0a000
  Failed to find blacklist 000101315a70a000
  Failed to find blacklist 0001013277e0a000
  Failed to find blacklist 000101305a20a000
  Failed to find blacklist 0001013277d0a000
  Failed to find blacklist 00010130bdc0a000
  Failed to find blacklist 00010130dc20a000
  Failed to find blacklist 000101309a00a000
  Failed to find blacklist 0001013277c0a000
  Failed to find blacklist 0001013277b0a000
  Failed to find blacklist 0001013277a0a000
  Failed to find blacklist 000101327790a000
  Failed to find blacklist 000101303140a000
  Failed to find blacklist 0001013a3280a000

To fix this bug, this introduces function_entry() macro to
retrieve the entry address from the given function pointer,
and uses for kallsyms_lookup_size_offset() while initializing
blacklist.

Changes in v5:
 - Use arch_deref_entry_point() instead of function_entry().

Changes in v4:
 - Add kernel_text_address() check for verifying the address.
 - Moved on the latest linus tree.

Changes in v3:
 - Fix a bug to get blacklist address based on function entry
   instead of function descriptor. (Suzuki's work, Thanks!)

Changes in V2:
 - Use function_entry() macro when lookin up symbols instead
   of storing it.
 - Update for the latest -next.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>
Reported-by: Tony Luck <tony.luck@gmail.com>
Tested-by: Tony Luck <tony.luck@intel.com>
Tested-by: Michael Ellerman <mpe@ellerman.id.au>
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (for powerpc)
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: linux-ia64@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
---
 kernel/kprobes.c |   15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Comments

Ingo Molnar July 17, 2014, 9:38 a.m. UTC | #1 
* Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>

Looks good, but this is not a valid SOB sequence: if Suzuki wrote the 
patch then he should be the first SOB (and should have a From line as 
well), if he acked it along the way then it should be an Acked-by - or 
Reviewed-by.

This fix should be in v3.16 as well, right?

Thanks,

	Ingo
Masami Hiramatsu July 17, 2014, 11:17 a.m. UTC | #2 
(2014/07/17 18:38), Ingo Molnar wrote:
> 
> * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> 
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>
> 
> Looks good, but this is not a valid SOB sequence: if Suzuki wrote the 
> patch then he should be the first SOB (and should have a From line as 
> well), if he acked it along the way then it should be an Acked-by - or 
> Reviewed-by.

Ah, OK, so I'll add his name as Fixed-by:.

> 
> This fix should be in v3.16 as well, right?

Right, it should be.

Thank you,
Masami Hiramatsu July 17, 2014, 11:40 a.m. UTC | #3 
(2014/07/17 20:17), Masami Hiramatsu wrote:
> (2014/07/17 18:38), Ingo Molnar wrote:
>>
>> * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>>
>>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>>> Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>
>>
>> Looks good, but this is not a valid SOB sequence: if Suzuki wrote the 
>> patch then he should be the first SOB (and should have a From line as 
>> well), if he acked it along the way then it should be an Acked-by - or 
>> Reviewed-by.
> 
> Ah, OK, so I'll add his name as Fixed-by:.

Oops, I also found that an old comment in the patch description
and unneeded "include". I'll fix and send that asap.

Thank you,
diff mbox

Patch

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 3214289..ec370cc 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -32,6 +32,7 @@ 
  *		<prasanna@in.ibm.com> added function-return probes.
  */
 #include <linux/kprobes.h>
+#include <linux/types.h>
 #include <linux/hash.h>
 #include <linux/init.h>
 #include <linux/slab.h>
@@ -2037,19 +2038,23 @@  static int __init populate_kprobe_blacklist(unsigned long *start,
 {
 	unsigned long *iter;
 	struct kprobe_blacklist_entry *ent;
-	unsigned long offset = 0, size = 0;
+	unsigned long entry, offset = 0, size = 0;
 
 	for (iter = start; iter < end; iter++) {
-		if (!kallsyms_lookup_size_offset(*iter, &size, &offset)) {
-			pr_err("Failed to find blacklist %p\n", (void *)*iter);
+		entry = arch_deref_entry_point((void *)*iter);
+
+		if (!kernel_text_address(entry) ||
+		    !kallsyms_lookup_size_offset(entry, &size, &offset)) {
+			pr_err("Failed to find blacklist at %p\n",
+				(void *)entry);
 			continue;
 		}
 
 		ent = kmalloc(sizeof(*ent), GFP_KERNEL);
 		if (!ent)
 			return -ENOMEM;
-		ent->start_addr = *iter;
-		ent->end_addr = *iter + size;
+		ent->start_addr = entry;
+		ent->end_addr = entry + size;
 		INIT_LIST_HEAD(&ent->list);
 		list_add_tail(&ent->list, &kprobe_blacklist);
 	}