Re: [Bug 111541] Race between cat /proc/kallsyms and rmmod

[Peter Zijlstra <peterz@infradead.org>]

Adding lkml to Cc so that there is an actual email record of this.

(I could for example not reply to Masami's later entries).

On Mon, Feb 01, 2016 at 12:02:01PM +1030, Rusty Russell wrote:
> > https://bugzilla.kernel.org/show_bug.cgi?id=111541

> Unfortunately, that would also create a DoS where anyone can stop module
> loading.

> diff --git a/kernel/module.c b/kernel/module.c
> index 8358f4697c0c..9f2de09b6d8c 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1992,6 +1992,9 @@ static void free_module(struct module *mod)
>  	mod->state = MODULE_STATE_UNFORMED;
>  	mutex_unlock(&module_mutex);
>  
> +	/* kallsyms walks list without mutex; make sure they see UNFORMED */
> +	synchronize_sched();
> +
>  	/* Remove dynamic debug info */
>  	ddebug_remove_module(mod->name);

So it all depends on when this mod->symtab stuff gets freed, and I could
not actually find this.

If this is with module_memfree(), then the above sync_sched() will not
matter.

Nor will those barriers Masami added do anything. The wmb is between a
->state store and a mutex_unlock. The unlock must already ensure the
unlock store happens after all stores inside its critical section.

The rmb doesn't matter because its a dependent load from the mod list.
That is we _must_ first observe the list entry, otherwise we cannot
dereference it to find the mod->state (except of course if you're Alpha,
but in that case the rcu_dereference in list_for_each_entry_rcu() will
have cured this).