linux-kernel - Re: [PATCH v4 3/3] modversions: treat symbol CRCs as 32 bit quantities on 64 bit archs

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: <CAKv+Gu-wqoQVXvZcEF8xfAnHmrakKU73ToMNO+2Lw158B4teJA@mail.gmail.com>
Date:   Wed, 18 Jan 2017 22:37:13 +0000
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Jessica Yu <jeyu@...hat.com>,
        Rusty Russell <rusty@...tcorp.com.au>,
        "Suzuki K. Poulose" <suzuki.poulose@....com>,
        Will Deacon <will.deacon@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Arnd Bergmann <arnd@...db.de>,
        Al Viro <viro@...iv.linux.org.uk>,
        ppc-dev <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH v4 3/3] modversions: treat symbol CRCs as 32 bit
 quantities on 64 bit archs

On 18 January 2017 at 18:35, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Wed, Jan 18, 2017 at 10:27 AM, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
>>
>> I wonder what happened to gold, and why it didn't take over. I'm
>> assuming it had _other_ bugs.. Oh well.
>
> Google for gold problems, I note that it has been reported to get
> "internal error"s during kernel builds - and at least some of them
> have been due to ksyms.
>
> So the core problem seems to mainly be that gcc normally itself never
> generates any absolute symbols, so the whole ksyms model depends on
> things that get almost zero testing in the toolchain.
>
> Oh well.
>

There is one alternative that gets rid of all the hackiness, but it
does increase the CRC footprint on 32-bit platforms:

diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c
index 06121ce524a7..9f739c7224c3 100644
--- a/scripts/genksyms/genksyms.c
+++ b/scripts/genksyms/genksyms.c
@@ -693,7 +693,10 @@ void export_symbol(const char *name)
                        fputs(">\n", debugfile);

                /* Used as a linker script. */
-               printf("%s__crc_%s = 0x%08lx ;\n", mod_prefix, name, crc);
+               printf("SECTIONS { .rodata.__crc_%s : ALIGN(4) "
+                      "{ %s__crcp_%s = .; LONG(0x%08lx); } }\n"
+                      "%s__crc_%s = 0x%08lx;\n",
+                      name, mod_prefix, name, crc, mod_prefix, name, crc);
        }
 }

diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
index e3508a3d6e53..5e95a552a871 100644
--- a/include/asm-generic/export.h
+++ b/include/asm-generic/export.h
@@ -49,8 +49,8 @@ KSYM(__kstrtab_\name):
        .section ___kcrctab\sec+\name,"a"
        .balign KCRC_ALIGN
 KSYM(__kcrctab_\name):
-       .long KSYM(__crc_\name)
-       .weak KSYM(__crc_\name)
+       .long KSYM(__crcp_\name) - .
+       .weak KSYM(__crcp_\name)
        .previous
 #endif
 #endif

(and an equivalent change in include/linux/export.h)

So the kcrctab contains the relative (signed) offset to where the CRC
is stored, which gets rid of any absolute relocations. To grab the
CRC, we only have to add the value of the kcrctab entry to its
address, and dereference the resulting pointer.

This would remove the need for patches 1 and 2, and get rid of the
overhead of the runtime relocation entries not only for arm64 but for
powerpc as well. It would also get rid of the abuse of ELF symbols
once and for all, hopefully avoiding bugs in GNU ld and gold in the
future.

For a ballpark number of 10,000 CRCs in the core kernel, this would
increase the size of the image by 40 KB for 32-bit architectures (and
if saving 40 KB is essential, chances are you won't be using
modversions in the first place). For 64-bit architectures, there would
be no change in size, of course, except for saving 24 bytes of __init
space *per CRC* for arm64 and powerpc64 with CONFIG_RELOCATABLE=y

I will send out a separate RFC so we can discuss this alternative