Skip to contentSkip to conversation

Fix race between cat /proc/slab_allocators and rmmod

PATCH In review Alexey Dobriyan 3 participants 4 messages Apr 2, 2007 – Apr 4, 2007

Subsystems (2)
Memory management Module support
lore
Share & tools
lore

Alexey Dobriyan

author
#0 lore raw

PATCH 5/5 Fix race between cat /proc/slab_allocators and rmmod

Same story as with cat /proc/*/wchan race vs rmmod race, only /proc/slab_allocators want more info than just symbol name.

Signed-off-by Alexey Dobriyan <[email protected]>

---

 include/linux/kallsyms.h |    6 ++++++
 include/linux/module.h   |    6 ++++++
 kernel/kallsyms.c        |   19 +++++++++++++++++++
 kernel/module.c          |   27 +++++++++++++++++++++++++++
 mm/slab.c                |   10 +++-------
 5 files changed, 61 insertions(+), 7 deletions(-)
include/linux/kallsyms.h +6 -0
@@ -23,6 +23,7 @@ const char *kallsyms_lookup(unsigned lon
 			    char **modname, char *namebuf);
 
 int lookup_symbol_name(unsigned long addr, char *symname);
+int lookup_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name);
 
 /* Replace "%s" in format with address, if found */
 extern void __print_symbol(const char *fmt, unsigned long address);

@@ -54,6 +55,11 @@ static inline int lookup_symbol_name(uns
 	return -ERANGE;
 }
 
+static inline int lookup_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name)
+{
+	return -ERANGE;
+}
+
 /* Stupid that this does nothing, but I didn't create this mess. */
 #define __print_symbol(fmt, addr)
 #endif /*CONFIG_KALLSYMS*/
include/linux/module.h +6 -0
@@ -457,6 +457,7 @@ const char *module_address_lookup(unsign
 				  unsigned long *offset,
 				  char **modname);
 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);
 
 /* For extable.c to search modules' exception tables. */
 const struct exception_table_entry *search_module_extables(unsigned long addr);

@@ -533,6 +534,11 @@ static inline int lookup_module_symbol_n
 	return -ERANGE;
 }
 
+static inline int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name)
+{
+	return -ERANGE;
+}
+
 static inline int module_get_kallsym(unsigned int symnum, unsigned long *value,
 					char *type, char *name,
 					char *module_name, int *exported)
kernel/kallsyms.c +19 -0
@@ -286,6 +286,25 @@ int lookup_symbol_name(unsigned long add
 	return lookup_module_symbol_name(addr, symname);
 }
 
+int lookup_symbol_attrs(unsigned long addr, unsigned long *size,
+			unsigned long *offset, char *modname, char *name)
+{
+	name[0] = '\0';
+	name[KSYM_NAME_LEN] = '\0';
+
+	if (is_ksym_addr(addr)) {
+		unsigned long pos;
+
+		pos = get_symbol_pos(addr, size, offset);
+		/* Grab name */
+		kallsyms_expand_symbol(get_symbol_offset(pos), name);
+		modname[0] = '\0';
+		return 0;
+	}
+	/* see if it's in a module */
+	return lookup_module_symbol_attrs(addr, size, offset, modname, name);
+}
+
 /* Replace "%s" in format with address, or returns -errno. */
 void __print_symbol(const char *fmt, unsigned long address)
 {
kernel/module.c +27 -0
@@ -2145,6 +2145,33 @@ out:
 	return -ERANGE;
 }
 
+int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size,
+			unsigned long *offset, char *modname, char *name)
+{
+	struct module *mod;
+
+	mutex_lock(&module_mutex);
+	list_for_each_entry(mod, &modules, list) {
+		if (within(addr, mod->module_init, mod->init_size) ||
+		    within(addr, mod->module_core, mod->core_size)) {
+			const char *sym;
+
+			sym = get_ksymbol(mod, addr, size, offset);
+			if (!sym)
+				goto out;
+			if (modname)
+				strlcpy(modname, mod->name, MODULE_NAME_LEN + 1);
+			if (name)
+				strlcpy(name, sym, KSYM_NAME_LEN + 1);
+			mutex_unlock(&module_mutex);
+			return 0;
+		}
+	}
+out:
+	mutex_unlock(&module_mutex);
+	return -ERANGE;
+}
+
 int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
 			char *name, char *module_name, int *exported)
 {
mm/slab.c +3 -7
@@ -4380,16 +4380,12 @@ static void handle_slab(unsigned long *n
 static void show_symbol(struct seq_file *m, unsigned long address)
 {
 #ifdef CONFIG_KALLSYMS
-	char *modname;
-	const char *name;
 	unsigned long offset, size;
-	char namebuf[KSYM_NAME_LEN+1];
-
-	name = kallsyms_lookup(address, &size, &offset, &modname, namebuf);
+	char modname[MODULE_NAME_LEN + 1], name[KSYM_NAME_LEN + 1];
 
-	if (name) {
+	if (lookup_symbol_attrs(address, &size, &offset, modname, name) == 0) {
 		seq_printf(m, "%s+%#lx/%#lx", name, offset, size);
-		if (modname)
+		if (modname[0])
 			seq_printf(m, " [%s]", modname);
 		return;
 	}
#1 lore raw
Quoted 4 lines · Alexey Dobriyan

Same story as with cat /proc/*/wchan race vs rmmod race, only /proc/slab_allocators want more info than just symbol name.

Signed-off-by Alexey Dobriyan <[email protected]>

All these look excellent. I hope Andrew picked them up?

Thanks,
Rusty.

maintainer
#2 lore raw
Quoted 8 lines · On Tue, 03 Apr 2007 09:09:55 +1000 Rusty Russell <[email protected]> wrote:
Quoted 4 lines · Alexey Dobriyan

Same story as with cat /proc/*/wchan race vs rmmod race, only /proc/slab_allocators want more info than just symbol name.

Signed-off-by Alexey Dobriyan <[email protected]>

All these look excellent. I hope Andrew picked them up?

I wasn't cc'ed on them, but I'll push them into the hole marked "In" anyway.

maintainer
#3 lore raw
In reply to Alexey Dobriyan
Quoted 17 lines · On Mon, 2 Apr 2007 19:03:16 +0400 Alexey Dobriyan <[email protected]> wrote:
kernel/module.c +17 -0
+int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size,
+			unsigned long *offset, char *modname, char *name)
+{
+	struct module *mod;
+
+	mutex_lock(&module_mutex);
+	list_for_each_entry(mod, &modules, list) {
+		if (within(addr, mod->module_init, mod->init_size) ||
+		    within(addr, mod->module_core, mod->core_size)) {
+			const char *sym;
+
+			sym = get_ksymbol(mod, addr, size, offset);
+			if (!sym)
+				goto out;
+			if (modname)
+				strlcpy(modname, mod->name, MODULE_NAME_LEN + 1);
+			if (name)

kernel/module.c out: · line 2174

Quoted 10 lines
kernel/module.c +10 -0
+				strlcpy(name, sym, KSYM_NAME_LEN + 1);
+			mutex_unlock(&module_mutex);
+			return 0;
+		}
+	}
+out:
+	mutex_unlock(&module_mutex);
+	return -ERANGE;
+}
+

The functions lookup_symbol_name() and lookup_symbol_attrs() are quite general and are likely to be used for other applications in the future.

Could we please pick better names for them? The names you've chosen are far too general-sounding - we have a lot of different types of "symbol" in the kernel! Perhaps module_lookup_symbol_attrs() and kallsyms_lookup_symbol() and kallsyms_lookup_attrs() or something. But something prefixed with kallsyms_ and module_.

Also, as these general-use functions are exported to all of vmlinux, a little bit of documentation for them would be good.