On Sun, Aug 27, 2017 at 4:52 PM, Stefan Agner <ste...@agner.ch> wrote: > 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 <ste...@agner.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 <m...@chromium.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)
Perhaps the reverse is the better solution? Leave "const" in MODULE_DEVICE_TABLE and remove the redundant usage. This means new cases of missing the const will never happen (which was the intent originally of putting const into the MODULE_DEVICE_TABLE macro, I assume). -Kees -- Kees Cook Pixel Security
