linux-kernel - Re: module: Remove const attribute from alias for MODULE_DEVICE

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: <9a909feb016e0c38d42d7c1fc0ce7dd0@agner.ch>
Date:   Sun, 27 Aug 2017 16:52:28 -0700
From:   Stefan Agner <stefan@...er.ch>
To:     Jessica Yu <jeyu@...nel.org>, Matthias Kaehlcke <mka@...omium.org>
Cc:     Rusty Russell <rusty@...tcorp.com.au>,
        linux-kernel@...r.kernel.org,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Michal Marek <mmarek@...e.com>,
        Kees Cook <keescook@...omium.org>,
        Arnd Bergmann <arnd@...db.de>,
        Doug Anderson <dianders@...omium.org>,
        Grant Grundler <grundler@...omium.org>,
        Greg Hackmann <ghackmann@...gle.com>,
        Michael Davidson <md@...gle.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Alexander Potapenko <glider@...gle.com>,
        Bernhard.Rosenkranzer@...aro.org
Subject: Re: module: Remove const attribute from alias for MODULE_DEVICE_TABLE

On 2017-07-24 18:27, Matthias Kaehlcke wrote:
> MODULE_DEVICE_TABLE(type, name) creates an alias of type 'extern const
> typeof(name)'. If 'name' is already constant the 'const' attribute is
> specified twice, which is not allowed in C89 (see discussion at
> https://lkml.org/lkml/2017/5/23/1440). Since the kernel is built with
> -std=gnu89 clang generates warnings like this:
> 
> drivers/thermal/x86_pkg_temp_thermal.c:509:1: warning: duplicate 'const'
>   declaration specifier
>       [-Wduplicate-decl-specifier]
> MODULE_DEVICE_TABLE(x86cpu, pkg_temp_thermal_ids);
> ^
> ./include/linux/module.h:212:8: note: expanded from macro 'MODULE_DEVICE_TABLE'
> extern const typeof(name) __mod_##type##__##name##_device_table
> 
> Remove the const attribute from the alias to avoid the duplicate
> specifier. After all it is only an alias and the attribute shouldn't
> have any effect.

Unfortunately, it has effect where const is missing in the original
variable declaration:

Before this patch:
13:10 $ size drivers/net/wireless/ath/ath6kl/ath6kl_usb.ko
   text    data     bss     dec     hex filename
   8825     728      40    9593    2579
drivers/net/wireless/ath/ath6kl/ath6kl_usb.ko

After this patch:
13:12 $ size drivers/net/wireless/ath/ath6kl/ath6kl_usb.ko
   text    data     bss     dec     hex filename
   8747     800      40    9587    2573
drivers/net/wireless/ath/ath6kl/ath6kl_usb.ko


Ideally we would fix all MODULE_DEVICE_TABLE usage sites. This would
also made it clearly visible that the device tables are const.

I created a semantic patch, it turns out that 620 sites are affected
(out of 4499)...

//
// Cocinelle Semantic Patch to constify module device tables
//
// Author: Stefan Agner <stefan@...er.ch>
//
@ module_device_table @
declarer name MODULE_DEVICE_TABLE;
identifier moduletype;
identifier name;
@@
MODULE_DEVICE_TABLE(moduletype, name);

@ add_const depends on module_device_table disable optional_qualifier @
identifier module_device_table.name;
type T;
@@
+const 
 T name[] = { 
 ...
 };

Thoughts?

--
Stefan


> 
> Signed-off-by: Matthias Kaehlcke <mka@...omium.org>
> ---
>  include/linux/module.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index e7bdd549e527..fe5aa3736707 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -209,7 +209,7 @@ extern void cleanup_module(void);
>  #ifdef MODULE
>  /* Creates an alias so file2alias.c can find device table. */
>  #define MODULE_DEVICE_TABLE(type, name)					\
> -extern const typeof(name) __mod_##type##__##name##_device_table		\
> +extern typeof(name) __mod_##type##__##name##_device_table		\
>    __attribute__ ((unused, alias(__stringify(name))))
>  #else  /* !MODULE */
>  #define MODULE_DEVICE_TABLE(type, name)