Commit be593f4c authored by Rusty Russell's avatar Rusty Russell

module: verify_export_symbols under the lock

It disabled preempt so it was "safe", but nothing stops another module
slipping in before this module is added to the global list now we don't
hold the lock the whole time.

So we check this just after we check for duplicate modules, and just
before we put the module in the global list.

(find_symbol finds symbols in coming and going modules, too).
Signed-off-by: default avatarRusty Russell <rusty@rustcorp.com.au>
parent 3bafeb62
...@@ -1571,6 +1571,8 @@ EXPORT_SYMBOL_GPL(__symbol_get); ...@@ -1571,6 +1571,8 @@ EXPORT_SYMBOL_GPL(__symbol_get);
/* /*
* Ensure that an exported symbol [global namespace] does not already exist * Ensure that an exported symbol [global namespace] does not already exist
* in the kernel or in some other module's exported symbol table. * in the kernel or in some other module's exported symbol table.
*
* You must hold the module_mutex.
*/ */
static int verify_export_symbols(struct module *mod) static int verify_export_symbols(struct module *mod)
{ {
...@@ -1592,14 +1594,7 @@ static int verify_export_symbols(struct module *mod) ...@@ -1592,14 +1594,7 @@ static int verify_export_symbols(struct module *mod)
for (i = 0; i < ARRAY_SIZE(arr); i++) { for (i = 0; i < ARRAY_SIZE(arr); i++) {
for (s = arr[i].sym; s < arr[i].sym + arr[i].num; s++) { for (s = arr[i].sym; s < arr[i].sym + arr[i].num; s++) {
const struct kernel_symbol *sym; if (find_symbol(s->name, &owner, NULL, true, false)) {
/* Stopping preemption makes find_symbol safe. */
preempt_disable();
sym = find_symbol(s->name, &owner, NULL, true, false);
preempt_enable();
if (sym) {
printk(KERN_ERR printk(KERN_ERR
"%s: exports duplicate symbol %s" "%s: exports duplicate symbol %s"
" (owned by %s)\n", " (owned by %s)\n",
...@@ -2440,11 +2435,6 @@ static noinline struct module *load_module(void __user *umod, ...@@ -2440,11 +2435,6 @@ static noinline struct module *load_module(void __user *umod,
goto cleanup; goto cleanup;
} }
/* Find duplicate symbols */
err = verify_export_symbols(mod);
if (err < 0)
goto cleanup;
/* Set up and sort exception table */ /* Set up and sort exception table */
mod->extable = section_objs(hdr, sechdrs, secstrings, "__ex_table", mod->extable = section_objs(hdr, sechdrs, secstrings, "__ex_table",
sizeof(*mod->extable), &mod->num_exentries); sizeof(*mod->extable), &mod->num_exentries);
...@@ -2506,10 +2496,14 @@ static noinline struct module *load_module(void __user *umod, ...@@ -2506,10 +2496,14 @@ static noinline struct module *load_module(void __user *umod,
mutex_lock(&module_mutex); mutex_lock(&module_mutex);
if (find_module(mod->name)) { if (find_module(mod->name)) {
err = -EEXIST; err = -EEXIST;
/* This will also unlock the mutex */ goto unlock;
goto already_exists;
} }
/* Find duplicate symbols */
err = verify_export_symbols(mod);
if (err < 0)
goto unlock;
list_add_rcu(&mod->list, &modules); list_add_rcu(&mod->list, &modules);
mutex_unlock(&module_mutex); mutex_unlock(&module_mutex);
...@@ -2536,7 +2530,7 @@ static noinline struct module *load_module(void __user *umod, ...@@ -2536,7 +2530,7 @@ static noinline struct module *load_module(void __user *umod,
mutex_lock(&module_mutex); mutex_lock(&module_mutex);
/* Unlink carefully: kallsyms could be walking list. */ /* Unlink carefully: kallsyms could be walking list. */
list_del_rcu(&mod->list); list_del_rcu(&mod->list);
already_exists: unlock:
mutex_unlock(&module_mutex); mutex_unlock(&module_mutex);
synchronize_sched(); synchronize_sched();
module_arch_cleanup(mod); module_arch_cleanup(mod);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment