[PATCH 1/3] Kprobes: Make kprobe modules more portable

[PATCH 1/3] Kprobes: Make kprobe modules more portable

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

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.

---
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Prasanna S.P <prasanna@in.ibm.com>


---
 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
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-powerpc/kprobes.h
+++ 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) || \
Index: linux-2.6.18-rc3/include/linux/kprobes.h
===================================================================
--- linux-2.6.18-rc3.orig/include/linux/kprobes.h
+++ 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;
 
Index: linux-2.6.18-rc3/kernel/kprobes.c
===================================================================
--- linux-2.6.18-rc3.orig/kernel/kprobes.c
+++ 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;

[PATCH 2/3] Kprobes: Define retval helper

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

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

---
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
Signed-off-by: Prasanna S.P <prasanna@in.ibm.com>


---
 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(+)

Index: linux-2.6.18-rc3/include/asm-i386/kprobes.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-i386/kprobes.h
+++ 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);
 
Index: linux-2.6.18-rc3/include/asm-ia64/kprobes.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-ia64/kprobes.h
+++ 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)
Index: linux-2.6.18-rc3/include/asm-powerpc/kprobes.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-powerpc/kprobes.h
+++ 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);
 
Index: linux-2.6.18-rc3/include/asm-x86_64/kprobes.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-x86_64/kprobes.h
+++ 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);
 

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

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

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

---
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Acked-by: Jim Keniston <jkenisto@us.ibm.com>


---
 Documentation/kprobes.txt |   77 ++++++++++++++++++++++++++++------------------
 1 files changed, 47 insertions(+), 30 deletions(-)

Index: linux-2.6.18-rc3/Documentation/kprobes.txt
===================================================================
--- linux-2.6.18-rc3.orig/Documentation/kprobes.txt
+++ 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)

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Ananth N Mavinakayanahalli]

On Mon, Aug 07, 2006 at 05:25:37PM +0530, Ananth N Mavinakayanahalli wrote:
> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> 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)

> 
> ---
> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Signed-off-by: Prasanna S.P <prasanna@in.ibm.com>

Acked-by: Dave Boutcher <sleddog@us.ibm.com>

> 
> 
> ---
>  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
> ===================================================================
> --- linux-2.6.18-rc3.orig/include/asm-powerpc/kprobes.h
> +++ 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) || \
> Index: linux-2.6.18-rc3/include/linux/kprobes.h
> ===================================================================
> --- linux-2.6.18-rc3.orig/include/linux/kprobes.h
> +++ 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;
>  
> Index: linux-2.6.18-rc3/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.18-rc3.orig/kernel/kprobes.c
> +++ 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;

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Christoph Hellwig]

On Mon, Aug 07, 2006 at 05:25:37PM +0530, Ananth N Mavinakayanahalli wrote:
> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> 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

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/include/asm-powerpc/kprobes.h
===================================================================
--- linux-2.6.orig/include/asm-powerpc/kprobes.h	2006-08-08 17:47:22.000000000 +0200
+++ linux-2.6/include/asm-powerpc/kprobes.h	2006-08-08 18:13:57.000000000 +0200
@@ -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
===================================================================
--- linux-2.6.orig/include/linux/kprobes.h	2006-08-08 17:47:22.000000000 +0200
+++ linux-2.6/include/linux/kprobes.h	2006-08-08 17:47:31.000000000 +0200
@@ -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
===================================================================
--- linux-2.6.orig/kernel/kprobes.c	2006-08-08 17:47:22.000000000 +0200
+++ linux-2.6/kernel/kprobes.c	2006-08-08 17:47:31.000000000 +0200
@@ -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
===================================================================
--- linux-2.6.orig/kernel/kallsyms.c	2006-08-08 17:13:14.000000000 +0200
+++ linux-2.6/kernel/kallsyms.c	2006-08-08 17:47:39.000000000 +0200
@@ -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
===================================================================
--- linux-2.6.orig/net/ipv4/tcp_probe.c	2006-08-08 18:13:55.000000000 +0200
+++ linux-2.6/net/ipv4/tcp_probe.c	2006-08-08 18:14:28.000000000 +0200
@@ -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),
 };
 
 

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

[Christoph Hellwig]

On Mon, Aug 07, 2006 at 05:30:24PM +0530, Ananth N Mavinakayanahalli wrote:
> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> 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.

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

[Christoph Hellwig]

On Mon, Aug 07, 2006 at 05:34:47PM +0530, Ananth N Mavinakayanahalli wrote:
> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> 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?

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Stephen Hemminger]

On Tue, 8 Aug 2006 17:24:21 +0100
Christoph Hellwig <hch@infradead.org> wrote:

> On Mon, Aug 07, 2006 at 05:25:37PM +0530, Ananth N Mavinakayanahalli wrote:
> > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > 
> > 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

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

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Christoph Hellwig]

On Tue, Aug 08, 2006 at 09:34:00AM -0700, Stephen Hemminger wrote:
> 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.

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Paulo Marques]

Christoph Hellwig wrote:
> On Tue, Aug 08, 2006 at 09:34:00AM -0700, Stephen Hemminger wrote:
>> 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.

-- 
Paulo Marques - www.grupopie.com

"The face of a child can say it all, especially the
mouth part of the face."

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Christoph Hellwig]

On Tue, Aug 08, 2006 at 06:28:28PM +0100, Paulo Marques wrote:
> 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.

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

[Andi Kleen]

Christoph Hellwig <hch@infradead.org> writes:

> On Mon, Aug 07, 2006 at 05:30:24PM +0530, Ananth N Mavinakayanahalli wrote:
> > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > 
> > 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

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

[Ananth N Mavinakayanahalli]

On Tue, Aug 08, 2006 at 05:25:59PM +0100, Christoph Hellwig wrote:
> On Mon, Aug 07, 2006 at 05:30:24PM +0530, Ananth N Mavinakayanahalli wrote:
> > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > 
> > 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.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

---
 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
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-i386/ptrace.h
+++ 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
Index: linux-2.6.18-rc3/include/asm-ia64/ptrace.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-ia64/ptrace.h
+++ 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.
  */
Index: linux-2.6.18-rc3/include/asm-powerpc/ptrace.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-powerpc/ptrace.h
+++ 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
Index: linux-2.6.18-rc3/include/asm-x86_64/ptrace.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-x86_64/ptrace.h
+++ 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);
 

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

[Christoph Hellwig]

On Wed, Aug 09, 2006 at 03:13:11PM +0530, Ananth N Mavinakayanahalli wrote:
> > 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

> 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.
> 
> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> ---
>  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
> ===================================================================
> --- linux-2.6.18-rc3.orig/include/asm-i386/ptrace.h
> +++ 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
> Index: linux-2.6.18-rc3/include/asm-ia64/ptrace.h
> ===================================================================
> --- linux-2.6.18-rc3.orig/include/asm-ia64/ptrace.h
> +++ 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.
>   */
> Index: linux-2.6.18-rc3/include/asm-powerpc/ptrace.h
> ===================================================================
> --- linux-2.6.18-rc3.orig/include/asm-powerpc/ptrace.h
> +++ 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
> Index: linux-2.6.18-rc3/include/asm-x86_64/ptrace.h
> ===================================================================
> --- linux-2.6.18-rc3.orig/include/asm-x86_64/ptrace.h
> +++ 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);
>  
---end quoted text---

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

[Ananth N Mavinakayanahalli]

On Tue, Aug 08, 2006 at 05:27:01PM +0100, Christoph Hellwig wrote:
> On Mon, Aug 07, 2006 at 05:34:47PM +0530, Ananth N Mavinakayanahalli wrote:
> > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > 
> > 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.

> 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


Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

---
 Documentation/kprobes.txt |   77 ++++++++++++++++++++++++++++------------------
 1 files changed, 47 insertions(+), 30 deletions(-)

Index: linux-2.6.18-rc3/Documentation/kprobes.txt
===================================================================
--- linux-2.6.18-rc3.orig/Documentation/kprobes.txt
+++ 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)

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Ananth N Mavinakayanahalli]

On Tue, Aug 08, 2006 at 05:24:21PM +0100, Christoph Hellwig wrote:
> On Mon, Aug 07, 2006 at 05:25:37PM +0530, Ananth N Mavinakayanahalli wrote:
> > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > 
> > 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

Agreed.

> 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
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

> Index: linux-2.6/include/asm-powerpc/kprobes.h
> ===================================================================
> --- linux-2.6.orig/include/asm-powerpc/kprobes.h	2006-08-08 17:47:22.000000000 +0200
> +++ linux-2.6/include/asm-powerpc/kprobes.h	2006-08-08 18:13:57.000000000 +0200
> @@ -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
> ===================================================================
> --- linux-2.6.orig/include/linux/kprobes.h	2006-08-08 17:47:22.000000000 +0200
> +++ linux-2.6/include/linux/kprobes.h	2006-08-08 17:47:31.000000000 +0200
> @@ -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
> ===================================================================
> --- linux-2.6.orig/kernel/kprobes.c	2006-08-08 17:47:22.000000000 +0200
> +++ linux-2.6/kernel/kprobes.c	2006-08-08 17:47:31.000000000 +0200
> @@ -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
> ===================================================================
> --- linux-2.6.orig/kernel/kallsyms.c	2006-08-08 17:13:14.000000000 +0200
> +++ linux-2.6/kernel/kallsyms.c	2006-08-08 17:47:39.000000000 +0200
> @@ -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
> ===================================================================
> --- linux-2.6.orig/net/ipv4/tcp_probe.c	2006-08-08 18:13:55.000000000 +0200
> +++ linux-2.6/net/ipv4/tcp_probe.c	2006-08-08 18:14:28.000000000 +0200
> @@ -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),
>  };
>  
>  

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

[Andi Kleen]

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

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

-Andi

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

[Ananth N Mavinakayanahalli]

On Wed, Aug 09, 2006 at 11:55:50AM +0200, Andi Kleen wrote:
> 
> > >  #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.


Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

---
 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
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-i386/ptrace.h
+++ 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
Index: linux-2.6.18-rc3/include/asm-ia64/ptrace.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-ia64/ptrace.h
+++ 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.
  */
Index: linux-2.6.18-rc3/include/asm-powerpc/ptrace.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-powerpc/ptrace.h
+++ 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
Index: linux-2.6.18-rc3/include/asm-x86_64/ptrace.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-x86_64/ptrace.h
+++ 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);
 

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

[Ananth N Mavinakayanahalli]

On Wed, Aug 09, 2006 at 03:18:59PM +0530, Ananth N Mavinakayanahalli wrote:
> On Tue, Aug 08, 2006 at 05:27:01PM +0100, Christoph Hellwig wrote:
> > On Mon, Aug 07, 2006 at 05:34:47PM +0530, Ananth N Mavinakayanahalli wrote:
> > > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > > 
> > > 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
 
 
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
 
---
 Documentation/kprobes.txt |   77 ++++++++++++++++++++++++++++------------------
 1 files changed, 47 insertions(+), 30 deletions(-)

Index: linux-2.6.18-rc3/Documentation/kprobes.txt
===================================================================
--- linux-2.6.18-rc3.orig/Documentation/kprobes.txt
+++ 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)

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

[Martin Schwidefsky]

On Wed, 2006-08-09 at 15:49 +0530, Ananth N Mavinakayanahalli wrote:
> 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.

-- 
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

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

[David Howells]

Christoph Hellwig <hch@infradead.org> wrote:

> > > 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

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

[Ananth N Mavinakayanahalli]

On Wed, Aug 09, 2006 at 02:16:04PM +0100, David Howells wrote:
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > > > 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.

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

Ananth

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[David Smith]

On Tue, 2006-08-08 at 17:24 +0100, Christoph Hellwig wrote:
> On Mon, Aug 07, 2006 at 05:25:37PM +0530, Ananth N Mavinakayanahalli wrote:

... stuff deleted ...

>  
> +	/*
> +	 * 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;
	}

-- 
David Smith
dsmith@redhat.com
Red Hat, Inc.
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Christoph Hellwig]

On Wed, Aug 09, 2006 at 11:01:45AM -0500, David Smith wrote:
> > +	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.

> 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.

> 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;
> 	}

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Christoph Hellwig]

On Wed, Aug 09, 2006 at 05:10:39PM +0100, Christoph Hellwig wrote:
> On Wed, Aug 09, 2006 at 11:01:45AM -0500, David Smith wrote:
> > > +	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.
> 
> > 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.


Index: linux-2.6/include/asm-powerpc/kprobes.h
===================================================================
--- linux-2.6.orig/include/asm-powerpc/kprobes.h	2006-08-09 13:44:39.000000000 +0200
+++ linux-2.6/include/asm-powerpc/kprobes.h	2006-08-09 13:44:57.000000000 +0200
@@ -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
===================================================================
--- linux-2.6.orig/include/linux/kprobes.h	2006-08-09 13:44:39.000000000 +0200
+++ linux-2.6/include/linux/kprobes.h	2006-08-09 13:44:57.000000000 +0200
@@ -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
===================================================================
--- linux-2.6.orig/kernel/kprobes.c	2006-08-09 13:44:39.000000000 +0200
+++ linux-2.6/kernel/kprobes.c	2006-08-09 18:12:47.000000000 +0200
@@ -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;
Index: linux-2.6/kernel/kallsyms.c
===================================================================
--- linux-2.6.orig/kernel/kallsyms.c	2006-08-09 13:44:39.000000000 +0200
+++ linux-2.6/kernel/kallsyms.c	2006-08-09 13:44:57.000000000 +0200
@@ -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
===================================================================
--- linux-2.6.orig/net/ipv4/tcp_probe.c	2006-08-09 13:44:39.000000000 +0200
+++ linux-2.6/net/ipv4/tcp_probe.c	2006-08-09 13:44:57.000000000 +0200
@@ -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),
 };
 
 
Index: linux-2.6/arch/i386/Kconfig
===================================================================
--- linux-2.6.orig/arch/i386/Kconfig	2006-08-08 17:13:13.000000000 +0200
+++ linux-2.6/arch/i386/Kconfig	2006-08-09 18:13:59.000000000 +0200
@@ -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
Index: linux-2.6/arch/ia64/Kconfig
===================================================================
--- linux-2.6.orig/arch/ia64/Kconfig	2006-08-08 17:13:13.000000000 +0200
+++ linux-2.6/arch/ia64/Kconfig	2006-08-09 18:14:13.000000000 +0200
@@ -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
Index: linux-2.6/arch/powerpc/Kconfig
===================================================================
--- linux-2.6.orig/arch/powerpc/Kconfig	2006-08-08 17:13:13.000000000 +0200
+++ linux-2.6/arch/powerpc/Kconfig	2006-08-09 18:15:00.000000000 +0200
@@ -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
Index: linux-2.6/arch/sparc64/Kconfig
===================================================================
--- linux-2.6.orig/arch/sparc64/Kconfig	2006-08-08 17:13:13.000000000 +0200
+++ linux-2.6/arch/sparc64/Kconfig	2006-08-09 18:14:42.000000000 +0200
@@ -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
Index: linux-2.6/arch/x86_64/Kconfig
===================================================================
--- linux-2.6.orig/arch/x86_64/Kconfig	2006-08-08 17:13:13.000000000 +0200
+++ linux-2.6/arch/x86_64/Kconfig	2006-08-09 18:14:27.000000000 +0200
@@ -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

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Ananth N Mavinakayanahalli]

On Wed, Aug 09, 2006 at 05:10:39PM +0100, Christoph Hellwig wrote:
> On Wed, Aug 09, 2006 at 11:01:45AM -0500, David Smith wrote:
> > > +	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

> 
> > 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.
> 
> > 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;
> > 	}

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Keshavamurthy Anil S]

On Wed, Aug 09, 2006 at 05:18:54PM +0100, Christoph Hellwig wrote:
> +	 */
> +	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

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Ananth N Mavinakayanahalli]

On Wed, Aug 09, 2006 at 05:18:54PM +0100, Christoph Hellwig wrote:
> On Wed, Aug 09, 2006 at 05:10:39PM +0100, Christoph Hellwig wrote:
> > On Wed, Aug 09, 2006 at 11:01:45AM -0500, David Smith wrote:
> > > > +	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.
> > 
> > > 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.

...
>  
> Index: linux-2.6/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.orig/kernel/kprobes.c	2006-08-09 13:44:39.000000000 +0200
> +++ linux-2.6/kernel/kprobes.c	2006-08-09 18:12:47.000000000 +0200
> @@ -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))

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

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

---
 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(-)

Index: linux-2.6.18-rc4/arch/i386/Kconfig
===================================================================
--- linux-2.6.18-rc4.orig/arch/i386/Kconfig
+++ 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
Index: linux-2.6.18-rc4/arch/ia64/Kconfig
===================================================================
--- linux-2.6.18-rc4.orig/arch/ia64/Kconfig
+++ 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
Index: linux-2.6.18-rc4/arch/powerpc/Kconfig
===================================================================
--- linux-2.6.18-rc4.orig/arch/powerpc/Kconfig
+++ 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
Index: linux-2.6.18-rc4/arch/sparc64/Kconfig
===================================================================
--- linux-2.6.18-rc4.orig/arch/sparc64/Kconfig
+++ 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
Index: linux-2.6.18-rc4/arch/x86_64/Kconfig
===================================================================
--- linux-2.6.18-rc4.orig/arch/x86_64/Kconfig
+++ 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
Index: linux-2.6.18-rc4/include/asm-powerpc/kprobes.h
===================================================================
--- linux-2.6.18-rc4.orig/include/asm-powerpc/kprobes.h
+++ 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) || \
Index: linux-2.6.18-rc4/include/linux/kprobes.h
===================================================================
--- linux-2.6.18-rc4.orig/include/linux/kprobes.h
+++ 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;
 
Index: linux-2.6.18-rc4/kernel/kallsyms.c
===================================================================
--- linux-2.6.18-rc4.orig/kernel/kallsyms.c
+++ 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
Index: linux-2.6.18-rc4/kernel/kprobes.c
===================================================================
--- linux-2.6.18-rc4.orig/kernel/kprobes.c
+++ 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;
Index: linux-2.6.18-rc4/net/ipv4/tcp_probe.c
===================================================================
--- linux-2.6.18-rc4.orig/net/ipv4/tcp_probe.c
+++ 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),
 };
 
 

Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable

[Christoph Hellwig]

On Thu, Aug 10, 2006 at 07:10:29AM -0700, Keshavamurthy Anil S wrote:
> 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..