module: make module_address_lookup safe

module_address_lookup releases preemption then returns a pointer into
the module space.  The only user (kallsyms) copies the result, so just
do that under the preempt disable.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/module.h b/include/linux/module.h
index c97bdb7eb957cdd71e625ac9cf66862132d153da..aedc06be1de846d4b9b8b3bb73dd2912fcd2c3a2 100644 (file)
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -446,11 +446,14 @@ static inline void __module_get(struct module *module)
        __mod ? __mod->name : "kernel";         \
 })
 
-/* For kallsyms to ask for address resolution.  NULL means not found. */
-const char *module_address_lookup(unsigned long addr,
-                                 unsigned long *symbolsize,
-                                 unsigned long *offset,
-                                 char **modname);
+/* For kallsyms to ask for address resolution.  namebuf should be at
+ * least KSYM_NAME_LEN long: a pointer to namebuf is returned if
+ * found, otherwise NULL. */
+char *module_address_lookup(unsigned long addr,
+                           unsigned long *symbolsize,
+                           unsigned long *offset,
+                           char **modname,
+                           char *namebuf);
 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);
 
@@ -516,10 +519,11 @@ static inline void module_put(struct module *module)
 #define module_name(mod) "kernel"
 
 /* For kallsyms to ask for address resolution.  NULL means not found. */
-static inline const char *module_address_lookup(unsigned long addr,
-                                               unsigned long *symbolsize,
-                                               unsigned long *offset,
-                                               char **modname)
+static inline char *module_address_lookup(unsigned long addr,
+                                         unsigned long *symbolsize,
+                                         unsigned long *offset,
+                                         char **modname,
+                                         char *namebuf)
 {
        return NULL;
 }

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 2fc25810509e187441297e4d2b8873e3d2db8ea3..7dadc71ce5162926529172e5a9ff71b86590a4a4 100644 (file)
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -233,10 +233,11 @@ static unsigned long get_symbol_pos(unsigned long addr,
 int kallsyms_lookup_size_offset(unsigned long addr, unsigned long *symbolsize,
                                unsigned long *offset)
 {
+       char namebuf[KSYM_NAME_LEN];
        if (is_ksym_addr(addr))
                return !!get_symbol_pos(addr, symbolsize, offset);
 
-       return !!module_address_lookup(addr, symbolsize, offset, NULL);
+       return !!module_address_lookup(addr, symbolsize, offset, NULL, namebuf);
 }
 
 /*
@@ -251,8 +252,6 @@ const char *kallsyms_lookup(unsigned long addr,
                            unsigned long *offset,
                            char **modname, char *namebuf)
 {
-       const char *msym;
-
        namebuf[KSYM_NAME_LEN - 1] = 0;
        namebuf[0] = 0;
 
@@ -268,10 +267,8 @@ const char *kallsyms_lookup(unsigned long addr,
        }
 
        /* see if it's in a module */
-       msym = module_address_lookup(addr, symbolsize, offset, modname);
-       if (msym)
-               return strncpy(namebuf, msym, KSYM_NAME_LEN - 1);
-
+       return module_address_lookup(addr, symbolsize, offset, modname,
+                                    namebuf);
        return NULL;
 }
 

diff --git a/kernel/module.c b/kernel/module.c
index 12067ff34d01c242e637bddf460d65328c293a63..e814cd7da63420acdd78d79df2f7cb89dcf86619 100644 (file)
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2230,14 +2230,13 @@ static const char *get_ksymbol(struct module *mod,
        return mod->strtab + mod->symtab[best].st_name;
 }
 
-/* For kallsyms to ask for address resolution.  NULL means not found.
-   We don't lock, as this is used for oops resolution and races are a
-   lesser concern. */
-/* FIXME: Risky: returns a pointer into a module w/o lock */
-const char *module_address_lookup(unsigned long addr,
-                                 unsigned long *size,
-                                 unsigned long *offset,
-                                 char **modname)
+/* For kallsyms to ask for address resolution.  NULL means not found.  Careful
+ * not to lock to avoid deadlock on oopses, simply disable preemption. */
+char *module_address_lookup(unsigned long addr,
+                           unsigned long *size,
+                           unsigned long *offset,
+                           char **modname,
+                           char *namebuf)
 {
        struct module *mod;
        const char *ret = NULL;
@@ -2252,8 +2251,13 @@ const char *module_address_lookup(unsigned long addr,
                        break;
                }
        }
+       /* Make a copy in here where it's safe */
+       if (ret) {
+               strncpy(namebuf, ret, KSYM_NAME_LEN - 1);
+               ret = namebuf;
+       }
        preempt_enable();
-       return ret;
+       return (char *)ret;
 }
 
 int lookup_module_symbol_name(unsigned long addr, char *symname)