Skip to contentSkip to conversation

Kprobes: Make kprobe modules more portable

PATCH In review Ananth N Mavinakayanahalli 9 participants 29 messages Aug 7, 2006 – Aug 16, 2006

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

PATCH 1/3 Kprobes: Make kprobe modules more portable

From: Ananth N Mavinakayanahalli <[email protected]>

This patch introduces KPROBE_ADDR, a macro that abstracts out the architecture-specific artefacts of getting the correct text address given a symbol. While we are at it, also introduce the symbol_name field in struct kprobe to allow for users to just specify the address to be probed in terms of the kernel symbol. In-kernel kprobes infrastructure decodes the actual text address to probe. The symbol resolution happens only if the kprobe.addr isn't explicitly specified.

---

---
 include/asm-powerpc/kprobes.h |    2 ++
 include/linux/kprobes.h       |    8 ++++++++
 kernel/kprobes.c              |    4 ++++
 3 files changed, 14 insertions(+)
include/asm-powerpc/kprobes.h +2 -0
Index: linux-2.6.18-rc3/include/asm-powerpc/kprobes.h
===================================================================
@@ -44,6 +44,8 @@ typedef unsigned int kprobe_opcode_t;
 #define IS_TDI(instr)		(((instr) & 0xfc000000) == 0x08000000)
 #define IS_TWI(instr)		(((instr) & 0xfc000000) == 0x0c000000)
 
+#define KPROBE_ADDR(name) *((kprobe_opcode_t **)(kallsyms_lookup_name(name)))
+
 #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)((func_descr_t *)pentry)
 
 #define is_trap(instr)	(IS_TW(instr) || IS_TD(instr) || \
include/linux/kprobes.h +8 -0
Index: linux-2.6.18-rc3/include/linux/kprobes.h
===================================================================
@@ -36,6 +36,7 @@
 #include <linux/spinlock.h>
 #include <linux/rcupdate.h>
 #include <linux/mutex.h>
+#include <linux/kallsyms.h>
 
 #ifdef CONFIG_KPROBES
 #include <asm/kprobes.h>
@@ -49,6 +50,10 @@
 /* Attach to insert probes on any functions which should be ignored*/
 #define __kprobes	__attribute__((__section__(".kprobes.text")))
 
+#ifndef KPROBE_ADDR	/* powerpc has its own definition */
+#define KPROBE_ADDR(name) (kprobe_opcode_t *)(kallsyms_lookup_name(name))
+#endif	/* KPROBE_ADDR */
+
 struct kprobe;
 struct pt_regs;
 struct kretprobe;
@@ -77,6 +82,9 @@ struct kprobe {
 	/* location of the probe point */
 	kprobe_opcode_t *addr;
 
+	/* Allow user to indicate symbol name of the probe point */
+	char *symbol_name;
+
 	/* Called before addr is executed. */
 	kprobe_pre_handler_t pre_handler;
 
kernel/kprobes.c +4 -0
Index: linux-2.6.18-rc3/kernel/kprobes.c
===================================================================
@@ -446,6 +446,10 @@ static int __kprobes __register_kprobe(s
 	struct kprobe *old_p;
 	struct module *probed_mod;
 
+	/* Do the kallsyms lookup only if p->addr == NULL */
+	if (!p->addr && (p->symbol_name))
+		p->addr = KPROBE_ADDR(p->symbol_name);
+
 	if ((!kernel_text_address((unsigned long) p->addr)) ||
 		in_kprobes_functions((unsigned long) p->addr))
 		return -EINVAL;
#1 lore raw

PATCH 2/3 Kprobes: Define retval helper

From: Ananth N Mavinakayanahalli <[email protected]>

Add the KPROBE_RETVAL macro to help extract the return value on different architectures, while using function-return probes.

---

---
 include/asm-i386/kprobes.h    |    2 ++
 include/asm-ia64/kprobes.h    |    2 ++
 include/asm-powerpc/kprobes.h |    2 ++
 include/asm-x86_64/kprobes.h  |    2 ++
 4 files changed, 8 insertions(+)
include/asm-i386/kprobes.h +2 -0
Index: linux-2.6.18-rc3/include/asm-i386/kprobes.h
===================================================================
@@ -46,6 +46,8 @@ typedef u8 kprobe_opcode_t;
 #define ARCH_SUPPORTS_KRETPROBES
 #define  ARCH_INACTIVE_KPROBE_COUNT 0
 
+#define KPROBE_RETVAL(regs)	regs->eax
+
 void arch_remove_kprobe(struct kprobe *p);
 void kretprobe_trampoline(void);
 
include/asm-ia64/kprobes.h +2 -0
Index: linux-2.6.18-rc3/include/asm-ia64/kprobes.h
===================================================================
@@ -84,6 +84,8 @@ struct kprobe_ctlblk {
 #define ARCH_SUPPORTS_KRETPROBES
 #define  ARCH_INACTIVE_KPROBE_COUNT 1
 
+#define KPROBE_RETVAL(regs)	regs->r8
+
 #define SLOT0_OPCODE_SHIFT	(37)
 #define SLOT1_p1_OPCODE_SHIFT	(37 - (64-46))
 #define SLOT2_OPCODE_SHIFT 	(37)
include/asm-powerpc/kprobes.h +2 -0
Index: linux-2.6.18-rc3/include/asm-powerpc/kprobes.h
===================================================================
@@ -54,6 +54,8 @@ typedef unsigned int kprobe_opcode_t;
 #define ARCH_SUPPORTS_KRETPROBES
 #define  ARCH_INACTIVE_KPROBE_COUNT 1
 
+#define KPROBE_RETVAL(regs)	regs->gpr[3]
+
 void kretprobe_trampoline(void);
 extern void arch_remove_kprobe(struct kprobe *p);
 
include/asm-x86_64/kprobes.h +2 -0
Index: linux-2.6.18-rc3/include/asm-x86_64/kprobes.h
===================================================================
@@ -45,6 +45,8 @@ typedef u8 kprobe_opcode_t;
 #define ARCH_SUPPORTS_KRETPROBES
 #define  ARCH_INACTIVE_KPROBE_COUNT 1
 
+#define KPROBE_RETVAL(regs)	regs->rax
+
 void kretprobe_trampoline(void);
 extern void arch_remove_kprobe(struct kprobe *p);
 
#2 lore raw

PATCH 3/3 Kprobes: Update Documentation/kprobes.txt

From: Ananth N Mavinakayanahalli <[email protected]>

Update Documentation/kprobes.txt to reflect addition of KPROBE_ADDR, KPROBE_RETVAL and the in-kernel symbol resolution.

While at it:
- Document usage of JPROBE_RETURN
- Update the references list
- Update module examples to use module_init/module_exit interfaces

---

---
 Documentation/kprobes.txt |   77 ++++++++++++++++++++++++++++------------------
 1 files changed, 47 insertions(+), 30 deletions(-)
Documentation/kprobes.txt +47 -30
Index: linux-2.6.18-rc3/Documentation/kprobes.txt
===================================================================
@@ -179,6 +179,21 @@ occurs during execution of kp->pre_handl
 or during single-stepping of the probed instruction, Kprobes calls
 kp->fault_handler.  Any or all handlers can be NULL.
 
+NOTE:
+1. The KPROBE_ADDR macro is provided as an aid to make kprobe modules
+portable. Some architectures (notably powerpc) uses function descriptors
+due to which a kallsyms_lookup_name("probepoint") will give a data address,
+which, incidentally, is a placeholder for the actual text address. It is
+therefore advised to use:
+
+	kp.addr = KPROBE_ADDR("probepoint");
+
+2. With the introduction of the "symbol_name" field to struct kprobe,
+the probepoint address resolution will now be taken care of by the kernel.
+The following will also work:
+
+	kp.symbol_name = "symbol_name";
+
 register_kprobe() returns 0 on success, or a negative errno otherwise.
 
 User's pre-handler (kp->pre_handler):
@@ -225,6 +240,12 @@ control to Kprobes.)  If the probed func
 fastcall, or anything else that affects how args are passed, the
 handler's declaration must match.
 
+NOTE: Similar to KPROBE_ADDR, a macro JPROBE_ENTRY is provided to handle
+architecture-specific aliasing of jp->entry. In the interest of
+portability, it is advised to use:
+
+	jp->entry = JPROBE_ENTRY(handler);
+
 register_jprobe() returns 0 on success, or a negative errno otherwise.
 
 4.3 register_kretprobe
@@ -251,6 +272,11 @@ of interest:
 - ret_addr: the return address
 - rp: points to the corresponding kretprobe object
 - task: points to the corresponding task struct
+
+The KPROBE_RETVAL(regs) macro provides a simple abstraction to extract
+the return value from the appropriate register as defined by the
+architecture's ABI.
+
 The handler's return value is currently ignored.
 
 4.4 unregister_*probe
@@ -369,7 +395,6 @@ stack trace and selected i386 registers 
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/kprobes.h>
-#include <linux/kallsyms.h>
 #include <linux/sched.h>
 
 /*For each probe you need to allocate a kprobe structure*/
@@ -403,18 +428,14 @@ int handler_fault(struct kprobe *p, stru
 	return 0;
 }
 
-int init_module(void)
+static int __init kprobe_init(void)
 {
 	int ret;
 	kp.pre_handler = handler_pre;
 	kp.post_handler = handler_post;
 	kp.fault_handler = handler_fault;
-	kp.addr = (kprobe_opcode_t*) kallsyms_lookup_name("do_fork");
-	/* register the kprobe now */
-	if (!kp.addr) {
-		printk("Couldn't find %s to plant kprobe\n", "do_fork");
-		return -1;
-	}
+	kp.symbol_name = "do_fork";
+
 	if ((ret = register_kprobe(&kp) < 0)) {
 		printk("register_kprobe failed, returned %d\n", ret);
 		return -1;
@@ -423,12 +444,14 @@ int init_module(void)
 	return 0;
 }
 
-void cleanup_module(void)
+static void __exit kprobe_exit(void)
 {
 	unregister_kprobe(&kp);
 	printk("kprobe unregistered\n");
 }
 
+module_init(kprobe_init)
+module_exit(kprobe_exit)
 MODULE_LICENSE("GPL");
 ----- cut here -----
 
@@ -463,7 +486,6 @@ the arguments of do_fork().
 #include <linux/fs.h>
 #include <linux/uio.h>
 #include <linux/kprobes.h>
-#include <linux/kallsyms.h>
 
 /*
  * Jumper probe for do_fork.
@@ -485,17 +507,13 @@ long jdo_fork(unsigned long clone_flags,
 }
 
 static struct jprobe my_jprobe = {
-	.entry = (kprobe_opcode_t *) jdo_fork
+	.entry = JPROBE_ENTRY(jdo_fork)
 };
 
-int init_module(void)
+static int __init jprobe_init(void)
 {
 	int ret;
-	my_jprobe.kp.addr = (kprobe_opcode_t *) kallsyms_lookup_name("do_fork");
-	if (!my_jprobe.kp.addr) {
-		printk("Couldn't find %s to plant jprobe\n", "do_fork");
-		return -1;
-	}
+	my_jprobe.kp.symbol_name = "do_fork";
 
 	if ((ret = register_jprobe(&my_jprobe)) <0) {
 		printk("register_jprobe failed, returned %d\n", ret);
@@ -506,12 +524,14 @@ int init_module(void)
 	return 0;
 }
 
-void cleanup_module(void)
+static void __exit jprobe_exit(void)
 {
 	unregister_jprobe(&my_jprobe);
 	printk("jprobe unregistered\n");
 }
 
+module_init(jprobe_init)
+module_exit(jprobe_exit)
 MODULE_LICENSE("GPL");
 ----- cut here -----
 
@@ -530,16 +550,13 @@ report failed calls to sys_open().
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/kprobes.h>
-#include <linux/kallsyms.h>
 
 static const char *probed_func = "sys_open";
 
 /* Return-probe handler: If the probed function fails, log the return value. */
 static int ret_handler(struct kretprobe_instance *ri, struct pt_regs *regs)
 {
-	// Substitute the appropriate register name for your architecture --
-	// e.g., regs->rax for x86_64, regs->gpr[3] for ppc64.
-	int retval = (int) regs->eax;
+	int retval = KPROBE_RETVAL(regs);
 	if (retval < 0) {
 		printk("%s returns %d\n", probed_func, retval);
 	}
@@ -552,15 +569,11 @@ static struct kretprobe my_kretprobe = {
 	.maxactive = 20
 };
 
-int init_module(void)
+static int __init kretprobe_init(void)
 {
 	int ret;
-	my_kretprobe.kp.addr =
-		(kprobe_opcode_t *) kallsyms_lookup_name(probed_func);
-	if (!my_kretprobe.kp.addr) {
-		printk("Couldn't find %s to plant return probe\n", probed_func);
-		return -1;
-	}
+	my_kretprobe.kp.symbol_name = (char *)probed_func;
+
 	if ((ret = register_kretprobe(&my_kretprobe)) < 0) {
 		printk("register_kretprobe failed, returned %d\n", ret);
 		return -1;
@@ -569,7 +582,7 @@ int init_module(void)
 	return 0;
 }
 
-void cleanup_module(void)
+static void __exit kretprobe_exit(void)
 {
 	unregister_kretprobe(&my_kretprobe);
 	printk("kretprobe unregistered\n");
@@ -578,6 +591,8 @@ void cleanup_module(void)
 		my_kretprobe.nmissed, probed_func);
 }
 
+module_init(kretprobe_init)
+module_exit(kretprobe_exit)
 MODULE_LICENSE("GPL");
 ----- cut here -----
 
@@ -590,3 +605,5 @@ messages.)
 For additional information on Kprobes, refer to the following URLs:
 http://www-106.ibm.com/developerworks/library/l-kprobes.html?ca=dgr-lnxw42Kprobe
 http://www.redhat.com/magazine/005mar05/features/kprobes/
+http://www-users.cs.umn.edu/~boutcher/kprobes/
+http://www.linuxsymposium.org/2006/linuxsymposium_procv2.pdf (pages 101-115)
#3 lore raw

Re: [PATCH 3/3] Kprobes: Update Documentation/kprobes.txt

Quoted 4 lines · Ananth N Mavinakayanahalli

From: Ananth N Mavinakayanahalli <[email protected]>

Update Documentation/kprobes.txt to reflect addition of KPROBE_ADDR, KPROBE_RETVAL and the in-kernel symbol resolution.

Thanks. With my updated patch we shouldn't document KPROBE_ADDR anymore but tell people to always use the symbol_name mechanisms.

Any chance to add some kerneldoc comments for the exported kprobes functions?

#4 lore raw

Re: [PATCH 3/3] Kprobes: Update Documentation/kprobes.txt

Quoted 8 lines · Christoph Hellwig
Quoted 4 lines · Ananth N Mavinakayanahalli

From: Ananth N Mavinakayanahalli <[email protected]>

Update Documentation/kprobes.txt to reflect addition of KPROBE_ADDR, KPROBE_RETVAL and the in-kernel symbol resolution.

Thanks. With my updated patch we shouldn't document KPROBE_ADDR anymore but tell people to always use the symbol_name mechanisms.

Done! Updated patch to documentation below.

Quoted 2 lines

Any chance to add some kerneldoc comments for the exported kprobes functions?

Its on the TODO list along with the other CodingStyle cleanups.

Ananth
---

Update Documentation/kprobes.txt:
- Add usage details of "symbol_name" and "offset" fields of struct kprobe
- Document JPROBE_ENTRY
- Update references list
- Update module examples to use module_init/module_exit interfaces

---
 Documentation/kprobes.txt |   77 ++++++++++++++++++++++++++++------------------
 1 files changed, 47 insertions(+), 30 deletions(-)
Documentation/kprobes.txt +47 -30
Index: linux-2.6.18-rc3/Documentation/kprobes.txt
===================================================================
@@ -179,6 +179,21 @@ occurs during execution of kp->pre_handl
 or during single-stepping of the probed instruction, Kprobes calls
 kp->fault_handler.  Any or all handlers can be NULL.
 
+NOTE:
+1. With the introduction of the "symbol_name" field to struct kprobe,
+the probepoint address resolution will now be taken care of by the kernel.
+The following will now work:
+
+	kp.symbol_name = "symbol_name";
+
+2. Use the "offset" field of struct kprobe if the offset into the symbol
+to install a probepoint is known. This field is used to calculate the
+probepoint address only if the "symbol_name" method of address resolution
+is used.
+
+3. Specify either the kprobe "symbol_name" with "offset" OR the "addr".
+If both are specified, kprobe registration will fail with -EINVAL.
+
 register_kprobe() returns 0 on success, or a negative errno otherwise.
 
 User's pre-handler (kp->pre_handler):
@@ -225,6 +240,12 @@ control to Kprobes.)  If the probed func
 fastcall, or anything else that affects how args are passed, the
 handler's declaration must match.
 
+NOTE: A macro JPROBE_ENTRY is provided to handle architecture-specific
+aliasing of jp->entry. In the interest of portability, it is advised
+to use:
+
+	jp->entry = JPROBE_ENTRY(handler);
+
 register_jprobe() returns 0 on success, or a negative errno otherwise.
 
 4.3 register_kretprobe
@@ -251,6 +272,11 @@ of interest:
 - ret_addr: the return address
 - rp: points to the corresponding kretprobe object
 - task: points to the corresponding task struct
+
+The get_retval(regs) macro provides a simple abstraction to extract
+the return value from the appropriate register as defined by the
+architecture's ABI.
+
 The handler's return value is currently ignored.
 
 4.4 unregister_*probe
@@ -369,7 +395,6 @@ stack trace and selected i386 registers 
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/kprobes.h>
-#include <linux/kallsyms.h>
 #include <linux/sched.h>
 
 /*For each probe you need to allocate a kprobe structure*/
@@ -403,18 +428,14 @@ int handler_fault(struct kprobe *p, stru
 	return 0;
 }
 
-int init_module(void)
+static int __init kprobe_init(void)
 {
 	int ret;
 	kp.pre_handler = handler_pre;
 	kp.post_handler = handler_post;
 	kp.fault_handler = handler_fault;
-	kp.addr = (kprobe_opcode_t*) kallsyms_lookup_name("do_fork");
-	/* register the kprobe now */
-	if (!kp.addr) {
-		printk("Couldn't find %s to plant kprobe\n", "do_fork");
-		return -1;
-	}
+	kp.symbol_name = "do_fork";
+
 	if ((ret = register_kprobe(&kp) < 0)) {
 		printk("register_kprobe failed, returned %d\n", ret);
 		return -1;
@@ -423,12 +444,14 @@ int init_module(void)
 	return 0;
 }
 
-void cleanup_module(void)
+static void __exit kprobe_exit(void)
 {
 	unregister_kprobe(&kp);
 	printk("kprobe unregistered\n");
 }
 
+module_init(kprobe_init)
+module_exit(kprobe_exit)
 MODULE_LICENSE("GPL");
 ----- cut here -----
 
@@ -463,7 +486,6 @@ the arguments of do_fork().
 #include <linux/fs.h>
 #include <linux/uio.h>
 #include <linux/kprobes.h>
-#include <linux/kallsyms.h>
 
 /*
  * Jumper probe for do_fork.
@@ -485,17 +507,13 @@ long jdo_fork(unsigned long clone_flags,
 }
 
 static struct jprobe my_jprobe = {
-	.entry = (kprobe_opcode_t *) jdo_fork
+	.entry = JPROBE_ENTRY(jdo_fork)
 };
 
-int init_module(void)
+static int __init jprobe_init(void)
 {
 	int ret;
-	my_jprobe.kp.addr = (kprobe_opcode_t *) kallsyms_lookup_name("do_fork");
-	if (!my_jprobe.kp.addr) {
-		printk("Couldn't find %s to plant jprobe\n", "do_fork");
-		return -1;
-	}
+	my_jprobe.kp.symbol_name = "do_fork";
 
 	if ((ret = register_jprobe(&my_jprobe)) <0) {
 		printk("register_jprobe failed, returned %d\n", ret);
@@ -506,12 +524,14 @@ int init_module(void)
 	return 0;
 }
 
-void cleanup_module(void)
+static void __exit jprobe_exit(void)
 {
 	unregister_jprobe(&my_jprobe);
 	printk("jprobe unregistered\n");
 }
 
+module_init(jprobe_init)
+module_exit(jprobe_exit)
 MODULE_LICENSE("GPL");
 ----- cut here -----
 
@@ -530,16 +550,13 @@ report failed calls to sys_open().
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/kprobes.h>
-#include <linux/kallsyms.h>
 
 static const char *probed_func = "sys_open";
 
 /* Return-probe handler: If the probed function fails, log the return value. */
 static int ret_handler(struct kretprobe_instance *ri, struct pt_regs *regs)
 {
-	// Substitute the appropriate register name for your architecture --
-	// e.g., regs->rax for x86_64, regs->gpr[3] for ppc64.
-	int retval = (int) regs->eax;
+	int retval = get_retval(regs);
 	if (retval < 0) {
 		printk("%s returns %d\n", probed_func, retval);
 	}
@@ -552,15 +569,11 @@ static struct kretprobe my_kretprobe = {
 	.maxactive = 20
 };
 
-int init_module(void)
+static int __init kretprobe_init(void)
 {
 	int ret;
-	my_kretprobe.kp.addr =
-		(kprobe_opcode_t *) kallsyms_lookup_name(probed_func);
-	if (!my_kretprobe.kp.addr) {
-		printk("Couldn't find %s to plant return probe\n", probed_func);
-		return -1;
-	}
+	my_kretprobe.kp.symbol_name = (char *)probed_func;
+
 	if ((ret = register_kretprobe(&my_kretprobe)) < 0) {
 		printk("register_kretprobe failed, returned %d\n", ret);
 		return -1;
@@ -569,7 +582,7 @@ int init_module(void)
 	return 0;
 }
 
-void cleanup_module(void)
+static void __exit kretprobe_exit(void)
 {
 	unregister_kretprobe(&my_kretprobe);
 	printk("kretprobe unregistered\n");
@@ -578,6 +591,8 @@ void cleanup_module(void)
 		my_kretprobe.nmissed, probed_func);
 }
 
+module_init(kretprobe_init)
+module_exit(kretprobe_exit)
 MODULE_LICENSE("GPL");
 ----- cut here -----
 
@@ -590,3 +605,5 @@ messages.)
 For additional information on Kprobes, refer to the following URLs:
 http://www-106.ibm.com/developerworks/library/l-kprobes.html?ca=dgr-lnxw42Kprobe
 http://www.redhat.com/magazine/005mar05/features/kprobes/
+http://www-users.cs.umn.edu/~boutcher/kprobes/
+http://www.linuxsymposium.org/2006/linuxsymposium_procv2.pdf (pages 101-115)
#5 lore raw

Re: [PATCH 3/3] Kprobes: Update Documentation/kprobes.txt

Quoted 10 lines · Ananth N Mavinakayanahalli
Quoted 8 lines · Christoph Hellwig
Quoted 4 lines · Ananth N Mavinakayanahalli

From: Ananth N Mavinakayanahalli <[email protected]>

Update Documentation/kprobes.txt to reflect addition of KPROBE_ADDR, KPROBE_RETVAL and the in-kernel symbol resolution.

Thanks. With my updated patch we shouldn't document KPROBE_ADDR anymore but tell people to always use the symbol_name mechanisms.

Hopefully the final version :)

Update Documentation/kprobes.txt:
- Add usage details of "symbol_name" and "offset" fields of struct kprobe
- Document return_value and JPROBE_ENTRY
- Update references list
- Update module examples to use module_init/module_exit interfaces

---
 Documentation/kprobes.txt |   77 ++++++++++++++++++++++++++++------------------
 1 files changed, 47 insertions(+), 30 deletions(-)
Documentation/kprobes.txt +47 -30
Index: linux-2.6.18-rc3/Documentation/kprobes.txt
===================================================================
@@ -179,6 +179,21 @@ occurs during execution of kp->pre_handl
 or during single-stepping of the probed instruction, Kprobes calls
 kp->fault_handler.  Any or all handlers can be NULL.
 
+NOTE:
+1. With the introduction of the "symbol_name" field to struct kprobe,
+the probepoint address resolution will now be taken care of by the kernel.
+The following will now work:
+
+	kp.symbol_name = "symbol_name";
+
+2. Use the "offset" field of struct kprobe if the offset into the symbol
+to install a probepoint is known. This field is used to calculate the
+probepoint address only if the "symbol_name" method of address resolution
+is used.
+
+3. Specify either the kprobe "symbol_name" with "offset" OR the "addr".
+If both are specified, kprobe registration will fail with -EINVAL.
+
 register_kprobe() returns 0 on success, or a negative errno otherwise.
 
 User's pre-handler (kp->pre_handler):
@@ -225,6 +240,12 @@ control to Kprobes.)  If the probed func
 fastcall, or anything else that affects how args are passed, the
 handler's declaration must match.
 
+NOTE: A macro JPROBE_ENTRY is provided to handle architecture-specific
+aliasing of jp->entry. In the interest of portability, it is advised
+to use:
+
+	jp->entry = JPROBE_ENTRY(handler);
+
 register_jprobe() returns 0 on success, or a negative errno otherwise.
 
 4.3 register_kretprobe
@@ -251,6 +272,11 @@ of interest:
 - ret_addr: the return address
 - rp: points to the corresponding kretprobe object
 - task: points to the corresponding task struct
+
+The return_value(regs) macro provides a simple abstraction to extract
+the return value from the appropriate register as defined by the
+architecture's ABI.
+
 The handler's return value is currently ignored.
 
 4.4 unregister_*probe
@@ -369,7 +395,6 @@ stack trace and selected i386 registers 
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/kprobes.h>
-#include <linux/kallsyms.h>
 #include <linux/sched.h>
 
 /*For each probe you need to allocate a kprobe structure*/
@@ -403,18 +428,14 @@ int handler_fault(struct kprobe *p, stru
 	return 0;
 }
 
-int init_module(void)
+static int __init kprobe_init(void)
 {
 	int ret;
 	kp.pre_handler = handler_pre;
 	kp.post_handler = handler_post;
 	kp.fault_handler = handler_fault;
-	kp.addr = (kprobe_opcode_t*) kallsyms_lookup_name("do_fork");
-	/* register the kprobe now */
-	if (!kp.addr) {
-		printk("Couldn't find %s to plant kprobe\n", "do_fork");
-		return -1;
-	}
+	kp.symbol_name = "do_fork";
+
 	if ((ret = register_kprobe(&kp) < 0)) {
 		printk("register_kprobe failed, returned %d\n", ret);
 		return -1;
@@ -423,12 +444,14 @@ int init_module(void)
 	return 0;
 }
 
-void cleanup_module(void)
+static void __exit kprobe_exit(void)
 {
 	unregister_kprobe(&kp);
 	printk("kprobe unregistered\n");
 }
 
+module_init(kprobe_init)
+module_exit(kprobe_exit)
 MODULE_LICENSE("GPL");
 ----- cut here -----
 
@@ -463,7 +486,6 @@ the arguments of do_fork().
 #include <linux/fs.h>
 #include <linux/uio.h>
 #include <linux/kprobes.h>
-#include <linux/kallsyms.h>
 
 /*
  * Jumper probe for do_fork.
@@ -485,17 +507,13 @@ long jdo_fork(unsigned long clone_flags,
 }
 
 static struct jprobe my_jprobe = {
-	.entry = (kprobe_opcode_t *) jdo_fork
+	.entry = JPROBE_ENTRY(jdo_fork)
 };
 
-int init_module(void)
+static int __init jprobe_init(void)
 {
 	int ret;
-	my_jprobe.kp.addr = (kprobe_opcode_t *) kallsyms_lookup_name("do_fork");
-	if (!my_jprobe.kp.addr) {
-		printk("Couldn't find %s to plant jprobe\n", "do_fork");
-		return -1;
-	}
+	my_jprobe.kp.symbol_name = "do_fork";
 
 	if ((ret = register_jprobe(&my_jprobe)) <0) {
 		printk("register_jprobe failed, returned %d\n", ret);
@@ -506,12 +524,14 @@ int init_module(void)
 	return 0;
 }
 
-void cleanup_module(void)
+static void __exit jprobe_exit(void)
 {
 	unregister_jprobe(&my_jprobe);
 	printk("jprobe unregistered\n");
 }
 
+module_init(jprobe_init)
+module_exit(jprobe_exit)
 MODULE_LICENSE("GPL");
 ----- cut here -----
 
@@ -530,16 +550,13 @@ report failed calls to sys_open().
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/kprobes.h>
-#include <linux/kallsyms.h>
 
 static const char *probed_func = "sys_open";
 
 /* Return-probe handler: If the probed function fails, log the return value. */
 static int ret_handler(struct kretprobe_instance *ri, struct pt_regs *regs)
 {
-	// Substitute the appropriate register name for your architecture --
-	// e.g., regs->rax for x86_64, regs->gpr[3] for ppc64.
-	int retval = (int) regs->eax;
+	int retval = return_value(regs);
 	if (retval < 0) {
 		printk("%s returns %d\n", probed_func, retval);
 	}
@@ -552,15 +569,11 @@ static struct kretprobe my_kretprobe = {
 	.maxactive = 20
 };
 
-int init_module(void)
+static int __init kretprobe_init(void)
 {
 	int ret;
-	my_kretprobe.kp.addr =
-		(kprobe_opcode_t *) kallsyms_lookup_name(probed_func);
-	if (!my_kretprobe.kp.addr) {
-		printk("Couldn't find %s to plant return probe\n", probed_func);
-		return -1;
-	}
+	my_kretprobe.kp.symbol_name = (char *)probed_func;
+
 	if ((ret = register_kretprobe(&my_kretprobe)) < 0) {
 		printk("register_kretprobe failed, returned %d\n", ret);
 		return -1;
@@ -569,7 +582,7 @@ int init_module(void)
 	return 0;
 }
 
-void cleanup_module(void)
+static void __exit kretprobe_exit(void)
 {
 	unregister_kretprobe(&my_kretprobe);
 	printk("kretprobe unregistered\n");
@@ -578,6 +591,8 @@ void cleanup_module(void)
 		my_kretprobe.nmissed, probed_func);
 }
 
+module_init(kretprobe_init)
+module_exit(kretprobe_exit)
 MODULE_LICENSE("GPL");
 ----- cut here -----
 
@@ -590,3 +605,5 @@ messages.)
 For additional information on Kprobes, refer to the following URLs:
 http://www-106.ibm.com/developerworks/library/l-kprobes.html?ca=dgr-lnxw42Kprobe
 http://www.redhat.com/magazine/005mar05/features/kprobes/
+http://www-users.cs.umn.edu/~boutcher/kprobes/
+http://www.linuxsymposium.org/2006/linuxsymposium_procv2.pdf (pages 101-115)
#6 lore raw

Re: [PATCH 2/3] Kprobes: Define retval helper

In reply to Ananth N Mavinakayanahalli
Quoted 4 lines · Ananth N Mavinakayanahalli

From: Ananth N Mavinakayanahalli <[email protected]>

Add the KPROBE_RETVAL macro to help extract the return value on different architectures, while using function-return probes.

Good idea. You should add parentheses around regs, otherwise the C preprocessor might bite users. Also the shouting name is quite ugly. In fact it should probably go to asm/system.h or similar and not have a kprobes name - it just extracts the return value from a struct pt_regs after all.

#7 lore raw

Re: [PATCH 2/3] Kprobes: Define retval helper

Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Ananth N Mavinakayanahalli

From: Ananth N Mavinakayanahalli <[email protected]>

Add the KPROBE_RETVAL macro to help extract the return value on different architectures, while using function-return probes.

Good idea. You should add parentheses around regs, otherwise the C preprocessor might bite users. Also the shouting name is quite ugly. In fact it should probably go to asm/system.h or similar

The other macros like this (instruction_pointer etc) are in asm/ptrace.h

-Andi

#8 lore raw

Re: [PATCH 2/3] Kprobes: Define retval helper

In reply to Christoph Hellwig
Quoted 11 lines · Christoph Hellwig
Quoted 4 lines · Ananth N Mavinakayanahalli

From: Ananth N Mavinakayanahalli <[email protected]>

Add the KPROBE_RETVAL macro to help extract the return value on different architectures, while using function-return probes.

Good idea. You should add parentheses around regs, otherwise the C preprocessor might bite users. Also the shouting name is quite ugly. In fact it should probably go to asm/system.h or similar and not have a kprobes name - it just extracts the return value from a struct pt_regs after all.

Done! How does this look? I added it to asm/ptrace.h so it lives along with the instruction_pointer() definition.

Ananth

---

Add the "get_retval" macro that just extracts the return value given the pt_regs. Useful in situations such as while using function-return probes.

---
 include/asm-i386/ptrace.h    |    3 +++
 include/asm-ia64/ptrace.h    |    3 +++
 include/asm-powerpc/ptrace.h |    2 ++
 include/asm-x86_64/ptrace.h  |    2 ++
 4 files changed, 10 insertions(+)
include/asm-i386/ptrace.h +3 -0
Index: linux-2.6.18-rc3/include/asm-i386/ptrace.h
===================================================================
@@ -79,7 +79,10 @@ static inline int user_mode_vm(struct pt
 {
 	return ((regs->xcs & 3) | (regs->eflags & VM_MASK)) != 0;
 }
+
 #define instruction_pointer(regs) ((regs)->eip)
+#define get_retval(regs) ((regs)->eax)
+
 #if defined(CONFIG_SMP) && defined(CONFIG_FRAME_POINTER)
 extern unsigned long profile_pc(struct pt_regs *regs);
 #else
include/asm-ia64/ptrace.h +3 -0
Index: linux-2.6.18-rc3/include/asm-ia64/ptrace.h
===================================================================
@@ -237,6 +237,9 @@ struct switch_stack {
  * the canonical representation by adding to instruction pointer.
  */
 # define instruction_pointer(regs) ((regs)->cr_iip + ia64_psr(regs)->ri)
+
+#define get_retval(regs) ((regs)->r8)
+
 /* Conserve space in histogram by encoding slot bits in address
  * bits 2 and 3 rather than bits 0 and 1.
  */
include/asm-powerpc/ptrace.h +2 -0
Index: linux-2.6.18-rc3/include/asm-powerpc/ptrace.h
===================================================================
@@ -73,6 +73,8 @@ struct pt_regs {
 #ifndef __ASSEMBLY__
 
 #define instruction_pointer(regs) ((regs)->nip)
+#define get_retval(regs) ((regs)->gpr[3])
+
 #ifdef CONFIG_SMP
 extern unsigned long profile_pc(struct pt_regs *regs);
 #else
include/asm-x86_64/ptrace.h +2 -0
Index: linux-2.6.18-rc3/include/asm-x86_64/ptrace.h
===================================================================
@@ -84,6 +84,8 @@ struct pt_regs {
 #define user_mode(regs) (!!((regs)->cs & 3))
 #define user_mode_vm(regs) user_mode(regs)
 #define instruction_pointer(regs) ((regs)->rip)
+#define get_retval(regs) ((regs)->rax)
+
 extern unsigned long profile_pc(struct pt_regs *regs);
 void signal_fault(struct pt_regs *regs, void __user *frame, char *where);
 
#9 lore raw

Re: [PATCH 2/3] Kprobes: Define retval helper

Quoted 8 lines · Ananth N Mavinakayanahalli
Quoted 5 lines

Good idea. You should add parentheses around regs, otherwise the C preprocessor might bite users. Also the shouting name is quite ugly. In fact it should probably go to asm/system.h or similar and not have a kprobes name - it just extracts the return value from a struct pt_regs after all.

Done! How does this look? I added it to asm/ptrace.h so it lives along with the instruction_pointer() definition.

Looks good, but it would be much better if we had it for every single architecture. I've cc'ed linux-arch so the arch maintainers can comments.

Even if we don't manage to get every architecture maintainer to help out you should at least add sparc and s390 to have full coverage of architectures with kprobes support

Quoted 60 lines

Add the "get_retval" macro that just extracts the return value given the pt_regs. Useful in situations such as while using function-return probes.

---
 include/asm-i386/ptrace.h    |    3 +++
 include/asm-ia64/ptrace.h    |    3 +++
 include/asm-powerpc/ptrace.h |    2 ++
 include/asm-x86_64/ptrace.h  |    2 ++
 4 files changed, 10 insertions(+)

Index: linux-2.6.18-rc3/include/asm-i386/ptrace.h
===================================================================

include/asm-i386/ptrace.h +3 -0
@@ -79,7 +79,10 @@ static inline int user_mode_vm(struct pt
 {
 	return ((regs->xcs & 3) | (regs->eflags & VM_MASK)) != 0;
 }
+
 #define instruction_pointer(regs) ((regs)->eip)
+#define get_retval(regs) ((regs)->eax)
+
 #if defined(CONFIG_SMP) && defined(CONFIG_FRAME_POINTER)
 extern unsigned long profile_pc(struct pt_regs *regs);
 #else

Index: linux-2.6.18-rc3/include/asm-ia64/ptrace.h
===================================================================

include/asm-ia64/ptrace.h +3 -0
@@ -237,6 +237,9 @@ struct switch_stack {
  * the canonical representation by adding to instruction pointer.
  */
 # define instruction_pointer(regs) ((regs)->cr_iip + ia64_psr(regs)->ri)
+
+#define get_retval(regs) ((regs)->r8)
+
 /* Conserve space in histogram by encoding slot bits in address
  * bits 2 and 3 rather than bits 0 and 1.
  */

Index: linux-2.6.18-rc3/include/asm-powerpc/ptrace.h
===================================================================

include/asm-powerpc/ptrace.h +2 -0
@@ -73,6 +73,8 @@ struct pt_regs {
 #ifndef __ASSEMBLY__
 
 #define instruction_pointer(regs) ((regs)->nip)
+#define get_retval(regs) ((regs)->gpr[3])
+
 #ifdef CONFIG_SMP
 extern unsigned long profile_pc(struct pt_regs *regs);
 #else

Index: linux-2.6.18-rc3/include/asm-x86_64/ptrace.h
===================================================================

include/asm-x86_64/ptrace.h struct pt_regs { · line 91

Quoted 8 lines
include/asm-x86_64/ptrace.h +2 -0
@@ -84,6 +84,8 @@ struct pt_regs {
 #define user_mode(regs) (!!((regs)->cs & 3))
 #define user_mode_vm(regs) user_mode(regs)
 #define instruction_pointer(regs) ((regs)->rip)
+#define get_retval(regs) ((regs)->rax)
+
 extern unsigned long profile_pc(struct pt_regs *regs);
 void signal_fault(struct pt_regs *regs, void __user *frame, char *where);
 

---end quoted text---

#10 lore raw

Re: [PATCH 2/3] Kprobes: Define retval helper

Quoted 3 lines
Quoted 3 lines

#define instruction_pointer(regs) ((regs)->eip)
+#define get_retval(regs) ((regs)->eax)

return_value() would match the names of the existing macro better

-Andi

#11 lore raw

Re: [PATCH 2/3] Kprobes: Define retval helper

Quoted 5 lines · Andi Kleen
Quoted 3 lines
Quoted 3 lines

#define instruction_pointer(regs) ((regs)->eip)
+#define get_retval(regs) ((regs)->eax)

return_value() would match the names of the existing macro better

Updated patch ...

Add the "return_value" macro that just extracts the return value given the pt_regs. Useful in situations such as while using function-return probes.

---
 include/asm-i386/ptrace.h    |    3 +++
 include/asm-ia64/ptrace.h    |    3 +++
 include/asm-powerpc/ptrace.h |    2 ++
 include/asm-x86_64/ptrace.h  |    2 ++
 4 files changed, 10 insertions(+)
include/asm-i386/ptrace.h +3 -0
Index: linux-2.6.18-rc3/include/asm-i386/ptrace.h
===================================================================
@@ -79,7 +79,10 @@ static inline int user_mode_vm(struct pt
 {
 	return ((regs->xcs & 3) | (regs->eflags & VM_MASK)) != 0;
 }
+
 #define instruction_pointer(regs) ((regs)->eip)
+#define return_value(regs) ((regs)->eax)
+
 #if defined(CONFIG_SMP) && defined(CONFIG_FRAME_POINTER)
 extern unsigned long profile_pc(struct pt_regs *regs);
 #else
include/asm-ia64/ptrace.h +3 -0
Index: linux-2.6.18-rc3/include/asm-ia64/ptrace.h
===================================================================
@@ -237,6 +237,9 @@ struct switch_stack {
  * the canonical representation by adding to instruction pointer.
  */
 # define instruction_pointer(regs) ((regs)->cr_iip + ia64_psr(regs)->ri)
+
+#define return_value(regs) ((regs)->r8)
+
 /* Conserve space in histogram by encoding slot bits in address
  * bits 2 and 3 rather than bits 0 and 1.
  */
include/asm-powerpc/ptrace.h +2 -0
Index: linux-2.6.18-rc3/include/asm-powerpc/ptrace.h
===================================================================
@@ -73,6 +73,8 @@ struct pt_regs {
 #ifndef __ASSEMBLY__
 
 #define instruction_pointer(regs) ((regs)->nip)
+#define return_value(regs) ((regs)->gpr[3])
+
 #ifdef CONFIG_SMP
 extern unsigned long profile_pc(struct pt_regs *regs);
 #else
include/asm-x86_64/ptrace.h +2 -0
Index: linux-2.6.18-rc3/include/asm-x86_64/ptrace.h
===================================================================
@@ -84,6 +84,8 @@ struct pt_regs {
 #define user_mode(regs) (!!((regs)->cs & 3))
 #define user_mode_vm(regs) user_mode(regs)
 #define instruction_pointer(regs) ((regs)->rip)
+#define return_value(regs) ((regs)->rax)
+
 extern unsigned long profile_pc(struct pt_regs *regs);
 void signal_fault(struct pt_regs *regs, void __user *frame, char *where);
 
#12 lore raw

Re: [PATCH 2/3] Kprobes: Define retval helper

Quoted 5 lines · Ananth N Mavinakayanahalli

Updated patch ...

Add the "return_value" macro that just extracts the return value given the pt_regs. Useful in situations such as while using function-return probes.

return_value definition for s390 see below.

Signature · 22 lines
blue skies,
  Martin.

Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH

"Reality continues to ruin my life." - Calvin.

---

diff -urpN linux-2.6.18-rc3/include/asm-s390/ptrace.h linux-2.6.18-s390/include/asm-s390/ptrace.h
--- linux-2.6.18-rc3/include/asm-s390/ptrace.h	2006-06-29 12:47:32.000000000 +0200
+++ linux-2.6.18-s390/include/asm-s390/ptrace.h	2006-08-09 12:25:23.000000000 +0200
@@ -472,6 +472,7 @@ struct user_regs_struct
 
 #define user_mode(regs) (((regs)->psw.mask & PSW_MASK_PSTATE) != 0)
 #define instruction_pointer(regs) ((regs)->psw.addr & PSW_ADDR_INSN)
+#define return_value(regs)((regs)->gprs[2])
 #define profile_pc(regs) instruction_pointer(regs)
 extern void show_regs(struct pt_regs * regs);
 #endif
#13 lore raw

Re: [PATCH 2/3] Kprobes: Define retval helper

In reply to Christoph Hellwig
Quoted 8 lines · Christoph Hellwig
Quoted 8 lines
Quoted 5 lines

Good idea. You should add parentheses around regs, otherwise the C preprocessor might bite users. Also the shouting name is quite ugly. In fact it should probably go to asm/system.h or similar and not have a kprobes name - it just extracts the return value from a struct pt_regs after all.

Done! How does this look? I added it to asm/ptrace.h so it lives along with the instruction_pointer() definition.

I presume we don't care about return values that span multiple registers - for instance if you return a 64-bit value on i386 it'll wind up in EDX:EAX.

David

#14 lore raw

Re: [PATCH 2/3] Kprobes: Define retval helper

Quoted 8 lines · David Howells
Quoted 6 lines · Christoph Hellwig
Quoted 6 lines
Quoted 5 lines

Good idea. You should add parentheses around regs, otherwise the C preprocessor might bite users. Also the shouting name is quite ugly. In fact it should probably go to asm/system.h or similar and not have a kprobes name - it just extracts the return value from a struct pt_regs after all.

Quoted 5 lines
Quoted 2 lines
Quoted 2 lines

Done! How does this look? I added it to asm/ptrace.h so it lives along with the instruction_pointer() definition.

I presume we don't care about return values that span multiple registers - for instance if you return a 64-bit value on i386 it'll wind up in EDX:EAX.

Yes. This helper is mostly to address the common case, not the 64-bit one.

Ananth

author Acked-by Dave Boutcher on v1 1/3
#15 lore raw
In reply to Ananth N Mavinakayanahalli
Quoted 9 lines · Ananth N Mavinakayanahalli

From: Ananth N Mavinakayanahalli <[email protected]>

This patch introduces KPROBE_ADDR, a macro that abstracts out the architecture-specific artefacts of getting the correct text address given a symbol. While we are at it, also introduce the symbol_name field in struct kprobe to allow for users to just specify the address to be probed in terms of the kernel symbol. In-kernel kprobes infrastructure decodes the actual text address to probe. The symbol resolution happens only if the kprobe.addr isn't explicitly specified.

This change was first mooted by Dave Boutcher during his kprobes tutorial @ OLS. Thanks Dave! (He had also signed off on the patch)

Quoted 4 lines

---

Quoted 69 lines

---
 include/asm-powerpc/kprobes.h |    2 ++
 include/linux/kprobes.h       |    8 ++++++++
 kernel/kprobes.c              |    4 ++++
 3 files changed, 14 insertions(+)

Index: linux-2.6.18-rc3/include/asm-powerpc/kprobes.h
===================================================================

include/asm-powerpc/kprobes.h +2 -0
@@ -44,6 +44,8 @@ typedef unsigned int kprobe_opcode_t;
 #define IS_TDI(instr)		(((instr) & 0xfc000000) == 0x08000000)
 #define IS_TWI(instr)		(((instr) & 0xfc000000) == 0x0c000000)
 
+#define KPROBE_ADDR(name) *((kprobe_opcode_t **)(kallsyms_lookup_name(name)))
+
 #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)((func_descr_t *)pentry)
 
 #define is_trap(instr)	(IS_TW(instr) || IS_TD(instr) || \

Index: linux-2.6.18-rc3/include/linux/kprobes.h
===================================================================

include/linux/kprobes.h +8 -0
@@ -36,6 +36,7 @@
 #include <linux/spinlock.h>
 #include <linux/rcupdate.h>
 #include <linux/mutex.h>
+#include <linux/kallsyms.h>
 
 #ifdef CONFIG_KPROBES
 #include <asm/kprobes.h>
@@ -49,6 +50,10 @@
 /* Attach to insert probes on any functions which should be ignored*/
 #define __kprobes	__attribute__((__section__(".kprobes.text")))
 
+#ifndef KPROBE_ADDR	/* powerpc has its own definition */
+#define KPROBE_ADDR(name) (kprobe_opcode_t *)(kallsyms_lookup_name(name))
+#endif	/* KPROBE_ADDR */
+
 struct kprobe;
 struct pt_regs;
 struct kretprobe;
@@ -77,6 +82,9 @@ struct kprobe {
 	/* location of the probe point */
 	kprobe_opcode_t *addr;
 
+	/* Allow user to indicate symbol name of the probe point */
+	char *symbol_name;
+
 	/* Called before addr is executed. */
 	kprobe_pre_handler_t pre_handler;
 

Index: linux-2.6.18-rc3/kernel/kprobes.c
===================================================================

kernel/kprobes.c +4 -0
@@ -446,6 +446,10 @@ static int __kprobes __register_kprobe(s
 	struct kprobe *old_p;
 	struct module *probed_mod;
 
+	/* Do the kallsyms lookup only if p->addr == NULL */
+	if (!p->addr && (p->symbol_name))
+		p->addr = KPROBE_ADDR(p->symbol_name);
+
 	if ((!kernel_text_address((unsigned long) p->addr)) ||
 		in_kprobes_functions((unsigned long) p->addr))
 		return -EINVAL;
#16 lore raw
In reply to Ananth N Mavinakayanahalli
Quoted 9 lines · Ananth N Mavinakayanahalli

From: Ananth N Mavinakayanahalli <[email protected]>

This patch introduces KPROBE_ADDR, a macro that abstracts out the architecture-specific artefacts of getting the correct text address given a symbol. While we are at it, also introduce the symbol_name field in struct kprobe to allow for users to just specify the address to be probed in terms of the kernel symbol. In-kernel kprobes infrastructure decodes the actual text address to probe. The symbol resolution happens only if the kprobe.addr isn't explicitly specified.

This looks good. A few issues are left:

- the KPROBE_ADDR macro is all uppercase and not exactly very descriptive.
- the symbol name variant should be the default, and no one outside kprobes.c should know about the KPROBE_ADDR macro
- we should return EINVAL instead of silently discarding things if people specify a symbol and an address.
- we should have and offset into the symbol specified

The updated patch below does that, aswell as updating the only inkernel kprobes user (tcp_probe.c) to the new interface (*) and removing the now obsolete kallsysms_lookup_name export.

(*) tcp_probe.c shows very well how horrible the old interface was, as it's
not portable to ppc64 as-is

include/asm-powerpc/kprobes.h +3 -0
Index: linux-2.6/include/asm-powerpc/kprobes.h
===================================================================
@@ -44,6 +44,9 @@
 #define IS_TDI(instr)		(((instr) & 0xfc000000) == 0x08000000)
 #define IS_TWI(instr)		(((instr) & 0xfc000000) == 0x0c000000)
 
+#define kprobe_lookup_name(name) \
+	(*((kprobe_opcode_t **)kallsyms_lookup_name(name)))
+
 #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)((func_descr_t *)pentry)
 
 #define is_trap(instr)	(IS_TW(instr) || IS_TD(instr) || \
include/linux/kprobes.h +6 -0
Index: linux-2.6/include/linux/kprobes.h
===================================================================
@@ -77,6 +77,12 @@
 	/* location of the probe point */
 	kprobe_opcode_t *addr;
 
+	/* Allow user to indicate symbol name of the probe point */
+	char *symbol_name;
+
+	/* Offset into the symbol */
+	unsigned int offset;
+
 	/* Called before addr is executed. */
 	kprobe_pre_handler_t pre_handler;
 
kernel/kprobes.c +22 -0
Index: linux-2.6/kernel/kprobes.c
===================================================================
@@ -37,6 +37,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/moduleloader.h>
+#include <linux/kallsyms.h>
 #include <asm-generic/sections.h>
 #include <asm/cacheflush.h>
 #include <asm/errno.h>
@@ -45,6 +46,16 @@
 #define KPROBE_HASH_BITS 6
 #define KPROBE_TABLE_SIZE (1 << KPROBE_HASH_BITS)
 
+
+/*
+ * Some oddball architectures like 64bit powerpc have function descriptors
+ * so this must be overridable.
+ */
+#ifndef kprobe_lookup_name
+#define kprobe_lookup_name(name) \
+	((kprobe_opcode_t *)(kallsyms_lookup_name(name))
+#endif
+
 static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
 static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
 static atomic_t kprobe_count;
@@ -447,6 +458,17 @@
 	struct kprobe *old_p;
 	struct module *probed_mod;
 
+	/*
+	 * If we have a symbol_name argument look it up,
+	 * and add it to the address.  That way the addr
+	 * field can either be global or relative to a symbol.
+	 */
+	if (p->symbol_name) {
+		if (p->addr)
+			return -EINVAL;
+		p->addr = kprobe_lookup_name(p->symbol_name) + p->offset;
+	}
+
 	if ((!kernel_text_address((unsigned long) p->addr)) ||
 		in_kprobes_functions((unsigned long) p->addr))
 		return -EINVAL;
kernel/kallsyms.c +0 -1
Index: linux-2.6/kernel/kallsyms.c
===================================================================
@@ -154,7 +154,6 @@
 	}
 	return module_kallsyms_lookup_name(name);
 }
-EXPORT_SYMBOL_GPL(kallsyms_lookup_name);
 
 /*
  * Lookup an address
net/ipv4/tcp_probe.c +4 -2
Index: linux-2.6/net/ipv4/tcp_probe.c
===================================================================
@@ -99,8 +99,10 @@
 }
 
 static struct jprobe tcp_send_probe = {
-	.kp = { .addr = (kprobe_opcode_t *) &tcp_sendmsg, },
-	.entry = (kprobe_opcode_t *) &jtcp_sendmsg,
+	.kp = {
+		.symbol_name	= "tcp_sendmsg",
+	},
+	.entry	= JPROBE_ENTRY(jtcp_sendmsg),
 };
 
 
#17 lore raw
Quoted 20 lines · On Tue, 8 Aug 2006 17:24:21 +0100 Christoph Hellwig <[email protected]> wrote:
Quoted 9 lines · Ananth N Mavinakayanahalli

From: Ananth N Mavinakayanahalli <[email protected]>

This patch introduces KPROBE_ADDR, a macro that abstracts out the architecture-specific artefacts of getting the correct text address given a symbol. While we are at it, also introduce the symbol_name field in struct kprobe to allow for users to just specify the address to be probed in terms of the kernel symbol. In-kernel kprobes infrastructure decodes the actual text address to probe. The symbol resolution happens only if the kprobe.addr isn't explicitly specified.

This looks good. A few issues are left:

- the KPROBE_ADDR macro is all uppercase and not exactly very descriptive.
- the symbol name variant should be the default, and no one outside kprobes.c should know about the KPROBE_ADDR macro
- we should return EINVAL instead of silently discarding things if people specify a symbol and an address.
- we should have and offset into the symbol specified

Quoted 6 lines

The updated patch below does that, aswell as updating the only inkernel kprobes user (tcp_probe.c) to the new interface (*) and removing the now obsolete kallsysms_lookup_name export.

(*) tcp_probe.c shows very well how horrible the old interface was, as it's
not portable to ppc64 as-is

Okay, does this makes kprobe's the first reflective kernel interface. Watch out or it end up like JNI!

#18 lore raw
Quoted 1 line · Stephen Hemminger

Okay, does this makes kprobe's the first reflective kernel interface.

Actually kallsyms_lookup_name was the first interface like that. And lots of external kprobes used it like that - in fact tcp_probe.c is the first one I've seen doing it differently. But kallsyms_lookup_name is a really awkward lowlevel buildingblock that's almost impossible to use correctly, so this patch hides it behind the proper kprobes interface.

#19 lore raw
Quoted 8 lines · Christoph Hellwig
Quoted 1 line · Stephen Hemminger

Okay, does this makes kprobe's the first reflective kernel interface.

Actually kallsyms_lookup_name was the first interface like that. And lots of external kprobes used it like that - in fact tcp_probe.c is the first one I've seen doing it differently. But kallsyms_lookup_name is a really awkward lowlevel buildingblock that's almost impossible to use correctly, so this patch hides it behind the proper kprobes interface.

Just one side note: kallsyms_lookup_name is _really_ inefficient. The kallsyms structure is tailored so that kallsyms_lookup (the most frequently used function) is really fast. Doing it the other way around involves a O(N) search, uncompressing every symbol name as it goes :P

I don't think this is really a performance problem for users like kprobes, but I just wanted people to keep in mind that there is a penalty involved in calling kallsyms_lookup_name.

Signature · 4 lines
Paulo Marques - www.grupopie.com

"The face of a child can say it all, especially the
mouth part of the face."
#20 lore raw
Quoted 8 lines · Paulo Marques

Just one side note: kallsyms_lookup_name is _really_ inefficient. The kallsyms structure is tailored so that kallsyms_lookup (the most frequently used function) is really fast. Doing it the other way around involves a O(N) search, uncompressing every symbol name as it goes :P

I don't think this is really a performance problem for users like kprobes, but I just wanted people to keep in mind that there is a penalty involved in calling kallsyms_lookup_name.

That's true. One more reason to not expose this interface to the public.

#21 lore raw
In reply to Christoph Hellwig
Quoted 11 lines · Christoph Hellwig
Quoted 9 lines · Ananth N Mavinakayanahalli

From: Ananth N Mavinakayanahalli <[email protected]>

This patch introduces KPROBE_ADDR, a macro that abstracts out the architecture-specific artefacts of getting the correct text address given a symbol. While we are at it, also introduce the symbol_name field in struct kprobe to allow for users to just specify the address to be probed in terms of the kernel symbol. In-kernel kprobes infrastructure decodes the actual text address to probe. The symbol resolution happens only if the kprobe.addr isn't explicitly specified.

Quoted 8 lines

This looks good. A few issues are left:

- the KPROBE_ADDR macro is all uppercase and not exactly very descriptive.
- the symbol name variant should be the default, and no one outside kprobes.c should know about the KPROBE_ADDR macro
- we should return EINVAL instead of silently discarding things if people specify a symbol and an address.
- we should have and offset into the symbol specified

Agreed.

Quoted 8 lines

The updated patch below does that, aswell as updating the only inkernel kprobes user (tcp_probe.c) to the new interface (*) and removing the now obsolete kallsysms_lookup_name export.

(*) tcp_probe.c shows very well how horrible the old interface was, as it's
not portable to ppc64 as-is

Quoted 107 lines

Index: linux-2.6/include/asm-powerpc/kprobes.h
===================================================================

include/asm-powerpc/kprobes.h +3 -0
@@ -44,6 +44,9 @@
 #define IS_TDI(instr)		(((instr) & 0xfc000000) == 0x08000000)
 #define IS_TWI(instr)		(((instr) & 0xfc000000) == 0x0c000000)
 
+#define kprobe_lookup_name(name) \
+	(*((kprobe_opcode_t **)kallsyms_lookup_name(name)))
+
 #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)((func_descr_t *)pentry)
 
 #define is_trap(instr)	(IS_TW(instr) || IS_TD(instr) || \

Index: linux-2.6/include/linux/kprobes.h
===================================================================

include/linux/kprobes.h +6 -0
@@ -77,6 +77,12 @@
 	/* location of the probe point */
 	kprobe_opcode_t *addr;
 
+	/* Allow user to indicate symbol name of the probe point */
+	char *symbol_name;
+
+	/* Offset into the symbol */
+	unsigned int offset;
+
 	/* Called before addr is executed. */
 	kprobe_pre_handler_t pre_handler;
 

Index: linux-2.6/kernel/kprobes.c
===================================================================

kernel/kprobes.c +22 -0
@@ -37,6 +37,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/moduleloader.h>
+#include <linux/kallsyms.h>
 #include <asm-generic/sections.h>
 #include <asm/cacheflush.h>
 #include <asm/errno.h>
@@ -45,6 +46,16 @@
 #define KPROBE_HASH_BITS 6
 #define KPROBE_TABLE_SIZE (1 << KPROBE_HASH_BITS)
 
+
+/*
+ * Some oddball architectures like 64bit powerpc have function descriptors
+ * so this must be overridable.
+ */
+#ifndef kprobe_lookup_name
+#define kprobe_lookup_name(name) \
+	((kprobe_opcode_t *)(kallsyms_lookup_name(name))
+#endif
+
 static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
 static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
 static atomic_t kprobe_count;
@@ -447,6 +458,17 @@
 	struct kprobe *old_p;
 	struct module *probed_mod;
 
+	/*
+	 * If we have a symbol_name argument look it up,
+	 * and add it to the address.  That way the addr
+	 * field can either be global or relative to a symbol.
+	 */
+	if (p->symbol_name) {
+		if (p->addr)
+			return -EINVAL;
+		p->addr = kprobe_lookup_name(p->symbol_name) + p->offset;
+	}
+
 	if ((!kernel_text_address((unsigned long) p->addr)) ||
 		in_kprobes_functions((unsigned long) p->addr))
 		return -EINVAL;

Index: linux-2.6/kernel/kallsyms.c
===================================================================

kernel/kallsyms.c +0 -1
@@ -154,7 +154,6 @@
 	}
 	return module_kallsyms_lookup_name(name);
 }
-EXPORT_SYMBOL_GPL(kallsyms_lookup_name);
 
 /*
  * Lookup an address

Index: linux-2.6/net/ipv4/tcp_probe.c
===================================================================

net/ipv4/tcp_probe.c +4 -2
@@ -99,8 +99,10 @@
 }
 
 static struct jprobe tcp_send_probe = {
-	.kp = { .addr = (kprobe_opcode_t *) &tcp_sendmsg, },
-	.entry = (kprobe_opcode_t *) &jtcp_sendmsg,
+	.kp = {
+		.symbol_name	= "tcp_sendmsg",
+	},
+	.entry	= JPROBE_ENTRY(jtcp_sendmsg),
 };
 
 
#22 lore raw
In reply to Christoph Hellwig
Quoted 1 line · Christoph Hellwig

On Mon, Aug 07, 2006 at 05:25:37PM +0530, Ananth N Mavinakayanahalli wrote:

... stuff deleted ...

kernel/kprobes.c line 470

Quoted 11 lines
kernel/kprobes.c +10 -0
+	/*
+	 * If we have a symbol_name argument look it up,
+	 * and add it to the address.  That way the addr
+	 * field can either be global or relative to a symbol.
+	 */
+	if (p->symbol_name) {
+		if (p->addr)
+			return -EINVAL;
+		p->addr = kprobe_lookup_name(p->symbol_name) + p->offset;
+	}

What if kprobe_lookup_name() fails or if CONFIG_KALLSYMS isn't set? The code following this might work, but the kprobe isn't going to be set at the location that was intended.

Perhaps this needs something like:

	if (p->symbol_name) {
		if (p->addr)
			return -EINVAL;
		p->addr = kprobe_lookup_name(p->symbol_name) + p->offset;
		if (p->addr == p->offset)
			return -EINVAL;
	}
Signature · 6 lines
David Smith
[email protected]
Red Hat, Inc.
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)
#23 lore raw
Quoted 7 lines · David Smith

kernel/kprobes.c line 470

Quoted 5 lines
kernel/kprobes.c +5 -0
+	if (p->symbol_name) {
+		if (p->addr)
+			return -EINVAL;
+		p->addr = kprobe_lookup_name(p->symbol_name) + p->offset;
+	}

What if kprobe_lookup_name() fails

for that case we need the check in your snipplet below.

Quoted 1 line

or if CONFIG_KALLSYMS isn't set?

I think we should just disallow that case. having kprobes without kallsyms is rather pointless. I'll send a patch to add the dependency to the Kconfig files.

Quoted 9 lines

Perhaps this needs something like:

	if (p->symbol_name) {
		if (p->addr)
			return -EINVAL;
		p->addr = kprobe_lookup_name(p->symbol_name) + p->offset;
		if (p->addr == p->offset)
			return -EINVAL;
	}
#24 lore raw
Quoted 9 lines · Christoph Hellwig
Quoted 7 lines · David Smith

Review

Quoted 5 lines
(quoted diff) +5 -0
+	if (p->symbol_name) {
+		if (p->addr)
+			return -EINVAL;
+		p->addr = kprobe_lookup_name(p->symbol_name) + p->offset;
+	}

What if kprobe_lookup_name() fails

Quoted 7 lines

for that case we need the check in your snipplet below.

Quoted 1 line

or if CONFIG_KALLSYMS isn't set?

I think we should just disallow that case. having kprobes without kallsyms is rather pointless. I'll send a patch to add the dependency to the Kconfig files.

Here's an updated version of that patch that a) adds a KALLYMS dependency and b) adds a check for p->addr. Note that this check is outside the if (p->symbol_name) block so it also returns EINVAL if the user didn't specifcy either and address or a symbol name.

include/asm-powerpc/kprobes.h +3 -0
Index: linux-2.6/include/asm-powerpc/kprobes.h
===================================================================
@@ -44,6 +44,9 @@
 #define IS_TDI(instr)		(((instr) & 0xfc000000) == 0x08000000)
 #define IS_TWI(instr)		(((instr) & 0xfc000000) == 0x0c000000)
 
+#define kprobe_lookup_name(name) \
+	(*((kprobe_opcode_t **)kallsyms_lookup_name(name)))
+
 #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)((func_descr_t *)pentry)
 
 #define is_trap(instr)	(IS_TW(instr) || IS_TD(instr) || \
include/linux/kprobes.h +6 -0
Index: linux-2.6/include/linux/kprobes.h
===================================================================
@@ -77,6 +77,12 @@
 	/* location of the probe point */
 	kprobe_opcode_t *addr;
 
+	/* Allow user to indicate symbol name of the probe point */
+	char *symbol_name;
+
+	/* Offset into the symbol */
+	unsigned int offset;
+
 	/* Called before addr is executed. */
 	kprobe_pre_handler_t pre_handler;
 
kernel/kprobes.c +26 -0
Index: linux-2.6/kernel/kprobes.c
===================================================================
@@ -37,6 +37,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/moduleloader.h>
+#include <linux/kallsyms.h>
 #include <asm-generic/sections.h>
 #include <asm/cacheflush.h>
 #include <asm/errno.h>
@@ -45,6 +46,16 @@
 #define KPROBE_HASH_BITS 6
 #define KPROBE_TABLE_SIZE (1 << KPROBE_HASH_BITS)
 
+
+/*
+ * Some oddball architectures like 64bit powerpc have function descriptors
+ * so this must be overridable.
+ */
+#ifndef kprobe_lookup_name
+#define kprobe_lookup_name(name) \
+	((kprobe_opcode_t *)(kallsyms_lookup_name(name))
+#endif
+
 static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
 static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
 static atomic_t kprobe_count;
@@ -447,6 +458,21 @@
 	struct kprobe *old_p;
 	struct module *probed_mod;
 
+	/*
+	 * If we have a symbol_name argument look it up,
+	 * and add it to the address.  That way the addr
+	 * field can either be global or relative to a symbol.
+	 */
+	if (p->symbol_name) {
+		if (p->addr)
+			return -EINVAL;
+		p->addr = kprobe_lookup_name(p->symbol_name);
+	}
+
+	if (!p->addr)
+		return -EINVAL;
+	p->addr += p->offset;
+
 	if ((!kernel_text_address((unsigned long) p->addr)) ||
 		in_kprobes_functions((unsigned long) p->addr))
 		return -EINVAL;
kernel/kallsyms.c +0 -1
Index: linux-2.6/kernel/kallsyms.c
===================================================================
@@ -154,7 +154,6 @@
 	}
 	return module_kallsyms_lookup_name(name);
 }
-EXPORT_SYMBOL_GPL(kallsyms_lookup_name);
 
 /*
  * Lookup an address
net/ipv4/tcp_probe.c +4 -2
Index: linux-2.6/net/ipv4/tcp_probe.c
===================================================================
@@ -99,8 +99,10 @@
 }
 
 static struct jprobe tcp_send_probe = {
-	.kp = { .addr = (kprobe_opcode_t *) &tcp_sendmsg, },
-	.entry = (kprobe_opcode_t *) &jtcp_sendmsg,
+	.kp = {
+		.symbol_name	= "tcp_sendmsg",
+	},
+	.entry	= JPROBE_ENTRY(jtcp_sendmsg),
 };
 
 
arch/i386/Kconfig +1 -1
Index: linux-2.6/arch/i386/Kconfig
===================================================================
@@ -1129,7 +1129,7 @@
 
 config KPROBES
 	bool "Kprobes (EXPERIMENTAL)"
-	depends on EXPERIMENTAL && MODULES
+	depends on KALLSYMS && EXPERIMENTAL && MODULES
 	help
 	  Kprobes allows you to trap at almost any kernel address and
 	  execute a callback function.  register_kprobe() establishes
arch/ia64/Kconfig +1 -1
Index: linux-2.6/arch/ia64/Kconfig
===================================================================
@@ -510,7 +510,7 @@
 
 config KPROBES
 	bool "Kprobes (EXPERIMENTAL)"
-	depends on EXPERIMENTAL && MODULES
+	depends on KALLSYMS && EXPERIMENTAL && MODULES
 	help
 	  Kprobes allows you to trap at almost any kernel address and
 	  execute a callback function.  register_kprobe() establishes
arch/powerpc/Kconfig +1 -1
Index: linux-2.6/arch/powerpc/Kconfig
===================================================================
@@ -1045,7 +1045,7 @@
 
 config KPROBES
 	bool "Kprobes (EXPERIMENTAL)"
-	depends on PPC64 && EXPERIMENTAL && MODULES
+	depends on PPC64 && KALLSYMS && EXPERIMENTAL && MODULES
 	help
 	  Kprobes allows you to trap at almost any kernel address and
 	  execute a callback function.  register_kprobe() establishes
arch/sparc64/Kconfig +1 -1
Index: linux-2.6/arch/sparc64/Kconfig
===================================================================
@@ -416,7 +416,7 @@
 
 config KPROBES
 	bool "Kprobes (EXPERIMENTAL)"
-	depends on EXPERIMENTAL && MODULES
+	depends on KALLSYMS && EXPERIMENTAL && MODULES
 	help
 	  Kprobes allows you to trap at almost any kernel address and
 	  execute a callback function.  register_kprobe() establishes
arch/x86_64/Kconfig +1 -1
Index: linux-2.6/arch/x86_64/Kconfig
===================================================================
@@ -639,7 +639,7 @@
 
 config KPROBES
 	bool "Kprobes (EXPERIMENTAL)"
-	depends on EXPERIMENTAL && MODULES
+	depends on KALLSYMS && EXPERIMENTAL && MODULES
 	help
 	  Kprobes allows you to trap at almost any kernel address and
 	  execute a callback function.  register_kprobe() establishes
#25 lore raw

kernel/kprobes.c line 474

Quoted 10 lines · Christoph Hellwig
kernel/kprobes.c +10 -0
+	 */
+	if (p->symbol_name) {
+		if (p->addr)
+			return -EINVAL;
+		p->addr = kprobe_lookup_name(p->symbol_name);
+	}
+
+	if (!p->addr)
+		return -EINVAL;
+	p->addr += p->offset;

This should be p->addr = (kprobe_opcode_t *)(((char *)p->addr) + p->offset), since p->addr is of type pointer to kprobe_opcode_t and the size of kprobe_opcode_t is different for different architecture. At least for ia64 this p->addr type is not a pointer to char.

-Anil Keshavamurthy

#26 lore raw
Quoted 3 lines · Keshavamurthy Anil S

This should be p->addr = (kprobe_opcode_t *)(((char *)p->addr) + p->offset), since p->addr is of type pointer to kprobe_opcode_t and the size of kprobe_opcode_t is different for different architecture. At least for ia64 this p->addr type is not a pointer to char.

Similarly for powerpc. We should either put this change in or drop the offset into symbol support until actual users pop up..

#27 lore raw
In reply to Christoph Hellwig
Quoted 22 lines · Christoph Hellwig
Quoted 9 lines · Christoph Hellwig
Quoted 7 lines · David Smith

Review

Quoted 5 lines
(quoted diff) +5 -0
+	if (p->symbol_name) {
+		if (p->addr)
+			return -EINVAL;
+		p->addr = kprobe_lookup_name(p->symbol_name) + p->offset;
+	}

What if kprobe_lookup_name() fails

Quoted 7 lines

for that case we need the check in your snipplet below.

Quoted 1 line

or if CONFIG_KALLSYMS isn't set?

I think we should just disallow that case. having kprobes without kallsyms is rather pointless. I'll send a patch to add the dependency to the Kconfig files.

Here's an updated version of that patch that a) adds a KALLYMS dependency and b) adds a check for p->addr. Note that this check is outside the if (p->symbol_name) block so it also returns EINVAL if the user didn't specifcy either and address or a symbol name.

...

Quoted 15 lines

Index: linux-2.6/kernel/kprobes.c
===================================================================

kernel/kprobes.c +1 -0
@@ -37,6 +37,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/moduleloader.h>
+#include <linux/kallsyms.h>
 #include <asm-generic/sections.h>
 #include <asm/cacheflush.h>
 #include <asm/errno.h>
@@ -45,6 +46,16 @@
 #define KPROBE_HASH_BITS 6

kernel/kprobes.c line 56

Quoted 10 lines
kernel/kprobes.c +8 -0
 #define KPROBE_TABLE_SIZE (1 << KPROBE_HASH_BITS)
 
+
+/*
+ * Some oddball architectures like 64bit powerpc have function descriptors
+ * so this must be overridable.
+ */
+#ifndef kprobe_lookup_name
+#define kprobe_lookup_name(name) \
+	((kprobe_opcode_t *)(kallsyms_lookup_name(name))

This is missing a ')'. Updated patch below...

---
 arch/i386/Kconfig             |    2 +-
 arch/ia64/Kconfig             |    2 +-
 arch/powerpc/Kconfig          |    2 +-
 arch/sparc64/Kconfig          |    2 +-
 arch/x86_64/Kconfig           |    2 +-
 include/asm-powerpc/kprobes.h |    3 +++
 include/linux/kprobes.h       |    6 ++++++
 kernel/kallsyms.c             |    1 -
 kernel/kprobes.c              |   26 ++++++++++++++++++++++++++
 net/ipv4/tcp_probe.c          |    6 ++++--
 10 files changed, 44 insertions(+), 8 deletions(-)
arch/i386/Kconfig +1 -1
Index: linux-2.6.18-rc4/arch/i386/Kconfig
===================================================================
@@ -1129,7 +1129,7 @@ source "arch/i386/oprofile/Kconfig"
 
 config KPROBES
 	bool "Kprobes (EXPERIMENTAL)"
-	depends on EXPERIMENTAL && MODULES
+	depends on KALLSYMS && EXPERIMENTAL && MODULES
 	help
 	  Kprobes allows you to trap at almost any kernel address and
 	  execute a callback function.  register_kprobe() establishes
arch/ia64/Kconfig +1 -1
Index: linux-2.6.18-rc4/arch/ia64/Kconfig
===================================================================
@@ -510,7 +510,7 @@ source "arch/ia64/oprofile/Kconfig"
 
 config KPROBES
 	bool "Kprobes (EXPERIMENTAL)"
-	depends on EXPERIMENTAL && MODULES
+	depends on KALLSYMS && EXPERIMENTAL && MODULES
 	help
 	  Kprobes allows you to trap at almost any kernel address and
 	  execute a callback function.  register_kprobe() establishes
arch/powerpc/Kconfig +1 -1
Index: linux-2.6.18-rc4/arch/powerpc/Kconfig
===================================================================
@@ -1045,7 +1045,7 @@ source "arch/powerpc/oprofile/Kconfig"
 
 config KPROBES
 	bool "Kprobes (EXPERIMENTAL)"
-	depends on PPC64 && EXPERIMENTAL && MODULES
+	depends on PPC64 && KALLSYMS && EXPERIMENTAL && MODULES
 	help
 	  Kprobes allows you to trap at almost any kernel address and
 	  execute a callback function.  register_kprobe() establishes
arch/sparc64/Kconfig +1 -1
Index: linux-2.6.18-rc4/arch/sparc64/Kconfig
===================================================================
@@ -416,7 +416,7 @@ source "arch/sparc64/oprofile/Kconfig"
 
 config KPROBES
 	bool "Kprobes (EXPERIMENTAL)"
-	depends on EXPERIMENTAL && MODULES
+	depends on KALLSYMS && EXPERIMENTAL && MODULES
 	help
 	  Kprobes allows you to trap at almost any kernel address and
 	  execute a callback function.  register_kprobe() establishes
arch/x86_64/Kconfig +1 -1
Index: linux-2.6.18-rc4/arch/x86_64/Kconfig
===================================================================
@@ -639,7 +639,7 @@ source "arch/x86_64/oprofile/Kconfig"
 
 config KPROBES
 	bool "Kprobes (EXPERIMENTAL)"
-	depends on EXPERIMENTAL && MODULES
+	depends on KALLSYMS && EXPERIMENTAL && MODULES
 	help
 	  Kprobes allows you to trap at almost any kernel address and
 	  execute a callback function.  register_kprobe() establishes
include/asm-powerpc/kprobes.h +3 -0
Index: linux-2.6.18-rc4/include/asm-powerpc/kprobes.h
===================================================================
@@ -44,6 +44,9 @@ typedef unsigned int kprobe_opcode_t;
 #define IS_TDI(instr)		(((instr) & 0xfc000000) == 0x08000000)
 #define IS_TWI(instr)		(((instr) & 0xfc000000) == 0x0c000000)
 
+#define kprobe_lookup_name(name) \
+	(*((kprobe_opcode_t **)kallsyms_lookup_name(name)))
+
 #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)((func_descr_t *)pentry)
 
 #define is_trap(instr)	(IS_TW(instr) || IS_TD(instr) || \
include/linux/kprobes.h +6 -0
Index: linux-2.6.18-rc4/include/linux/kprobes.h
===================================================================
@@ -77,6 +77,12 @@ struct kprobe {
 	/* location of the probe point */
 	kprobe_opcode_t *addr;
 
+	/* Allow user to indicate symbol name of the probe point */
+	char *symbol_name;
+
+	/* Offset into the symbol */
+	unsigned int offset;
+
 	/* Called before addr is executed. */
 	kprobe_pre_handler_t pre_handler;
 
kernel/kallsyms.c +0 -1
Index: linux-2.6.18-rc4/kernel/kallsyms.c
===================================================================
@@ -154,7 +154,6 @@ unsigned long kallsyms_lookup_name(const
 	}
 	return module_kallsyms_lookup_name(name);
 }
-EXPORT_SYMBOL_GPL(kallsyms_lookup_name);
 
 /*
  * Lookup an address
kernel/kprobes.c +26 -0
Index: linux-2.6.18-rc4/kernel/kprobes.c
===================================================================
@@ -37,6 +37,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/moduleloader.h>
+#include <linux/kallsyms.h>
 #include <asm-generic/sections.h>
 #include <asm/cacheflush.h>
 #include <asm/errno.h>
@@ -45,6 +46,16 @@
 #define KPROBE_HASH_BITS 6
 #define KPROBE_TABLE_SIZE (1 << KPROBE_HASH_BITS)
 
+
+/*
+ * Some oddball architectures like 64bit powerpc have function descriptors
+ * so this must be overridable.
+ */
+#ifndef kprobe_lookup_name
+#define kprobe_lookup_name(name) \
+	((kprobe_opcode_t *)(kallsyms_lookup_name(name)))
+#endif
+
 static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
 static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
 static atomic_t kprobe_count;
@@ -447,6 +458,21 @@ static int __kprobes __register_kprobe(s
 	struct kprobe *old_p;
 	struct module *probed_mod;
 
+	/*
+	 * If we have a symbol_name argument look it up,
+	 * and add it to the address.  That way the addr
+	 * field can either be global or relative to a symbol.
+	 */
+	if (p->symbol_name) {
+		if (p->addr)
+			return -EINVAL;
+		p->addr = kprobe_lookup_name(p->symbol_name);
+	}
+
+	if (!p->addr)
+		return -EINVAL;
+	p->addr += p->offset;
+
 	if ((!kernel_text_address((unsigned long) p->addr)) ||
 		in_kprobes_functions((unsigned long) p->addr))
 		return -EINVAL;
net/ipv4/tcp_probe.c +4 -2
Index: linux-2.6.18-rc4/net/ipv4/tcp_probe.c
===================================================================
@@ -99,8 +99,10 @@ static int jtcp_sendmsg(struct kiocb *io
 }
 
 static struct jprobe tcp_send_probe = {
-	.kp = { .addr = (kprobe_opcode_t *) &tcp_sendmsg, },
-	.entry = (kprobe_opcode_t *) &jtcp_sendmsg,
+	.kp = {
+		.symbol_name	= "tcp_sendmsg",
+	},
+	.entry	= JPROBE_ENTRY(jtcp_sendmsg),
 };
 
 
#28 lore raw
In reply to Christoph Hellwig
Quoted 10 lines · Christoph Hellwig
Quoted 7 lines · David Smith

kernel/kprobes.c line 470

Quoted 5 lines
kernel/kprobes.c +5 -0
+	if (p->symbol_name) {
+		if (p->addr)
+			return -EINVAL;
+		p->addr = kprobe_lookup_name(p->symbol_name) + p->offset;
+	}

What if kprobe_lookup_name() fails

for that case we need the check in your snipplet below.

The next check:

        if ((!kernel_text_address((unsigned long) p->addr)) ||
                in_kprobes_functions((unsigned long) p->addr))
                return -EINVAL;

will catch this case anyway (unless some arch has kernel text starting at vaddr 0)

Ananth

Quoted 16 lines
Quoted 2 lines

or if CONFIG_KALLSYMS isn't set?

I think we should just disallow that case. having kprobes without kallsyms is rather pointless. I'll send a patch to add the dependency to the Kconfig files.

Quoted 9 lines

Perhaps this needs something like:

	if (p->symbol_name) {
		if (p->addr)
			return -EINVAL;
		p->addr = kprobe_lookup_name(p->symbol_name) + p->offset;
		if (p->addr == p->offset)
			return -EINVAL;
	}