linux-kernel - Re: [PATCH] x86/boot: Refuse to build with data relocations

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: <3b9c1266-f69b-50e4-36fd-ab3e75875293@zytor.com>
Date:	Tue, 17 May 2016 02:31:07 -0700
From:	"H. Peter Anvin" <hpa@...or.com>
To:	Kees Cook <keescook@...omium.org>, Ingo Molnar <mingo@...nel.org>
Cc:	Borislav Petkov <bp@...e.de>, Baoquan He <bhe@...hat.com>,
	Yinghai Lu <yinghai@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"x86@...nel.org" <x86@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Josh Poimboeuf <jpoimboe@...hat.com>,
	Andrey Ryabinin <aryabinin@...tuozzo.com>,
	"H.J. Lu" <hjl.tools@...il.com>,
	Dmitry Vyukov <dvyukov@...gle.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/boot: Refuse to build with data relocations

On 05/17/16 01:13, Kees Cook wrote:
> On Mon, May 16, 2016 at 3:30 AM, Ingo Molnar <mingo@...nel.org> wrote:
>>
>> * H. Peter Anvin <hpa@...or.com> wrote:
>>
>>> On 05/12/16 15:54, Kees Cook wrote:
>>>>>
>>>>> It would be far better to warn on the *type* of relocations rather than in which section they feel.
>>>>
>>>> I'm open to specific changes. What's the best way to detect what you want here?
>>>>
>>>
>>> Use readelf -r and look for inappropriate relocation types (which are
>>> basically the same ones that we should have to muck with for the main
>>> kernel in relocs.c.)
>>
>> I suspect initially we are good if we don't allow any relocations in
>> arch/x86/boot/compressed/vmlinux:
> 
> No, examining vmlinux is already too late, since it was built with a
> linker script that explicitly drops all sections it doesn't know how
> to handle (including these "bad" relocations).

Either look at the inputs, or add the -q option to the link line
(--emit-relocs); that preserves the relocations into the output file
(the same we use to generate the relocation tables to be able to
relocate the kernel proper.)

> The "good" relocations are those that are either PC-relative or
> resolved during .o linking (For example, all the assembler .o files
> have non-PC relocations that are fully resolved after getting linked
> together into the vmlinux, since those are organized by section.)
> 
> So, we do specifically need to look at the _section_, not the _type_,
> to draw any meaningful conclusion. I think my original patch is
> correct and sufficient.

No.  This is completely wrong, because it *IS* the TYPE that matters.
You can end up with absolute relocations in text because someone is
careless writing assembly, for example.

Adding -q to the linker command line will preserve the relocations,
again, what we do to generate the relocation tables for the kernel
proper as well as the realmode stub.

I have to admit that peeking at the object code I have to wonder if we
wouldn't be better off just doing what we do for the kernel and just
relocate it explicitly.  We already have to relocate the GOT; the code
to do general relocation is no more complex and we already have done it
multiple times.

I *do* see a disturbing number of absolute references in the current
object; some of those are references to absolute symbols, however.  This
is where leveraging our existing relocs program would be awesome,
because it already has the necessary logic to deal with absolute symbols
and so on.

	-hpa