linux-kernel - Re: [PATCH 3/3] include/linux/module.h: copy __init/__exit attrs to init/cleanup

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: <20190211150108.GB20732@linux-8ccs>
Date:   Mon, 11 Feb 2019 16:01:09 +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>,
        Arnd Bergmann <arnd@...db.de>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        James Morris <james.morris@...rosoft.com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Borislav Petkov <bp@...e.de>,
        Kees Cook <keescook@...omium.org>,
        Matt Mullins <mmullins@...com>,
        Vincent Whitchurch <vincent.whitchurch@...s.com>,
        WANG Chao <chao.wang@...oud.cn>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] include/linux/module.h: copy __init/__exit attrs to
 init/cleanup_module

+++ Miguel Ojeda [09/02/19 01:08 +0100]:
>The upcoming GCC 9 release extends the -Wmissing-attributes warnings
>(enabled by -Wall) to C and aliases: it warns when particular function
>attributes are missing in the aliases but not in their target.
>
>In particular, it triggers for all the init/cleanup_module
>aliases in the kernel (defined by the module_init/exit macros),
>ending up being very noisy.
>
>These aliases point to the __init/__exit functions of a module,
>which are defined as __cold (among other attributes). However,
>the aliases themselves do not have the __cold attribute.
>
>Since the compiler behaves differently when compiling a __cold
>function as well as when compiling paths leading to calls
>to __cold functions, the warning is trying to point out
>the possibly-forgotten attribute in the alias.
>
>In order to keep the warning enabled, we decided to silence
>this case. Ideally, we would mark the aliases directly
>as __init/__exit. However, there are currently around 132 modules
>in the kernel which are missing __init/__exit in their init/cleanup
>functions (either because they are missing, or for other reasons,
>e.g. the functions being called from somewhere else); and
>a section mismatch is a hard error.
>
>A conservative alternative was to mark the aliases as __cold only.
>However, since we would like to eventually enforce __init/__exit
>to be always marked,  we chose to use the new __copy function
>attribute (introduced by GCC 9 as well to deal with this).
>With it, we copy the attributes used by the target functions
>into the aliases. This way, functions that were not marked
>as __init/__exit won't have their aliases marked either,
>and therefore there won't be a section mismatch.
>
>Note that the warning would go away marking either the extern
>declaration, the definition, or both. However, we only mark
>the definition of the alias, since we do not want callers
>(which only see the declaration) to be compiled as if the function
>was __cold (and therefore the paths leading to those calls
>would be assumed to be unlikely).
>
>Link: https://lore.kernel.org/lkml/20190123173707.GA16603@gmail.com/
>Link: https://lore.kernel.org/lkml/20190206175627.GA20399@gmail.com/
>Suggested-by: Martin Sebor <msebor@....gnu.org>
>Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>

Acked-by: Jessica Yu <jeyu@...nel.org>

Thanks!

>---
> include/linux/module.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/include/linux/module.h b/include/linux/module.h
>index 8fa38d3e7538..f5bc4c046461 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) __copy(initfn) __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) __copy(exitfn) __attribute__((alias(#exitfn)));
>
> #endif
>
>-- 
>2.17.1
>