linux-kernel - Re: [RFC PATCH v2] modversions: redefine kcrctab entries as relative CRC pointers

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] [day] [month] [year] [list]

Message-ID: <CAKv+Gu8D=PWbm5YcoBLER918qquo1usuwprCvDPSgM=H5y7AeA@mail.gmail.com>
Date:   Thu, 19 Jan 2017 17:00:56 +0000
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc:     linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        Rusty Russell <rusty@...tcorp.com.au>,
        Jessica Yu <jeyu@...hat.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>
Subject: Re: [RFC PATCH v2] modversions: redefine kcrctab entries as relative
 CRC pointers

On 19 January 2017 at 16:55, Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote:
> On 19 January 2017 at 12:02, Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote:
>> The modversion symbol CRCs are emitted as ELF symbols, which allows us to
>> easily populate the kcrctab sections by relying on the linker to associate
>> each kcrctab slot with the correct value.
>>
>> This has a couple of downsides:
>> - On architectures that support runtime relocation, a R_<arch>_RELATIVE
>>   relocation entry is emitted for each CRC value, which identifies it as
>>   a quantity that requires fixing up based on the actual runtime load
>>   offset of the kernel. This results in corrupted CRCs unless we
>>   explicitly undo the fixup (and this is currently being handled in the
>>   core module code),
>> - On 64 bit architectures, such runtime relocation entries take up 24
>>   bytes of __init space each, resulting in a x8 overhead in [uncompressed]
>>   kernel size for CRCs,
>> - The use of ELF symbols to represent things other than virtual addresses
>>   is poorly supported (and mostly untested) in GNU binutils.
>>
>> So avoid these issues by redefining the kcrctab entries as signed relative
>> offsets pointing to the actual CRC value stored elsewhere in the kernel
>> image (or in the module). This removes all the ELF hackery involving
>> symbols and relocations, at the expense of 4 additional bytes of space per
>> CRC on 32-bit architectures. (On 64-bit architectures, each 8 byte CRC
>> symbol reference is replaced by a 4 byte relative offset and the 4 byte
>> CRC value elsewhere in the image)
>>
>> Note that this mostly reverts commit d4703aefdbc8 ("module: handle ppc64
>> relocating kcrctabs when CONFIG_RELOCATABLE=y")
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>
>> ---
>> v2: update modpost as well, so that genksyms no longer has to emit symbols
>>     for both the actual CRC value and the reference to where it is stored
>>     in the image
>>
>
> Annoyingly, the following is required on top of this patch to ensure
> that we only subtract the section VMA from the symbol value in modpost
> if we are dealing with a fully linked file. This is necessary since,
> as it turns out, partially linked ELF objects may have non-zero
> section VMAs (for whatever reason).
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index d69f3ceae19e..75b0dac1cc11 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -624,7 +624,8 @@
>                 if (sym->st_shndx != SHN_UNDEF)
>                         crcp = (void *)info->hdr + sym->st_value +
>                                info->sechdrs[sym->st_shndx].sh_offset -
> -                              info->sechdrs[sym->st_shndx].sh_addr;
> +                              (hdr->e_type != ET_REL ?

This should be info->hdr->e_type ^^^. Apologies.

> +                               info->sechdrs[sym->st_shndx].sh_addr : 0);
>
>                 is_crc = true;
>                 crc = crcp ? *crcp : 0;
>