module: move find_module check to end (3bafeb62) · Commits · 戴 / test

I think Rusty may have made the lock a bit _too_ finegrained there, and didn't add it to some places that needed it. It looks, for example, like PATCH 1/2 actually drops the lock in places where it's needed ("find_module()" is documented to need it, but now load_module() didn't hold it at all when it did the find_module()). Rather than adding a new "module_loading" list, I think we should be able to just use the existing "modules" list, and just fix up the locking a bit. In fact, maybe we could just move the "look up existing module" a bit later - optimistically assuming that the module doesn't exist, and then just undoing the work if it turns out that we were wrong, just before adding ourselves to the list. Signed-off-by:

Rusty Russell <rusty@rustcorp.com.au>

kernel/module.c

+7 −5

Original line number	Diff line number	Diff line
		@@ -2226,11 +2226,6 @@ static noinline struct module load_module(void __user umod,
		goto free_mod;
		}

		if (find_module(mod->name)) {
		err = -EEXIST;
		goto free_mod;
		}

		mod->state = MODULE_STATE_COMING;

		/* Allow arches to frob section contents and sizes. */
		@@ -2509,6 +2504,12 @@ static noinline struct module load_module(void __user umod,
		* The mutex protects against concurrent writers.
		*/
		mutex_lock(&module_mutex);
		if (find_module(mod->name)) {
		err = -EEXIST;
		/* This will also unlock the mutex */
		goto already_exists;
		}

		list_add_rcu(&mod->list, &modules);
		mutex_unlock(&module_mutex);

		@@ -2535,6 +2536,7 @@ static noinline struct module load_module(void __user umod,
		mutex_lock(&module_mutex);
		/* Unlink carefully: kallsyms could be walking list. */
		list_del_rcu(&mod->list);
		already_exists:
		mutex_unlock(&module_mutex);
		synchronize_sched();
		module_arch_cleanup(mod);

Admin message