Messages in this thread | | | Date | Thu, 12 May 2016 15:54:24 -0700 | Subject | Re: [PATCH] x86/boot: Refuse to build with data relocations | From | Kees Cook <> |
| |
On Thu, May 12, 2016 at 3:29 PM, H. Peter Anvin <hpa@zytor.com> wrote: > On May 12, 2016 1:31:04 PM PDT, Kees Cook <keescook@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 <mingo@kernel.org> >>Signed-off-by: Kees Cook <keescook@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
| |