On Thu, May 12, 2016 at 3:29 PM, H. Peter Anvin <h...@zytor.com> wrote:
> On May 12, 2016 1:31:04 PM PDT, Kees Cook <keesc...@chromium.org> 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 <mi...@kernel.org>
>>Signed-off-by: Kees Cook <keesc...@chromium.org>
>>---
>> 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.
I'm open to specific changes. What's the best way to detect what you want here?
-Kees
--
Kees Cook
Chrome OS & Brillo Security
