From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752415AbcELWaS (ORCPT ); Thu, 12 May 2016 18:30:18 -0400 Received: from terminus.zytor.com ([198.137.202.10]:55910 "EHLO mail.zytor.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752169AbcELWaP (ORCPT ); Thu, 12 May 2016 18:30:15 -0400 User-Agent: K-9 Mail for Android In-Reply-To: <20160512203104.GA7425@www.outflux.net> References: <20160512203104.GA7425@www.outflux.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Subject: Re: [PATCH] x86/boot: Refuse to build with data relocations From: "H. Peter Anvin" Date: Thu, 12 May 2016 15:29:50 -0700 To: Kees Cook , Ingo Molnar CC: Borislav Petkov , Baoquan He , Yinghai Lu , Thomas Gleixner , Ingo Molnar , x86@kernel.org, Andrew Morton , Josh Poimboeuf , Andrey Ryabinin , "H.J. Lu" , Dmitry Vyukov , linux-kernel@vger.kernel.org Message-ID: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On May 12, 2016 1:31:04 PM PDT, Kees Cook wrote: >The compressed kernel is built with -fPIC/-fPIE so that it can run in >any >location a bootloader happens to put it. However, since ELF relocation >processing is not happening (and all the relocation information has >already been stripped at link time), none of the code can use data >relocations (e.g. static assignments of pointers). This is already >noted >in a warning comment at the top of misc.c, but this adds an explicit >check for the condition during the linking stage to block any such bugs >from appearing. > >If this was in place with the earlier bug in pagetable.c, the build >would fail like this: > > ... > CC arch/x86/boot/compressed/pagetable.o > DATAREL arch/x86/boot/compressed/vmlinux > error: arch/x86/boot/compressed/pagetable.o has data relocations! > make[2]: *** [arch/x86/boot/compressed/vmlinux] Error 1 > ... > >A clean build shows the new check: > > ... > CC arch/x86/boot/compressed/pagetable.o > DATAREL arch/x86/boot/compressed/vmlinux > LD arch/x86/boot/compressed/vmlinux > ... > >Suggested-by: Ingo Molnar >Signed-off-by: Kees Cook >--- > arch/x86/boot/compressed/Makefile | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > >diff --git a/arch/x86/boot/compressed/Makefile >b/arch/x86/boot/compressed/Makefile >index cfdd8c3f8af2..25d477fcd5b4 100644 >--- a/arch/x86/boot/compressed/Makefile >+++ b/arch/x86/boot/compressed/Makefile >@@ -85,7 +85,25 @@ vmlinux-objs-$(CONFIG_EFI_STUB) += $(obj)/eboot.o >$(obj)/efi_stub_$(BITS).o \ > $(objtree)/drivers/firmware/efi/libstub/lib.a > vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o > >+# The compressed kernel is built with -fPIC/-fPIE so that a boot >loader >+# can place it anywhere in memory and it will still run. However, >since >+# it is executed as-is without any ELF relocation processing performed >+# (and has already had all relocation sections stripped from the >binary), >+# none of the code can use data relocations (e.g. static assignments >of >+# pointer values), since they will be meaningless at runtime. This >check >+# will refuse to link the vmlinux if any of these relocations are >found. >+quiet_cmd_check_data_rel = DATAREL $@ >+define cmd_check_data_rel >+ for obj in $(filter %.o,$^); do \ >+ readelf -S $$obj | grep -qF .data.rel && { \ >+ echo "error: $$obj has data relocations!" >&2; \ >+ exit 1; \ >+ } || true; \ >+ done >+endef >+ > $(obj)/vmlinux: $(vmlinux-objs-y) FORCE >+ $(call if_changed,check_data_rel) > $(call if_changed,ld) > @: > It would be far better to warn on the *type* of relocations rather than in which section they feel. -- Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.