linux-kernel - Re: [PATCH v2] include/linux/module.h: mark init/cleanup_module aliases as _

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]

Date:   Thu, 7 Feb 2019 11:54:31 +0100
From:   Jessica Yu <jeyu@...nel.org>
To:     Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc:     Laura Abbott <labbott@...hat.com>,
        Martin Sebor <msebor@....gnu.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] include/linux/module.h: mark init/cleanup_module
 aliases as __init/exit

+++ Miguel Ojeda [06/02/19 22:19 +0100]:
>On Wed, Feb 6, 2019 at 6:56 PM Miguel Ojeda
><miguel.ojeda.sandonis@...il.com> wrote:
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index 8fa38d3e7538..1b5e370f1bc0 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -129,13 +129,13 @@ extern void cleanup_module(void);
>>  #define module_init(initfn)                                    \
>>         static inline initcall_t __maybe_unused __inittest(void)                \
>>         { return initfn; }                                      \
>> -       int init_module(void) __attribute__((alias(#initfn)));
>> +       int init_module(void) __init __attribute__((alias(#initfn)));
>>
>>  /* This is only required if you want to be unloadable. */
>>  #define module_exit(exitfn)                                    \
>>         static inline exitcall_t __maybe_unused __exittest(void)                \
>>         { return exitfn; }                                      \
>> -       void cleanup_module(void) __attribute__((alias(#exitfn)));
>> +       void cleanup_module(void) __exit __attribute__((alias(#exitfn)));
>
>It turns out that there are some modules without __init/__exit marked
>functions, which GCC complains about, since now the alias is in a
>different section than the target:
>
>  * In some cases, this is due to a missing __init/__exit marking
>(e.g. drivers/connector/connector.c). These should be fixed in any
>case.
>  * In other cases, the cleanup function is not marked as such because
>it is called from another place in the TU, like the init function
>(e.g. arch/x86/kvm/vmx.c). We would need to create an actual cleanup
>function (marked as __exit) that simply calls the current exit static
>function.

Grr, I guess it was not so simple after all. :)

>So we have a few alternatives:
>
>  1) Going only with __cold.
>  2) Using the new __copy attribute (because then they are copied only
>in the cases they are actually used).
>  3) Fix and go for __init/__exit. While this requires some tweaking
>as explained above, it would be good if we can achieve it since then
>we are enforcing proper __init/__exit markings for all modules
>(whereas __copy wouldn't spot).
>
>I think it is worth achieving 3), but that will take a bit more of
>time. In that case, I suggest we push 1) or 2) for the moment (so that
>the warning is fixed) and then work on fixing all instances. As soon
>as that is done, we can push 3).

My order of preference would be 2, 1, striving for 3 eventually. 

Thanks Miguel!

Jessica