Re: [PATCH v2] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols

From: Jessica Yu
Date: Fri Jan 15 2021 - 03:15:47 EST


+++ Marco Elver [15/01/21 08:03 +0100]:
On Thu, 14 Jan 2021 at 22:54, Fangrui Song <maskray@xxxxxxxxxx> wrote:
clang-12 -fno-pic (since
https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
can emit `call __stack_chk_fail@PLT` instead of `call __stack_chk_fail`
on x86. The two forms should have identical behaviors on x86-64 but the
former causes GNU as<2.37 to produce an unreferenced undefined symbol
_GLOBAL_OFFSET_TABLE_.

(On x86-32, there is an R_386_PC32 vs R_386_PLT32 difference but the
linker behavior is identical as far as Linux kernel is concerned.)

Simply ignore _GLOBAL_OFFSET_TABLE_ for now, like what
scripts/mod/modpost.c:ignore_undef_symbol does. This also fixes the
problem for gcc/clang -fpie and -fpic, which may emit `call foo@PLT` for
external function calls on x86.

Note: ld -z defs and dynamic loaders do not error for unreferenced
undefined symbols so the module loader is reading too much. If we ever
need to ignore more symbols, the code should be refactored to ignore
unreferenced symbols.

Reported-by: Marco Elver <elver@xxxxxxxxxx>
Link: https://github.com/ClangBuiltLinux/linux/issues/1250
Signed-off-by: Fangrui Song <maskray@xxxxxxxxxx>

Tested-by: Marco Elver <elver@xxxxxxxxxx>

Thank you for the patch!

---
kernel/module.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
---
Changes in v2:
* Fix Marco's email address
* Add a function ignore_undef_symbol similar to scripts/mod/modpost.c:ignore_undef_symbol

diff --git a/kernel/module.c b/kernel/module.c
index 4bf30e4b3eaa..278f5129bde2 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2348,6 +2348,20 @@ static int verify_exported_symbols(struct module *mod)
return 0;
}

+static int ignore_undef_symbol(Elf_Half emachine, const char *name)

Why not 'bool' return-type?

+{
+ /* On x86, PIC code and Clang non-PIC code may have call foo@PLT. GNU as

Not sure if checkpatch.pl warns about this, but this multi-line
comment does not follow the normal kernel-style (see elsewhere in
file):

/*
* ...
*/

+1 to Marco's comments. Otherwise, patch looks good to me.

Thanks Fangrui!

Jessica