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: <CAGXu5jJOnADjRkkC_v3rC3ruFrNxM3s7vG9u4jMnQY=yJUT1kA@mail.gmail.com>
Date:	Tue, 17 May 2016 01:13:14 -0700
From:	Kees Cook <keescook@...omium.org>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	"H. Peter Anvin" <hpa@...or.com>, 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 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).

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.

-Kees

>
>  fomalhaut:~/linux/linux> readelf -r arch/x86/boot/compressed/vmlinux | grep -q  'There are no relocations in this file' ; echo $?
>  0
>
> versus a regular object file with lots of relocations:
>
>  fomalhaut:~/linux/linux> readelf -r arch/x86/built-in.o | grep -q 'There are no relocations in this file' ; echo $?
>  1
>
> I.e. the relevant portion of Kees's patch would do something like:
>
> quiet_cmd_check_data_rel = DATAREL $@
> define cmd_check_data_rel
>        for obj in $(filter %.o,$^); do \
>                readelf -r $$obj | grep -qF 'There are no relocations in this file' && exit 0 || { \
>                        echo "error: $$obj has data relocations!" >&2; \
>                        exit 1; \
>                } \
>        done
> endef
>
> (totally untested)
>
> Agreed?
>
> Thanks,
>
>         Ingo



-- 
Kees Cook
Chrome OS & Brillo Security