Ingo Molnar wrote:
> * Alexey Dobriyan <adobriyan@sw.ru> wrote:
>
>> [cc'ing folks whose proc files are affected]
>>
>> kallsyms_lookup() can call module_address_lookup() which iterates over
>> modules list without module_mutex taken. Comment at the top of
>> module_address_lookup() says it's for oops resolution so races are
>> irrelevant, but in some cases it's reachable from regular code:
>
> looking at the problem from another angle: wouldnt this be something
> that would benefit from freeze_processes()/unfreeze_processes(), and
> hence no locking would be required?
I also considered this, but it seemed a little too "blunt" to stop
everything (including completely unrelated processes and kernel threads)
just to remove a module.
How about something like this instead? (just for review)
It's a little more intrusive, since the interface for symbol lookup is
changed (and it affects the callers), but it makes the caller aware that
it should set "safe_to_lock" if possible.
This way we avoid exposing module_mutex outside of module.c and avoid
returning any data pointer to some data structure that might disappear
before the caller tries to use it.
Some of these changes are actually cleanups, because the callers where
creating a dummy modname variables that they didn't use afterwards.
The thing I like the less about this patch is the increase of stack
usage on some code paths by about 60 bytes.
Anyway, I don't have very strong feelings about this, so if you think it
would be better to use freeze_processes()/unfreeze_processes(), I can
cook up a patch for that too...
--
Paulo Marques - www.grupopie.com
"Very funny Scotty. Now beam up my clothes."
diffstat:
> arch/parisc/kernel/unwind.c | 3 --
> arch/powerpc/xmon/xmon.c | 11 ++++-----
> arch/ppc/xmon/xmon.c | 8 +++---
> arch/sh64/kernel/unwind.c | 4 +--
> arch/x86_64/kernel/traps.c | 10 ++++----
> fs/proc/base.c | 3 --
> include/linux/kallsyms.h | 6 +++-
> include/linux/module.h | 44 +++++++++++++++++++-----------------
> kernel/kallsyms.c | 53 +++++++++++++++++++++-----------------------
> kernel/kprobes.c | 6 ++--
> kernel/lockdep.c | 3 --
> kernel/module.c | 44 +++++++++++++++++++++++++++---------
> kernel/time/timer_list.c | 3 --
> kernel/time/timer_stats.c | 3 --
> mm/slab.c | 6 ++--
> 15 files changed, 114 insertions(+), 93 deletions(-)
diff --git a/arch/parisc/kernel/unwind.c b/arch/parisc/kernel/unwind.c
--- a/arch/parisc/kernel/unwind.c
+++ b/arch/parisc/kernel/unwind.c
@@ -216,11 +216,10 @@ static void unwind_frame_regs(struct unw
/* Handle some frequent special cases.... */
{
char symname[KSYM_NAME_LEN+1];
- char *modname;
unsigned long symsize, offset;
kallsyms_lookup(info->ip, &symsize, &offset,
- &modname, symname);
+ NULL, symname, 0);
dbg("info->ip = 0x%lx, name = %s\n", info->ip, symname);
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1218,7 +1218,6 @@ static void get_function_bounds(unsigned
{
unsigned long size, offset;
const char *name;
- char *modname;
*startp = *endp = 0;
if (pc == 0)
@@ -1226,7 +1225,7 @@ static void get_function_bounds(unsigned
if (setjmp(bus_error_jmp) == 0) {
catch_memory_errors = 1;
sync();
- name = kallsyms_lookup(pc, &size, &offset, &modname, tmpstr);
+ name = kallsyms_lookup(pc, &size, &offset, NULL, tmpstr, 0);
if (name != NULL) {
*startp = pc - offset;
*endp = pc - offset + size;
@@ -2496,7 +2495,7 @@ symbol_lookup(void)
static void xmon_print_symbol(unsigned long address, const char *mid,
const char *after)
{
- char *modname;
+ char modname[MODULE_NAME_LEN + 1];
const char *name = NULL;
unsigned long offset, size;
@@ -2504,8 +2503,8 @@ static void xmon_print_symbol(unsigned l
if (setjmp(bus_error_jmp) == 0) {
catch_memory_errors = 1;
sync();
- name = kallsyms_lookup(address, &size, &offset, &modname,
- tmpstr);
+ name = kallsyms_lookup(address, &size, &offset, modname,
+ tmpstr, 0);
sync();
/* wait a little while to see if we get a machine check */
__delay(200);
@@ -2515,7 +2514,7 @@ static void xmon_print_symbol(unsigned l
if (name) {
printf("%s%s+%#lx/%#lx", mid, name, offset, size);
- if (modname)
+ if (modname[0])
printf(" [%s]", modname);
}
printf("%s", after);
diff --git a/arch/ppc/xmon/xmon.c b/arch/ppc/xmon/xmon.c
--- a/arch/ppc/xmon/xmon.c
+++ b/arch/ppc/xmon/xmon.c
@@ -177,7 +177,7 @@ extern inline void __delay(unsigned int
static void xmon_print_symbol(unsigned long address, const char *mid,
const char *after)
{
- char *modname;
+ char modname[MODULE_NAME_LEN+1];
const char *name = NULL;
unsigned long offset, size;
static char tmpstr[128];
@@ -186,8 +186,8 @@ static void xmon_print_symbol(unsigned l
if (setjmp(bus_error_jmp) == 0) {
debugger_fault_handler = handle_fault;
sync();
- name = kallsyms_lookup(address, &size, &offset, &modname,
- tmpstr);
+ name = kallsyms_lookup(address, &size, &offset, modname,
+ tmpstr, 0);
sync();
/* wait a little while to see if we get a machine check */
__delay(200);
@@ -196,7 +196,7 @@ static void xmon_print_symbol(unsigned l
if (name) {
printf("%s%s+%#lx/%#lx", mid, name, offset, size);
- if (modname)
+ if (modname[0])
printf(" [%s]", modname);
}
printf("%s", after);
diff --git a/arch/sh64/kernel/unwind.c b/arch/sh64/kernel/unwind.c
--- a/arch/sh64/kernel/unwind.c
+++ b/arch/sh64/kernel/unwind.c
@@ -46,7 +46,7 @@ static int lookup_prev_stack_frame(unsig
struct pt_regs *regs)
{
const char *sym;
- char *modname, namebuf[128];
+ char namebuf[128];
unsigned long offset, size;
unsigned long prologue = 0;
unsigned long fp_displacement = 0;
@@ -54,7 +54,7 @@ static int lookup_prev_stack_frame(unsig
unsigned long offset_r14 = 0, offset_r18 = 0;
int i, found_prologue_end = 0;
- sym = kallsyms_lookup(pc, &size, &offset, &modname, namebuf);
+ sym = kallsyms_lookup(pc, &size, &offset, NULL, namebuf, 0);
if (!sym)
return -EINVAL;
diff --git a/arch/x86_64/kernel/traps.c b/arch/x86_64/kernel/traps.c
--- a/arch/x86_64/kernel/traps.c
+++ b/arch/x86_64/kernel/traps.c
@@ -116,18 +116,18 @@ void printk_address(unsigned long addres
{
unsigned long offset = 0, symsize;
const char *symname;
- char *modname;
+ char modname[MODULE_NAME_LEN+1];
char *delim = ":";
- char namebuf[128];
+ char namebuf[KSYM_NAME_LEN+1];
symname = kallsyms_lookup(address, &symsize, &offset,
- &modname, namebuf);
+ modname, namebuf, 0);
if (!symname) {
printk(" [<%016lx>]\n", address);
return;
}
- if (!modname)
- modname = delim = "";
+ if (!modname[0])
+ delim = "";
printk(" [<%016lx>] %s%s%s%s+0x%lx/0x%lx\n",
address, delim, modname, delim, symname, offset, symsize);
}
diff --git a/fs/proc/base.c b/fs/proc/base.c
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -275,14 +275,13 @@ static int proc_pid_auxv(struct task_str
*/
static int proc_pid_wchan(struct task_struct *task, char *buffer)
{
- char *modname;
const char *sym_name;
unsigned long wchan, size, offset;
char namebuf[KSYM_NAME_LEN+1];
wchan = get_wchan(task);
- sym_name = kallsyms_lookup(wchan, &size, &offset, &modname, namebuf);
+ sym_name = kallsyms_lookup(wchan, &size, &offset, NULL, namebuf, 1);
if (sym_name)
return sprintf(buffer, "%s", sym_name);
return sprintf(buffer, "%lu", wchan);
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -20,7 +20,8 @@ extern int kallsyms_lookup_size_offset(u
const char *kallsyms_lookup(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset,
- char **modname, char *namebuf);
+ char *modname, char *namebuf,
+ int safe_to_lock);
/* Replace "%s" in format with address, if found */
extern void __print_symbol(const char *fmt, unsigned long address);
@@ -42,7 +43,8 @@ static inline int kallsyms_lookup_size_o
static inline const char *kallsyms_lookup(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset,
- char **modname, char *namebuf)
+ char *modname, char *namebuf,
+ int safe_to_lock)
{
return NULL;
}
diff --git a/include/linux/module.h b/include/linux/module.h
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -370,10 +370,10 @@ struct module *module_text_address(unsig
struct module *__module_text_address(unsigned long addr);
int is_module_address(unsigned long addr);
-/* Returns module and fills in value, defined and namebuf, or NULL if
- symnum out of range. */
-struct module *module_get_kallsym(unsigned int symnum, unsigned long *value,
- char *type, char *name, size_t namelen);
+/* fills in value, type, name and module_name. returns 0 on success
+ or -ENOENT if symnum out of range. */
+int module_get_kallsym(unsigned int symnum, unsigned long *value,
+ char *type, char *name, char *module_name);
/* Look for this name: can be of form module:name. */
unsigned long module_kallsyms_lookup_name(const char *name);
@@ -451,11 +451,15 @@ do { \
} \
} while(0)
-/* For kallsyms to ask for address resolution. NULL means not found. */
-const char *module_address_lookup(unsigned long addr,
-
...
|
OpenVZ Forum
Members
Search
Help
Register
Login
Home
