All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masahiro Yamada <yamada.masahiro@socionext.com>
To: linux-kbuild@vger.kernel.org
Cc: Arnd Bergmann <arnd@arndb.de>, Denis Efremov <efremov@linux.com>,
	Nicolas Pitre <nico@fluxnic.net>,
	Masahiro Yamada <yamada.masahiro@socionext.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/2] export.h, genksyms: do not make genksyms calculate CRC of trimmed symbols
Date: Mon,  9 Sep 2019 19:53:17 +0900	[thread overview]
Message-ID: <20190909105317.20473-2-yamada.masahiro@socionext.com> (raw)
In-Reply-To: <20190909105317.20473-1-yamada.masahiro@socionext.com>

Arnd Bergmann reported false-positive modpost warnings detected by
his randconfig testing of linux-next:

  https://lkml.org/lkml/2019/9/6/619

Actually, this happens under the combination of CONFIG_MODVERSIONS=y
and CONFIG_TRIM_UNUSED_KSYMS=y since commit 15bfc2348d54 ("modpost:
check for static EXPORT_SYMBOL* functions").

For example, arch/arm/config/multi_v7_defconfig + CONFIG_MODVERSIONS=y
+ CONFIG_TRIM_UNUSED_KSYMS=y produces the following false-positives:

WARNING: "__lshrdi3" [vmlinux] is a static (unknown)
WARNING: "__ashrdi3" [vmlinux] is a static (unknown)
WARNING: "__aeabi_lasr" [vmlinux] is a static (unknown)
WARNING: "__aeabi_llsr" [vmlinux] is a static (unknown)
WARNING: "ftrace_set_clr_event" [vmlinux] is a static (unknown)
WARNING: "__muldi3" [vmlinux] is a static (unknown)
WARNING: "__aeabi_ulcmp" [vmlinux] is a static (unknown)
WARNING: "__ucmpdi2" [vmlinux] is a static (unknown)
WARNING: "__aeabi_lmul" [vmlinux] is a static (unknown)
WARNING: "__bswapsi2" [vmlinux] is a static (unknown)
WARNING: "__bswapdi2" [vmlinux] is a static (unknown)
WARNING: "__ashldi3" [vmlinux] is a static (unknown)
WARNING: "__aeabi_llsl" [vmlinux] is a static (unknown)

The root cause of the problem is not in the modpost, but in the
implementation of CONFIG_TRIM_UNUSED_KSYMS.

If there is at least one untrimmed symbol in the file, genksyms is
invoked to calculate CRC of *all* the symbols in that file even if
some of them have been trimmed due to no caller existing.

As a result, .tmp_*.ver files contain CRC of trimmed symbols, thus
unneeded __crc* symbols are added to objects. It has been harmless
until recently.

Since commit 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL*
functions"), it is harmful because the bogus __crc* symbols make
modpost call sym_update_crc(), and then new_symbol(), but there is
no one that clears the ->is_static member.

I gave Fixes to the first commit that uncovered the issue, but the
potential problem has long existed since commit f235541699bc
("export.h: allow for per-symbol configurable EXPORT_SYMBOL()").

Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 include/linux/export.h      | 42 ++++++++++++++-----------------------
 scripts/genksyms/keywords.c |  6 +-----
 2 files changed, 17 insertions(+), 31 deletions(-)

diff --git a/include/linux/export.h b/include/linux/export.h
index cdd98a0d918c..7d8c112a8b61 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -18,9 +18,6 @@ extern struct module __this_module;
 #define THIS_MODULE ((struct module *)0)
 #endif
 
-#ifdef CONFIG_MODULES
-
-#if !defined(__GENKSYMS__)
 #ifdef CONFIG_MODVERSIONS
 /* Mark the CRC weak since genksyms apparently decides not to
  * generate a checksums for some symbols */
@@ -74,6 +71,12 @@ struct kernel_symbol {
 };
 #endif
 
+#ifdef __GENKSYMS__
+
+#define ___EXPORT_SYMBOL(sym, sec)	__GENKSYMS_EXPORT_SYMBOL(sym)
+
+#else
+
 /* For every exported symbol, place a struct in the __ksymtab section */
 #define ___EXPORT_SYMBOL(sym, sec)					\
 	extern typeof(sym) sym;						\
@@ -83,7 +86,9 @@ struct kernel_symbol {
 	= #sym;								\
 	__KSYMTAB_ENTRY(sym, sec)
 
-#if defined(__DISABLE_EXPORTS)
+#endif
+
+#if !defined(CONFIG_MODULES) || defined(__DISABLE_EXPORTS)
 
 /*
  * Allow symbol exports to be disabled completely so that C code may
@@ -117,37 +122,22 @@ struct kernel_symbol {
 #define __cond_export_sym_0(sym, sec) /* nothing */
 
 #else
-#define __EXPORT_SYMBOL ___EXPORT_SYMBOL
-#endif
 
-#define EXPORT_SYMBOL(sym)					\
-	__EXPORT_SYMBOL(sym, "")
+#define __EXPORT_SYMBOL(sym, sec)	___EXPORT_SYMBOL(sym, sec)
 
-#define EXPORT_SYMBOL_GPL(sym)					\
-	__EXPORT_SYMBOL(sym, "_gpl")
-
-#define EXPORT_SYMBOL_GPL_FUTURE(sym)				\
-	__EXPORT_SYMBOL(sym, "_gpl_future")
+#endif /* CONFIG_MODULES */
 
+#define EXPORT_SYMBOL(sym)		__EXPORT_SYMBOL(sym, "")
+#define EXPORT_SYMBOL_GPL(sym)		__EXPORT_SYMBOL(sym, "_gpl")
+#define EXPORT_SYMBOL_GPL_FUTURE(sym)	__EXPORT_SYMBOL(sym, "_gpl_future")
 #ifdef CONFIG_UNUSED_SYMBOLS
-#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused")
-#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl")
+#define EXPORT_UNUSED_SYMBOL(sym)	__EXPORT_SYMBOL(sym, "_unused")
+#define EXPORT_UNUSED_SYMBOL_GPL(sym)	__EXPORT_SYMBOL(sym, "_unused_gpl")
 #else
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 #endif
 
-#endif	/* __GENKSYMS__ */
-
-#else /* !CONFIG_MODULES... */
-
-#define EXPORT_SYMBOL(sym)
-#define EXPORT_SYMBOL_GPL(sym)
-#define EXPORT_SYMBOL_GPL_FUTURE(sym)
-#define EXPORT_UNUSED_SYMBOL(sym)
-#define EXPORT_UNUSED_SYMBOL_GPL(sym)
-
-#endif /* CONFIG_MODULES */
 #endif /* !__ASSEMBLY__ */
 
 #endif /* _LINUX_EXPORT_H */
diff --git a/scripts/genksyms/keywords.c b/scripts/genksyms/keywords.c
index c586d32dd2c3..7a85c4e21175 100644
--- a/scripts/genksyms/keywords.c
+++ b/scripts/genksyms/keywords.c
@@ -3,11 +3,7 @@ static struct resword {
 	const char *name;
 	int token;
 } keywords[] = {
-	{ "EXPORT_SYMBOL", EXPORT_SYMBOL_KEYW },
-	{ "EXPORT_SYMBOL_GPL", EXPORT_SYMBOL_KEYW },
-	{ "EXPORT_SYMBOL_GPL_FUTURE", EXPORT_SYMBOL_KEYW },
-	{ "EXPORT_UNUSED_SYMBOL", EXPORT_SYMBOL_KEYW },
-	{ "EXPORT_UNUSED_SYMBOL_GPL", EXPORT_SYMBOL_KEYW },
+	{ "__GENKSYMS_EXPORT_SYMBOL", EXPORT_SYMBOL_KEYW },
 	{ "__asm", ASM_KEYW },
 	{ "__asm__", ASM_KEYW },
 	{ "__attribute", ATTRIBUTE_KEYW },
-- 
2.17.1


  reply	other threads:[~2019-09-09 10:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-09 10:53 [PATCH 1/2] export.h: remove defined(__KERNEL__) Masahiro Yamada
2019-09-09 10:53 ` Masahiro Yamada [this message]
2019-09-09 13:43   ` [PATCH 2/2] export.h, genksyms: do not make genksyms calculate CRC of trimmed symbols Arnd Bergmann
2019-09-09 13:48 ` [PATCH 1/2] export.h: remove defined(__KERNEL__) Nicolas Pitre
2019-09-09 14:34   ` Masahiro Yamada
2019-09-09 16:06     ` Nicolas Pitre
2019-09-10  2:10       ` Masahiro Yamada
2019-09-10  2:21         ` Nicolas Pitre
2019-09-12 11:48 ` Masahiro Yamada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190909105317.20473-2-yamada.masahiro@socionext.com \
    --to=yamada.masahiro@socionext.com \
    --cc=arnd@arndb.de \
    --cc=efremov@linux.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nico@fluxnic.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.