linux-kernel - [PATCH v2 2/2] modpost: skip ELF local symbols by default during section mismatch check

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: <20181115005602.30746-3-paul.walmsley@sifive.com>
Date:   Wed, 14 Nov 2018 16:56:02 -0800
From:   Paul Walmsley <paul.walmsley@...ive.com>
To:     linux-kernel@...r.kernel.org
Cc:     Paul Walmsley <paul.walmsley@...ive.com>,
        Russell King <linux@...linux.org.uk>,
        Jim Wilson <jimw@...ive.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Michal Marek <michal.lkml@...kovi.net>,
        Sam Ravnborg <sam@...nborg.org>, linux-kbuild@...r.kernel.org,
        Paul Walmsley <paul@...an.com>
Subject: [PATCH v2 2/2] modpost: skip ELF local symbols by default during section mismatch check

During development of a serial console driver with a gcc 8.2.0
toolchain for RISC-V, the following modpost warning appeared:

----
WARNING: vmlinux.o(.data+0x19b10): Section mismatch in reference from the variable .LANCHOR1 to the function .init.text:sifive_serial_console_setup()
The variable .LANCHOR1 references
the function __init sifive_serial_console_setup()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
----

".LANCHOR1" is an ELF local symbol, automatically created by gcc's section
anchor generation code:

https://gcc.gnu.org/onlinedocs/gccint/Anchored-Addresses.html

https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/varasm.c;h=cd9591a45617464946dcf9a126dde277d9de9804;hb=9fb89fa845c1b2e0a18d85ada0b077c84508ab78#l7473

This was verified by compiling the kernel with -fno-section-anchors.
The serial driver code idiom triggering the warning is standard serial
driver practice, and one that has a specific whitelist inclusion in
modpost.c.

I'm neither a modpost nor an ELF expert, but naively, it doesn't seem
useful for modpost to report section mismatch warnings caused by ELF
local symbols by default.  Local symbols have compiler-generated
names, and thus bypass modpost's whitelisting algorithm, which relies
on the presence of a non-autogenerated symbol name.  This increases
the likelihood that false positive warnings will be generated (as in
the above case).

Thus, disable section mismatch reporting on ELF local symbols.  The
rationale here is similar to that of commit 2e3a10a1551d ("ARM: avoid
ARM binutils leaking ELF local symbols") and of similar code already
present in modpost.c:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/mod/modpost.c?h=v4.19-rc4&id=7876320f88802b22d4e2daf7eb027dd14175a0f8#n1256

This second version of the patch drops the option to keep section
mismatch warnings for local sections, based on feedback from Sam
Ravnborg <sam@...nborg.org>; and clarifies that these warnings
appear with gcc 8.2.0.

Cc: Russell King <linux@...linux.org.uk>
Cc: Jim Wilson <jimw@...ive.com>
Cc: Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc: Michal Marek <michal.lkml@...kovi.net>
Cc: Sam Ravnborg <sam@...nborg.org>
Cc: linux-kbuild@...r.kernel.org
Cc: linux-kernel@...r.kernel.org
Signed-off-by: Paul Walmsley <paul.walmsley@...ive.com>
Signed-off-by: Paul Walmsley <paul@...an.com>
---
 scripts/mod/modpost.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 85bd93c63180..0fb148171b78 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1253,6 +1253,17 @@ static inline int is_arm_mapping_symbol(const char *str)
 	       && (str[2] == '\0' || str[2] == '.');
 }
 
+/*
+ * If a symbol name follows the convention for ELF-local symbols (i.e., the
+ * name begins with a ".L"), return true; otherwise false.  This is used to
+ * skip section mismatch reporting on ELF-local symbols, due to the risk of
+ * false positives.
+ */
+static inline int is_local_symbol(const char *str)
+{
+	return str[0] == '.' && str[1] == 'L';
+}
+
 /*
  * If there's no name there, ignore it; likewise, ignore it if it's
  * one of the magic symbols emitted used by current ARM tools.
@@ -1535,6 +1546,9 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 	if (strstarts(fromsym, "reference___initcall"))
 		return;
 
+	if (is_local_symbol(fromsym))
+		return;
+
 	tosec = sec_name(elf, get_secindex(elf, sym));
 	to = find_elf_symbol(elf, r->r_addend, sym);
 	tosym = sym_name(elf, to);
-- 
2.19.1