mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Christoph Hellwig <hch@infradead.org>,
	David Smith <dsmith@redhat.com>,
	Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	linux-kernel@vger.kernel.org, shemminger@osdl.org
Subject: Re: [PATCH 1/3] Kprobes: Make kprobe modules more portable
Date: Wed, 9 Aug 2006 17:18:54 +0100	[thread overview]
Message-ID: <20060809161854.GA13622@infradead.org> (raw)
In-Reply-To: <20060809161039.GA30856@infradead.org>

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

  reply	other threads:[~2006-08-09 16:19 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-07 11:55 Ananth N Mavinakayanahalli
2006-08-07 12:00 ` [PATCH 2/3] Kprobes: Define retval helper Ananth N Mavinakayanahalli
2006-08-07 12:04   ` [PATCH 3/3] Kprobes: Update Documentation/kprobes.txt Ananth N Mavinakayanahalli
2006-08-08 16:27     ` Christoph Hellwig
2006-08-09  9:48       ` Ananth N Mavinakayanahalli
2006-08-09 10:24         ` Ananth N Mavinakayanahalli
2006-08-08 16:25   ` [PATCH 2/3] Kprobes: Define retval helper Christoph Hellwig
2006-08-09  4:37     ` Andi Kleen
2006-08-09  9:43     ` Ananth N Mavinakayanahalli
2006-08-09  9:45       ` Christoph Hellwig
2006-08-09  9:55         ` Andi Kleen
2006-08-09 10:19           ` Ananth N Mavinakayanahalli
2006-08-09 10:28             ` Martin Schwidefsky
2006-08-09 13:16       ` David Howells
2006-08-09 15:20         ` Ananth N Mavinakayanahalli
2006-08-07 12:06 ` [PATCH 1/3] Kprobes: Make kprobe modules more portable Ananth N Mavinakayanahalli
2006-08-08 16:24 ` Christoph Hellwig
2006-08-08 16:34   ` Stephen Hemminger
2006-08-08 16:40     ` Christoph Hellwig
2006-08-08 17:28       ` Paulo Marques
2006-08-08 17:32         ` Christoph Hellwig
2006-08-09  9:51   ` Ananth N Mavinakayanahalli
2006-08-09 16:01   ` David Smith
2006-08-09 16:10     ` Christoph Hellwig
2006-08-09 16:18       ` Christoph Hellwig [this message]
2006-08-10 14:10         ` Keshavamurthy Anil S
2006-08-16 13:20           ` Christoph Hellwig
2006-08-11  8:53         ` Ananth N Mavinakayanahalli
2006-08-10  6:18       ` Ananth N Mavinakayanahalli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060809161854.GA13622@infradead.org \
    --to=hch@infradead.org \
    --cc=ananth@in.ibm.com \
    --cc=dsmith@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shemminger@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome