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

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

[Ananth N Mavinakayanahalli]

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

In an effort to make kprobe modules more portable, here is a patch that:

o Introduces the "symbol_name" field to struct kprobe.
  The symbol->address resolution now happens in the kernel in an
  architecture agnostic manner. 64-bit powerpc users no longer have
  to specify the ".symbols"
o Introduces the "offset" field to struct kprobe to allow a user to
  specify an offset into a symbol.
o The legacy mechanism of specifying the kprobe.addr is still supported.
  However, if both the kprobe.addr and kprobe.symbol_name are specified,
  probe registration fails with an -EINVAL.
o The symbol resolution code uses kallsyms_lookup_name(). So
  CONFIG_KPROBES now depends on CONFIG_KALLSYMS
o Apparantly kprobe modules were the only legitimate out-of-tree user of
  the kallsyms_lookup_name() EXPORT. Now that the symbol resolution
  happens in-kernel, remove the EXPORT as suggested by Christoph Hellwig
o Modify tcp_probe.c that uses the kprobe interface so as to make it
  work on multiple platforms (in its earlier form, the code wouldn't
  work, say, on powerpc)
---

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

---
 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 = (kprobe_opcode_t *)(((char *)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),
 };
 
 

[PATCH 2/3] Add retval_value helper (updated)

[Ananth N Mavinakayanahalli]

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

Add the return_value() macro to extract the return value in an
architecture agnostic manner, given the pt_regs.

Other architecture maintainers may want to add similar helpers.

---

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

---
 include/asm-i386/ptrace.h    |    3 +++
 include/asm-ia64/ptrace.h    |    3 +++
 include/asm-powerpc/ptrace.h |    2 ++
 include/asm-s390/ptrace.h    |    1 +
 include/asm-x86_64/ptrace.h  |    2 ++
 5 files changed, 11 insertions(+)

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

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

[Ananth N Mavinakayanahalli]

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

Documentation/kprobes.txt updated to reflect:

o In-kernel symbol resolution
o CONFIG_KALLSYMS dependency
o Usage of JPROBE_ENTRY
o Addition of return_value()

Also update the references list and usage examples to use correct module
interfaces.
---

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

---
 Documentation/kprobes.txt |   89 ++++++++++++++++++++++++++++------------------
 1 files changed, 56 insertions(+), 33 deletions(-)

Index: linux-2.6.18-rc4/Documentation/kprobes.txt
===================================================================
--- linux-2.6.18-rc4.orig/Documentation/kprobes.txt
+++ linux-2.6.18-rc4/Documentation/kprobes.txt
@@ -151,9 +151,9 @@ So that you can load and unload Kprobes-
 make sure "Loadable module support" (CONFIG_MODULES) and "Module
 unloading" (CONFIG_MODULE_UNLOAD) are set to "y".
 
-You may also want to ensure that CONFIG_KALLSYMS and perhaps even
-CONFIG_KALLSYMS_ALL are set to "y", since kallsyms_lookup_name()
-is a handy, version-independent way to find a function's address.
+Also make sure that CONFIG_KALLSYMS and perhaps even CONFIG_KALLSYMS_ALL
+are set to "y", since kallsyms_lookup_name() is used by the in-kernel
+kprobe address resolution code.
 
 If you need to insert a probe in the middle of a function, you may find
 it useful to "Compile the kernel with debug info" (CONFIG_DEBUG_INFO),
@@ -179,6 +179,27 @@ 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";
+
+(64-bit powerpc intricacies such as function descriptors are handled
+transparently)
+
+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.
+
+3. Specify either the kprobe "symbol_name" OR the "addr". If both are
+specified, kprobe registration will fail with -EINVAL.
+
+4. With CISC architectures (such as i386 and x86_64), the kprobes code
+does not validate if the kprobe.addr is at an instruction boundary.
+Use "offset" with caution.
+
 register_kprobe() returns 0 on success, or a negative errno otherwise.
 
 User's pre-handler (kp->pre_handler):
@@ -225,6 +246,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 +278,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 +401,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 +434,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 +450,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 +492,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 +513,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 +530,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 +556,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 +575,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 +588,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 +597,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 +611,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] Add retval_value helper (updated)

[Andrew Morton]

On Tue, 22 Aug 2006 10:58:41 +0530
Ananth N Mavinakayanahalli <ananth@in.ibm.com> wrote:

> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> Add the return_value() macro to extract the return value in an
> architecture agnostic manner, given the pt_regs.
> 
> Other architecture maintainers may want to add similar helpers.

return_value() is quite a generic-sounding thing.

box:/usr/src/linux-2.6.18-rc4> grep -r return_value . | wc -l
66


How about regs_return_value()?

Re: [PATCH 2/3] Add retval_value helper (updated)

[Ananth N Mavinakayanahalli]

On Tue, Aug 22, 2006 at 02:13:07PM -0700, Andrew Morton wrote:
> On Tue, 22 Aug 2006 10:58:41 +0530
> Ananth N Mavinakayanahalli <ananth@in.ibm.com> wrote:
> 
> > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > 
> > Add the return_value() macro to extract the return value in an
> > architecture agnostic manner, given the pt_regs.
> > 
> > Other architecture maintainers may want to add similar helpers.
> 
> return_value() is quite a generic-sounding thing.
> 
> box:/usr/src/linux-2.6.18-rc4> grep -r return_value . | wc -l
> 66
> 
> 
> How about regs_return_value()?

Yes, that sounds fine too.

Ananth

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

[Ananth N Mavinakayanahalli]

On Tue, Aug 22, 2006 at 10:54:48AM +0530, Ananth N Mavinakayanahalli wrote:
> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> In an effort to make kprobe modules more portable, here is a patch that:
> 
> o Introduces the "symbol_name" field to struct kprobe.
>   The symbol->address resolution now happens in the kernel in an
>   architecture agnostic manner. 64-bit powerpc users no longer have
>   to specify the ".symbols"
> o Introduces the "offset" field to struct kprobe to allow a user to
>   specify an offset into a symbol.
> o The legacy mechanism of specifying the kprobe.addr is still supported.
>   However, if both the kprobe.addr and kprobe.symbol_name are specified,
>   probe registration fails with an -EINVAL.
> o The symbol resolution code uses kallsyms_lookup_name(). So
>   CONFIG_KPROBES now depends on CONFIG_KALLSYMS
> o Apparantly kprobe modules were the only legitimate out-of-tree user of
>   the kallsyms_lookup_name() EXPORT. Now that the symbol resolution
>   happens in-kernel, remove the EXPORT as suggested by Christoph Hellwig
> o Modify tcp_probe.c that uses the kprobe interface so as to make it
>   work on multiple platforms (in its earlier form, the code wouldn't
>   work, say, on powerpc)

Andrew,

Anil discovered that this patch has a case where, on powerpc, we potentially
end up dereferencing a NULL pointer. I have fixed that part and updated
the patch.

Please drop kprobes-make-kprobe-modules-more-portable.patch that is in
-mm currently. Please use this patch instead.

Thanks
Ananth
---

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

In an effort to make kprobe modules more portable, here is a patch that:

o Introduces the "symbol_name" field to struct kprobe.
  The symbol->address resolution now happens in the kernel in an
  architecture agnostic manner.
o Introduces the "offset" field to struct kprobe to allow a user to
  specify an offset into a symbol.
o The legacy mechanism of specifying the kprobe.addr is still supported.
  However, if both the kprobe.addr and kprobe.symbol_name are specified,
  probe registration fails with an -EINVAL.
o The symbol resolution code uses kallsyms_lookup_name(). So
  CONFIG_KPROBES now depends on CONFIG_KALLSYMS
o Apparantly kprobe modules were the only legitimate out-of-tree user of
  the kallsyms_lookup_name() EXPORT. Now that the symbol resolution
  happens in-kernel, remove the EXPORT as suggested by Christoph Hellwig
o Modify tcp_probe.c that uses the kprobe interface so as to make it
  work on multiple platforms (in its earlier form, the code wouldn't
  work, say, on powerpc)

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Prasanna S Panchamukhi <prasanna@in.ibm.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.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 |   15 +++++++++++++++
 include/linux/kprobes.h       |    6 ++++++
 kernel/kallsyms.c             |    1 -
 kernel/kprobes.c              |   26 ++++++++++++++++++++++++++
 net/ipv4/tcp_probe.c          |    6 ++++--
 10 files changed, 56 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,21 @@ typedef unsigned int kprobe_opcode_t;
 #define IS_TDI(instr)		(((instr) & 0xfc000000) == 0x08000000)
 #define IS_TWI(instr)		(((instr) & 0xfc000000) == 0x0c000000)
 
+/*
+ * 64bit powerpc uses function descriptors.
+ * Handle cases where:
+ * 		- User passes a <.symbol>
+ * 		- User passes a <symbol>
+ * 		- User passes a non-existant symbol, kallsyms_lookup_name
+ * 		  returns 0. Don't deref the NULL pointer in that case
+ */
+#define kprobe_lookup_name(name, addr)					\
+{									\
+	addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);		\
+	if (!(name[0] == '.') && addr)					\
+		addr = *(kprobe_opcode_t **)addr;			\
+}
+
 #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, addr) \
+	addr = ((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;
+		kprobe_lookup_name(p->symbol_name, p->addr);
+	}
+
+	if (!p->addr)
+		return -EINVAL;
+	p->addr = (kprobe_opcode_t *)(((char *)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 3/3] Update Documentation/kprobes.txt

[Ananth N Mavinakayanahalli]

On Tue, Aug 22, 2006 at 11:00:09AM +0530, Ananth N Mavinakayanahalli wrote:
> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> Documentation/kprobes.txt updated to reflect:
> 
> o In-kernel symbol resolution
> o CONFIG_KALLSYMS dependency
> o Usage of JPROBE_ENTRY
> o Addition of return_value()

Andrew,

Now that the proposed return_value() got changed to regs_return_value(),
the kprobes.txt needs to be updated so the example modules build
off-the-box.

Please drop update-documentation-kprobestxt.patch currently in -mm.
Please incorporate this patch instead.

Thanks,
Ananth

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

Documentation/kprobes.txt updated to reflect:

o In-kernel symbol resolution
o CONFIG_KALLSYMS dependency
o Usage of JPROBE_ENTRY
o Addition of regs_return_value()

Also update the usage examples to use correct module interfaces.
---

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

---
 Documentation/kprobes.txt |   89 ++++++++++++++++++++++++++++------------------
 1 files changed, 56 insertions(+), 33 deletions(-)

Index: linux-2.6.18-rc4/Documentation/kprobes.txt
===================================================================
--- linux-2.6.18-rc4.orig/Documentation/kprobes.txt
+++ linux-2.6.18-rc4/Documentation/kprobes.txt
@@ -151,9 +151,9 @@ So that you can load and unload Kprobes-
 make sure "Loadable module support" (CONFIG_MODULES) and "Module
 unloading" (CONFIG_MODULE_UNLOAD) are set to "y".
 
-You may also want to ensure that CONFIG_KALLSYMS and perhaps even
-CONFIG_KALLSYMS_ALL are set to "y", since kallsyms_lookup_name()
-is a handy, version-independent way to find a function's address.
+Also make sure that CONFIG_KALLSYMS and perhaps even CONFIG_KALLSYMS_ALL
+are set to "y", since kallsyms_lookup_name() is used by the in-kernel
+kprobe address resolution code.
 
 If you need to insert a probe in the middle of a function, you may find
 it useful to "Compile the kernel with debug info" (CONFIG_DEBUG_INFO),
@@ -179,6 +179,27 @@ 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";
+
+(64-bit powerpc intricacies such as function descriptors are handled
+transparently)
+
+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.
+
+3. Specify either the kprobe "symbol_name" OR the "addr". If both are
+specified, kprobe registration will fail with -EINVAL.
+
+4. With CISC architectures (such as i386 and x86_64), the kprobes code
+does not validate if the kprobe.addr is at an instruction boundary.
+Use "offset" with caution.
+
 register_kprobe() returns 0 on success, or a negative errno otherwise.
 
 User's pre-handler (kp->pre_handler):
@@ -225,6 +246,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 +278,11 @@ of interest:
 - ret_addr: the return address
 - rp: points to the corresponding kretprobe object
 - task: points to the corresponding task struct
+
+The regs_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 +401,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 +434,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 +450,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 +492,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 +513,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 +530,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 +556,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 = regs_return_value(regs);
 	if (retval < 0) {
 		printk("%s returns %d\n", probed_func, retval);
 	}
@@ -552,15 +575,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 +588,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 +597,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 +611,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)