lld discards sections that start with ".gnu.linkonce." preventing kernel modules from being loaded

[nickdesaulniers]

Filing a copy of this internal bug I stumbled upon yesterday at work, so that it's visible for others looking to use LLD:

Hmmm...looks like there's a problem loading kernel modules.

redacted:/data/local/tmp # insmod redacted.ko                                  
insmod: failed to load redacted.ko: Exec format error
1|redacted:/data/local/tmp # dmesg | tail -n 1                                           
[  265.108479] redacted: No module found in object

I'll probably need to dig into how the kernel load modules to see what's going wrong.

kernel/module.c:

             info->index.mod = find_sec(info, ".gnu.linkonce.this_module");          
             if (!info->index.mod) {                                                 
                     pr_warn("%s: No module found in object\n",                      
                             info->name ?: "(missing .modinfo name field)");

__this_module is a D (public data) symbol with bfd but U (undefined reference) with lld.
.gnu.linkonce.this_module is listed as a section with readelf with bfd, but is missing with lld.

readelf bfd:
    47: 0000000000000000   896 OBJECT  GLOBAL DEFAULT   21 __this_module
readelf lld:
    57: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  UND __this_module

Now to see if lld is accidentally or intentionally omitting this data/symbol/section, or if the linker script is malformed.

https://reviews.llvm.org/D28430 looks like it might be problematic. Looks like it purposely discards any section name that starts with ".gnu.linkonce.". Seems like that code is still in lld.

May I present, my most significant contribution to mankind to date:

diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 9a6c3487c173..c8f05f297d5b 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -735,7 +735,7 @@ InputSectionBase *ObjFile<ELFT>::createInputSection(const Elf_Shdr &Sec) {
   // sections. Drop those sections to avoid duplicate symbol errors.
   // FIXME: This is glibc PR20543, we should remove this hack once that has been
   // fixed for a while.
-  if (Name.startswith(".gnu.linkonce."))
+  if (Name.startswith(".gnu.linkonce.t."))
     return &InputSection::Discarded;
 
   // If we are creating a new .build-id section, strip existing .build-id

https://sourceware.org/bugzilla/show_bug.cgi?id=20543

Will post patch to phabricator upstream Monday, assuming I can figure out arcanist+the new github monorepo (and remember my svn password, if those are still in use).

[nickdesaulniers]

https://reviews.llvm.org/D57294

[nickdesaulniers]

Fixed: https://reviews.llvm.org/rL352302

Need to request it gets cherry picked into the lld-8 release.

[nickdesaulniers]

https://llvm.org/pr40485

[shenki]

The Linux kernel should use a comdat section instead of .gnu.linkonce.
The minimum version of binutils supported by the kernel supports comdat
sections. The kernel is also not relying on the old linkonce behavior;
it seems to have chosen a name that contains a deprecated GNU feature.

Have we brought this up with the kernel maintainers?

We can fix the module section name issue with something like this:

diff --git a/kernel/module.c b/kernel/module.c
index 2ad1b5239910..5cef8e0bbc75 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2978,7 +2978,7 @@ static int setup_load_info(struct load_info *info, int flags)
                return -ENOEXEC;
        }
 
-       info->index.mod = find_sec(info, ".gnu.linkonce.this_module");
+       info->index.mod = find_sec(info, ".data.this_module");
        if (!info->index.mod) {
                pr_warn("%s: No module found in object\n",
                        info->name ?: "(missing .modinfo name field)");
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 26bf886bd168..8964eb8e6d0a 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2164,7 +2164,7 @@ static void add_header(struct buffer *b, struct module *mod)
        buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
        buf_printf(b, "\n");
        buf_printf(b, "__visible struct module __this_module\n");
-       buf_printf(b, "__attribute__((section(\".gnu.linkonce.this_module\"))) = {\n");
+       buf_printf(b, "__attribute__((section(\".data.this_module\"))) = {\n");
        buf_printf(b, "\t.name = KBUILD_MODNAME,\n");
        if (mod->has_init)
                buf_printf(b, "\t.init = init_module,\n");

I happen to share an office with the author of the module loader and they said: this is the reason the module loader is in the kernel, so we can make arbitrary changes and not worry about an external tool depending on them. You're right that it was just an arbitrary string, however it does follow the semantics of linkonce (each module needs one of these descriptions, but only one of them).

I did some more digging and I wasn't sure where you read that the linkonce section was deprecated?

Secondly, a grep for .gnu.linkonce shows that most arches use this in their vdso linker scripts, so the vdso might have been broken with clang in addition to modules.

$ git grep -l gnu.linkonce
arch/arc/kernel/vmlinux.lds.S
arch/arm/vdso/vdso.lds.S
arch/arm64/kernel/vdso/vdso.lds.S
arch/c6x/kernel/vmlinux.lds.S
arch/ia64/hp/sim/boot/bootloader.lds
arch/ia64/kernel/gate.lds.S
arch/ia64/kernel/vmlinux.lds.S
arch/mips/vdso/vdso.lds.S
arch/nds32/kernel/vdso/vdso.lds.S
arch/powerpc/kernel/vdso32/vdso32.lds.S
arch/powerpc/kernel/vdso64/vdso64.lds.S
arch/riscv/kernel/vdso/vdso.lds.S
arch/s390/kernel/vdso32/vdso32.lds.S
arch/s390/kernel/vdso64/vdso64.lds.S
arch/sh/kernel/vsyscall/vsyscall.lds.S
arch/sparc/vdso/vdso-layout.lds.S
arch/um/kernel/dyn.lds.S
arch/um/kernel/uml.lds.S
arch/x86/entry/vdso/vdso-layout.lds.S
arch/x86/um/vdso/vdso-layout.lds.S
arch/xtensa/kernel/vmlinux.lds.S
include/asm-generic/vmlinux.lds.h
kernel/module.c
scripts/mod/modpost.c
tools/perf/util/probe-event.c

[nickdesaulniers]

I did some more digging and I wasn't sure where you read that the linkonce section was deprecated?

Most of that wording is from pcc, who does a lot of work on LLD. I start an email thread with all of us.

[nickdesaulniers]

Secondly, a grep for .gnu.linkonce shows that most arches use this in their vdso linker scripts, so the vdso might have been broken with clang in addition to modules.

Oh! Thank you for pointing this out. How can you check from userspace that this exists correctly? Doesn't it show up under /proc//maps IIRC?

[nickdesaulniers]

Fixed in lld-9, but the patch was cherry picked to the lld-8 release branch (so will be available in lld-8).