linux-kernel - Re: [PATCH 1/2] module: fix symbol waiting when module fails before init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMOw1v4LydKrkxCMNqX3QJex9KEUBHaEv-QF0yfwpM4WjEWx8w@mail.gmail.com>
Date:	Fri, 14 Sep 2012 13:41:18 -0300
From:	Lucas De Marchi <lucas.demarchi@...fusion.mobi>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Jon Masters <jonathan@...masters.org>
Subject: Re: [PATCH 1/2] module: fix symbol waiting when module fails before init

On Fri, Sep 14, 2012 at 4:09 AM, Rusty Russell <rusty@...tcorp.com.au> wrote:
> We use resolve_symbol_wait(), which blocks if the module containing
> the symbol is still loading.  However:
>
> 1) The module_wq we use is only woken after calling the modules' init
>    function, but there are other failure paths after the module is
>    placed in the linked list where we need to do the same thing.
>
> 2) wake_up() only wakes one waiter, and our waitqueue is shared by all
>    modules, so we need to wake them all.
>
> 3) wake_up_all() doesn't imply a memory barrier: I feel happier calling
>    it after we've grabbed and dropped the module_mutex, not just after
>    the state assignment.
>
> Signed-off-by: Rusty Russell <rusty@...tcorp.com.au>
> ---
>  kernel/module.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2959,7 +2959,7 @@ static struct module *load_module(void _
>         /* Unlink carefully: kallsyms could be walking list. */
>         list_del_rcu(&mod->list);
>         module_bug_cleanup(mod);
> -
> +       wake_up_all(&module_wq);
>   ddebug:
>         dynamic_debug_remove(info.debug);
>   unlock:
> @@ -3034,7 +3034,7 @@ SYSCALL_DEFINE3(init_module, void __user
>                 blocking_notifier_call_chain(&module_notify_list,
>                                              MODULE_STATE_GOING, mod);
>                 free_module(mod);
> -               wake_up(&module_wq);
> +               wake_up_all(&module_wq);
>                 return ret;
>         }
>         if (ret > 0) {
> @@ -3046,9 +3046,8 @@ SYSCALL_DEFINE3(init_module, void __user
>                 dump_stack();
>         }
>
> -       /* Now it's a first class citizen!  Wake up anyone waiting for it. */
> +       /* Now it's a first class citizen! */
>         mod->state = MODULE_STATE_LIVE;
> -       wake_up(&module_wq);
>         blocking_notifier_call_chain(&module_notify_list,
>                                      MODULE_STATE_LIVE, mod);
>
> @@ -3071,6 +3070,7 @@ SYSCALL_DEFINE3(init_module, void __user
>         mod->init_ro_size = 0;
>         mod->init_text_size = 0;
>         mutex_unlock(&module_mutex);
> +       wake_up_all(&module_wq);
>
>         return 0;
>  }

Ack.

cheers,
Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/