linux-kernel - Re: [PATCH v3 5/7] x86/boot: Remove run-time relocations from .head.text code

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: <202006290859.14DD408A29@keescook>
Date:   Mon, 29 Jun 2020 09:04:04 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Arvind Sankar <nivedita@...m.mit.edu>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Fangrui Song <maskray@...gle.com>,
        Dmitry Golovin <dima@...ovin.in>,
        clang-built-linux@...glegroups.com,
        Ard Biesheuvel <ardb@...nel.org>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Daniel Kiper <daniel.kiper@...cle.com>,
        Sedat Dilek <sedat.dilek@...il.com>,
        Nathan Chancellor <natechancellor@...il.com>,
        Arnd Bergmann <arnd@...db.de>,
        "H . J . Lu" <hjl@...rceware.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 5/7] x86/boot: Remove run-time relocations from
 .head.text code

On Mon, Jun 29, 2020 at 10:09:26AM -0400, Arvind Sankar wrote:
> The assembly code in head_{32,64}.S, while meant to be
> position-independent, generates run-time relocations because it uses
> instructions such as
> 	leal	gdt(%edx), %eax
> which make the assembler and linker think that the code is using %edx as
> an index into gdt, and hence gdt needs to be relocated to its run-time
> address.
> 
> On 32-bit, with lld Dmitry Golovin reports that this results in a
> link-time error with default options (i.e. unless -z notext is
> explicitly passed):
>   LD      arch/x86/boot/compressed/vmlinux
> ld.lld: error: can't create dynamic relocation R_386_32 against local
> symbol in readonly segment; recompile object files with -fPIC or pass
> '-Wl,-z,notext' to allow text relocations in the output
> 
> With the BFD linker, this generates a warning during the build, if
> --warn-shared-textrel is enabled, which at least Gentoo enables by
> default:
>   LD      arch/x86/boot/compressed/vmlinux
> ld: arch/x86/boot/compressed/head_32.o: warning: relocation in read-only section `.head.text'
> ld: warning: creating a DT_TEXTREL in object
> 
> On 64-bit, it is not possible to link the kernel as -pie with lld, and
> it is only possible with a BFD linker that supports -z noreloc-overflow,
> i.e. versions >2.26. This is because these instructions cannot really be
> relocated: the displacement field is only 32-bits wide, and thus cannot
> be relocated for a 64-bit load address. The -z noreloc-overflow option
> simply overrides the linker error, and results in R_X86_64_RELATIVE
> relocations that apply a 64-bit relocation to a 32-bit field anyway.
> This happens to work because nothing will process these run-time
> relocations.

Are any of Thomas Garnier's PIE fixes useful here too? He had a lot of
fixes to make changes for PC-relative addressing in the various
assembly bits:
https://lore.kernel.org/lkml/20200228000105.165012-9-thgarnie@chromium.org/

> 
> Start fixing this by removing relocations from .head.text:
> - On 32-bit, use a base register that holds the address of the GOT and
>   reference symbol addresses using @GOTOFF, i.e.
> 	leal	gdt@...OFF(%edx), %eax
> - On 64-bit, most of the code can (and already does) use %rip-relative
>   addressing, however the .code32 bits can't, and the 64-bit code also
>   needs to reference symbol addresses as they will be after moving the
>   compressed kernel to the end of the decompression buffer.
>   For these cases, reference the symbols as an offset to startup_32 to
>   avoid creating relocations, i.e.
>   	leal	(gdt-startup_32)(%bp), %eax
>   This only works in .head.text as the subtraction cannot be represented
>   as a PC-relative relocation unless startup_32 is in the same section
>   as the code. Move efi32_pe_entry into .head.text so that it can use
>   the same method to avoid relocations.
> 
> Signed-off-by: Arvind Sankar <nivedita@...m.mit.edu>
> Reviewed-by: Ard Biesheuvel <ardb@...nel.org>
> Reviewed-by: Fangrui Song <maskray@...gle.com>

Nice.

Reviewed-by: Kees Cook <keescook@...omium.org>

-- 
Kees Cook