mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Nick Andrew <nick@nick-andrew.net>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	netdev@vger.kernel.org, Bjorn Helgaas <bjorn.helgaas@hp.com>
Subject: [PATCH] vsprintf.c: remove stack variable ksym from
Date: Sat, 13 Mar 2010 11:54:39 -0800	[thread overview]
Message-ID: <1268510079.30289.69.camel@Joe-Laptop.home> (raw)
In-Reply-To: <1268502295.30289.44.camel@Joe-Laptop.home>

On Sat, 2010-03-13 at 09:44 -0800, Joe Perches wrote:
> On Sat, 2010-03-13 at 07:35 -0800, Linus Torvalds wrote:
> > On Fri, 12 Mar 2010, Andrew Morton wrote:
> > > nice.
> > But the kallsyms_lookup()/sprint_symbol() functions don't take a 
> > length parameter, so we have to do the worst-case thing (which itself has 
> > tons of unnecessary padding).
> Perhaps a new snprint_symbol function with the
> other kallsyms... functions changed as necessary.

Perhaps something like this:

Functions added:

kallsyms_lookup_n	length limited kallsyms_lookup
snprint_symbol		length limited sprint_symbol
module_address_lookup_n	length limited module_address_lookup

Changes to vsprintf.c, function symbol_string:

remove stack variable ksym, just use output buffer
and length limited functions.

Compiled, untested

Signed-off-by: Joe Perches
---
 include/linux/kallsyms.h |    7 ++++
 include/linux/module.h   |   15 ++++++++
 kernel/kallsyms.c        |   91 ++++++++++++++++++++++++++++++++++++++++------
 kernel/module.c          |   33 +++++++++++++++++
 lib/vsprintf.c           |   14 ++++---
 5 files changed, 143 insertions(+), 17 deletions(-)

diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index d8e9b3d..b1729b5 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -34,8 +34,15 @@ const char *kallsyms_lookup(unsigned long addr,
 			    unsigned long *offset,
 			    char **modname, char *namebuf);
 
+const char *kallsyms_lookup_n(unsigned long addr,
+			      unsigned long *symbolsize,
+			      unsigned long *offset,
+			      char **modname, char *namebuf, size_t size);
+
 /* Look up a kernel symbol and return it in a text buffer. */
 extern int sprint_symbol(char *buffer, unsigned long address);
+/* Look up a kernel symbol and return it in a text buffer. */
+extern int snprint_symbol(char *buffer, size_t size, unsigned long address);
 
 /* Look up a kernel symbol and print it to the kernel messages. */
 extern void __print_symbol(const char *fmt, unsigned long address);
diff --git a/include/linux/module.h b/include/linux/module.h
index 5e869ff..7a65fad 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -520,6 +520,11 @@ const char *module_address_lookup(unsigned long addr,
 			    unsigned long *offset,
 			    char **modname,
 			    char *namebuf);
+const char *module_address_lookup_n(unsigned long addr,
+				    unsigned long *symbolsize,
+				    unsigned long *offset,
+				    char **modname,
+				    char *namebuf, size_t size);
 int lookup_module_symbol_name(unsigned long addr, char *symname);
 int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name);
 
@@ -598,6 +603,16 @@ static inline const char *module_address_lookup(unsigned long addr,
 	return NULL;
 }
 
+/* For kallsyms to ask for address resolution.  NULL means not found. */
+static inline const char *module_address_lookup_n(unsigned long addr,
+						  unsigned long *symbolsize,
+						  unsigned long *offset,
+						  char **modname,
+						  char *namebuf, size_t size)
+{
+	return NULL;
+}
+
 static inline int lookup_module_symbol_name(unsigned long addr, char *symname)
 {
 	return -ERANGE;
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 8e5288a..0516d23 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -84,9 +84,11 @@ static int is_ksym_addr(unsigned long addr)
  * Expand a compressed symbol data into the resulting uncompressed string,
  * given the offset to where the symbol is in the compressed stream.
  */
-static unsigned int kallsyms_expand_symbol(unsigned int off, char *result)
+static unsigned int kallsyms_expand_symbol(unsigned int off,
+					   char *result, size_t size)
 {
-	int len, skipped_first = 0;
+	int len;
+	bool skipped_first = false;
 	const u8 *tptr, *data;
 
 	/* Get the compressed symbol length from the first symbol byte. */
@@ -110,16 +112,18 @@ static unsigned int kallsyms_expand_symbol(unsigned int off, char *result)
 		len--;
 
 		while (*tptr) {
-			if (skipped_first) {
+			if (skipped_first && size > 1) {
 				*result = *tptr;
 				result++;
+				size--;
 			} else
-				skipped_first = 1;
+				skipped_first = true;
 			tptr++;
 		}
 	}
 
-	*result = '\0';
+	if (size)
+		*result = '\0';
 
 	/* Return to offset to the next symbol. */
 	return off;
@@ -174,7 +178,7 @@ unsigned long kallsyms_lookup_name(const char *name)
 	unsigned int off;
 
 	for (i = 0, off = 0; i < kallsyms_num_syms; i++) {
-		off = kallsyms_expand_symbol(off, namebuf);
+		off = kallsyms_expand_symbol(off, namebuf, sizeof(namebuf));
 
 		if (strcmp(namebuf, name) == 0)
 			return kallsyms_addresses[i];
@@ -193,7 +197,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
 	int ret;
 
 	for (i = 0, off = 0; i < kallsyms_num_syms; i++) {
-		off = kallsyms_expand_symbol(off, namebuf);
+		off = kallsyms_expand_symbol(off, namebuf, sizeof(namebuf));
 		ret = fn(data, namebuf, NULL, kallsyms_addresses[i]);
 		if (ret != 0)
 			return ret;
@@ -292,7 +296,8 @@ const char *kallsyms_lookup(unsigned long addr,
 
 		pos = get_symbol_pos(addr, symbolsize, offset);
 		/* Grab name */
-		kallsyms_expand_symbol(get_symbol_offset(pos), namebuf);
+		kallsyms_expand_symbol(get_symbol_offset(pos),
+				       namebuf, KSYM_NAME_LEN);
 		if (modname)
 			*modname = NULL;
 		return namebuf;
@@ -303,6 +308,38 @@ const char *kallsyms_lookup(unsigned long addr,
 				     namebuf);
 }
 
+/*
+ * Lookup an address (length_limited)
+ * - modname is set to NULL if it's in the kernel.
+ * - We guarantee that the returned name is valid until we reschedule even if.
+ *   It resides in a module.
+ * - We also guarantee that modname will be valid until rescheduled.
+ */
+const char *kallsyms_lookup_n(unsigned long addr,
+			      unsigned long *symbolsize,
+			      unsigned long *offset,
+			      char **modname, char *namebuf, size_t size)
+{
+	if (size)
+		namebuf[size - 1] = 0;
+	namebuf[0] = 0;
+
+	if (is_ksym_addr(addr)) {
+		unsigned long pos;
+
+		pos = get_symbol_pos(addr, symbolsize, offset);
+		/* Grab name */
+		kallsyms_expand_symbol(get_symbol_offset(pos), namebuf, size);
+		if (modname)
+			*modname = NULL;
+		return namebuf;
+	}
+
+	/* See if it's in a module. */
+	return module_address_lookup_n(addr, symbolsize, offset, modname,
+				       namebuf, size);
+}
+
 int lookup_symbol_name(unsigned long addr, char *symname)
 {
 	symname[0] = '\0';
@@ -313,7 +350,8 @@ int lookup_symbol_name(unsigned long addr, char *symname)
 
 		pos = get_symbol_pos(addr, NULL, NULL);
 		/* Grab name */
-		kallsyms_expand_symbol(get_symbol_offset(pos), symname);
+		kallsyms_expand_symbol(get_symbol_offset(pos),
+				       symname, KSYM_NAME_LEN);
 		return 0;
 	}
 	/* See if it's in a module. */
@@ -331,7 +369,8 @@ int lookup_symbol_attrs(unsigned long addr, unsigned long *size,
 
 		pos = get_symbol_pos(addr, size, offset);
 		/* Grab name */
-		kallsyms_expand_symbol(get_symbol_offset(pos), name);
+		kallsyms_expand_symbol(get_symbol_offset(pos),
+				       name, KSYM_NAME_LEN);
 		modname[0] = '\0';
 		return 0;
 	}
@@ -340,6 +379,36 @@ int lookup_symbol_attrs(unsigned long addr, unsigned long *size,
 }
 
 /* Look up a kernel symbol and return it in a text buffer. */
+int snprint_symbol(char *buffer, size_t size, unsigned long address)
+{
+	char *modname;
+	const char *name;
+	unsigned long offset, ksize;
+	int len;
+
+	name = kallsyms_lookup_n(address, &ksize, &offset, &modname,
+				 buffer, size);
+	if (!name)
+		return snprintf(buffer, size, "0x%lx", address);
+
+	if (name != buffer)
+		strncpy(buffer, name, size);
+	len = strlen(buffer);
+	buffer += len;
+
+	if (modname)
+		len += snprintf(buffer, size - len,
+				"+%#lx/%#lx [%s]", offset, ksize, modname);
+	else
+		len += snprintf(buffer, size - len,
+				"+%#lx/%#lx", offset, ksize);
+
+	return len;
+}
+EXPORT_SYMBOL_GPL(snprint_symbol);
+
+
+/* Look up a kernel symbol and return it in a text buffer. */
 int sprint_symbol(char *buffer, unsigned long address)
 {
 	char *modname;
@@ -407,7 +476,7 @@ static unsigned long get_ksymbol_core(struct kallsym_iter *iter)
 
 	iter->type = kallsyms_get_symbol_type(off);
 
-	off = kallsyms_expand_symbol(off, iter->name);
+	off = kallsyms_expand_symbol(off, iter->name, sizeof(iter->name));
 
 	return off - iter->nameoff;
 }
diff --git a/kernel/module.c b/kernel/module.c
index c968d36..17a879a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2607,6 +2607,39 @@ const char *module_address_lookup(unsigned long addr,
 	return ret;
 }
 
+/* For kallsyms to ask for address resolution.  NULL means not found.  Careful
+ * not to lock to avoid deadlock on oopses, simply disable preemption. */
+const char *module_address_lookup_n(unsigned long addr,
+				    unsigned long *ksize,
+				    unsigned long *offset,
+				    char **modname,
+				    char *namebuf, size_t size)
+{
+	struct module *mod;
+	const char *ret = NULL;
+
+	preempt_disable();
+	list_for_each_entry_rcu(mod, &modules, list) {
+		if (within_module_init(addr, mod) ||
+		    within_module_core(addr, mod)) {
+			if (modname)
+				*modname = mod->name;
+			ret = get_ksymbol(mod, addr, ksize, offset);
+			break;
+		}
+	}
+	/* Make a copy in here where it's safe */
+	if (ret) {
+		if (size) {
+			strncpy(namebuf, ret, size - 1);
+			namebuf[size - 1] = 0;
+		}
+		ret = namebuf;
+	}
+	preempt_enable();
+	return ret;
+}
+
 int lookup_module_symbol_name(unsigned long addr, char *symname)
 {
 	struct module *mod;
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 0d461c7..dab6354 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -571,13 +571,15 @@ static char *symbol_string(char *buf, char *end, void *ptr,
 {
 	unsigned long value = (unsigned long) ptr;
 #ifdef CONFIG_KALLSYMS
-	char sym[KSYM_SYMBOL_LEN];
-	if (ext != 'f' && ext != 's')
-		sprint_symbol(sym, value);
-	else
-		kallsyms_lookup(value, NULL, NULL, NULL, sym);
+	/* Cap ending position to spec.field_width if specified */
+	if (spec.field_width > 0 && (end - buf) > spec.field_width)
+		end = buf + spec.field_width;
 
-	return string(buf, end, sym, spec);
+	if (ext != 'f' && ext != 's')
+		return buf + snprint_symbol(buf, end - buf, value);
+		
+	kallsyms_lookup_n(value, NULL, NULL, NULL, buf, end - buf);
+	return buf + strlen(buf);
 #else
 	spec.field_width = 2 * sizeof(void *);
 	spec.flags |= SPECIAL | SMALL | ZEROPAD;



  reply	other threads:[~2010-03-13 19:54 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-04  6:21 [PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf Joe Perches
2010-03-04  6:21 ` [PATCH 1/2] vsprintf: Recursive vsnprintf: Add "%pV", struct va_format Joe Perches
2010-03-04  6:21 ` [PATCH 2/2] device.h drivers/base/core.c Convert dev_<level> macros to functions Joe Perches
2010-03-05  0:56   ` Andrew Morton
2010-03-05  1:00     ` Andrew Morton
2010-03-05  2:46       ` Joe Perches
2010-03-04 22:38 ` [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf Andrew Morton
2010-03-04 23:06   ` Linus Torvalds
2010-03-06 21:36     ` Joe Perches
2010-03-06 22:03       ` Linus Torvalds
2010-03-06 22:30         ` Joe Perches
2010-03-06 22:52           ` Linus Torvalds
2010-03-06 22:57             ` Linus Torvalds
2010-03-06 23:35               ` Joe Perches
2010-03-06 23:46                 ` Linus Torvalds
2010-03-06 23:48                   ` Linus Torvalds
2010-03-06 23:57                     ` Joe Perches
2010-03-06 23:58                     ` Linus Torvalds
2010-03-07  1:10                       ` [PATCH] vsprintf.c: Reduce sizeof struct printf_spec from 24 to 8 bytes Joe Perches
2010-03-07  2:03                         ` Linus Torvalds
2010-03-07  2:24                           ` Linus Torvalds
2010-03-07  2:33                           ` [PATCH] vsprintf.c: Use noinline_for_stack Joe Perches
2010-03-08 23:39                             ` Joe Perches
2010-03-13  0:25                             ` Andrew Morton
2010-03-13 15:35                               ` Linus Torvalds
2010-03-13 17:44                                 ` Joe Perches
2010-03-13 19:54                                   ` Joe Perches [this message]
2010-03-15 15:01                                     ` [PATCH] vsprintf.c: remove stack variable ksym from Paulo Marques

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=1268510079.30289.69.camel@Joe-Laptop.home \
    --to=joe@perches.com \
    --cc=akpm@linux-foundation.org \
    --cc=bjorn.helgaas@hp.com \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nick@nick-andrew.net \
    --cc=torvalds@linux-foundation.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

all inboxes | Powered by JetHome®