diff mbox series

[v4,07/26] x86/build: Check W^X of vmlinux during build

Message ID 3ca525852ce14a8e04949ff115cb6ec28c8f120b.1671098103.git.baskov@ispras.ru
State New
Headers
Series x86_64: Improvements at compressed kernel stage |

Commit Message

Evgeniy Baskov Dec. 15, 2022, 12:37 p.m. UTC 
  Check if there are simultaneously writable and executable
program segments in vmlinux ELF image and fail build if there are any.

This would prevent accidental introduction of RWX segments.

Tested-by: Mario Limonciello <mario.limonciello@amd.com>
Tested-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
---
 arch/x86/boot/compressed/Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Ard Biesheuvel March 8, 2023, 9:34 a.m. UTC | #1 
On Thu, 15 Dec 2022 at 13:38, Evgeniy Baskov <baskov@ispras.ru> wrote:
>
> Check if there are simultaneously writable and executable
> program segments in vmlinux ELF image and fail build if there are any.
>
> This would prevent accidental introduction of RWX segments.
>
> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
> Tested-by: Peter Jones <pjones@redhat.com>
> Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
> ---
>  arch/x86/boot/compressed/Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 1acff356d97a..4dcab38f5a38 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -112,11 +112,17 @@ vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi.o
>  vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_mixed.o
>  vmlinux-objs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
>
> +quiet_cmd_wx_check = WXCHK   $<
> +cmd_wx_check = if $(OBJDUMP) -p $< | grep "flags .wx" > /dev/null; \
> +              then (echo >&2 "$<: Simultaneously writable and executable sections are prohibited"; \
> +                    /bin/false); fi
> +
>  $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
>         $(call if_changed,ld)
>
>  OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
>  $(obj)/vmlinux.bin: vmlinux FORCE
> +       $(call cmd,wx_check)

This breaks the way we track dependencies between make targets: the
FORCE will result in the check being performed every time, even if
nothing gets rebuilt.

Better to do something like the below (apologies for the alphabet soup)


--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -112,18 +112,17 @@ vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi.o
 vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_mixed.o
 vmlinux-objs-$(CONFIG_EFI_STUB) +=
$(objtree)/drivers/firmware/efi/libstub/lib.a

-quiet_cmd_wx_check = WXCHK   $<
-cmd_wx_check = if $(OBJDUMP) -p $< | grep "flags .wx" > /dev/null; \
-              then (echo >&2 "$<: Simultaneously writable and
executable sections are prohibited"; \
-                    /bin/false); fi
+quiet_cmd_objcopy_and_wx_check = $(quiet_cmd_objcopy)
+      cmd_objcopy_and_wx_check = if $(OBJDUMP) -p $< | grep "flags
.wx" > /dev/null; then \
+                                       (echo >&2 "$<: Simultaneously
writable and executable sections are prohibited"; \
+                                       /bin/false); else $(cmd_objcopy); fi

 $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
        $(call if_changed,ld)

 OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
 $(obj)/vmlinux.bin: vmlinux FORCE
-       $(call cmd,wx_check)
-       $(call if_changed,objcopy)
+       $(call if_changed,objcopy_and_wx_check)
Evgeniy Baskov March 8, 2023, 4:05 p.m. UTC | #2 
On 2023-03-08 12:34, Ard Biesheuvel wrote:
> On Thu, 15 Dec 2022 at 13:38, Evgeniy Baskov <baskov@ispras.ru> wrote:
>> 
>> Check if there are simultaneously writable and executable
>> program segments in vmlinux ELF image and fail build if there are any.
>> 
>> This would prevent accidental introduction of RWX segments.
>> 
>> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
>> Tested-by: Peter Jones <pjones@redhat.com>
>> Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
>> ---
>>  arch/x86/boot/compressed/Makefile | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/arch/x86/boot/compressed/Makefile 
>> b/arch/x86/boot/compressed/Makefile
>> index 1acff356d97a..4dcab38f5a38 100644
>> --- a/arch/x86/boot/compressed/Makefile
>> +++ b/arch/x86/boot/compressed/Makefile
>> @@ -112,11 +112,17 @@ vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi.o
>>  vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_mixed.o
>>  vmlinux-objs-$(CONFIG_EFI_STUB) += 
>> $(objtree)/drivers/firmware/efi/libstub/lib.a
>> 
>> +quiet_cmd_wx_check = WXCHK   $<
>> +cmd_wx_check = if $(OBJDUMP) -p $< | grep "flags .wx" > /dev/null; \
>> +              then (echo >&2 "$<: Simultaneously writable and 
>> executable sections are prohibited"; \
>> +                    /bin/false); fi
>> +
>>  $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
>>         $(call if_changed,ld)
>> 
>>  OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
>>  $(obj)/vmlinux.bin: vmlinux FORCE
>> +       $(call cmd,wx_check)
> 
> This breaks the way we track dependencies between make targets: the
> FORCE will result in the check being performed every time, even if
> nothing gets rebuilt.
> 
> Better to do something like the below (apologies for the alphabet soup)
> 
> 
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -112,18 +112,17 @@ vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi.o
>  vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_mixed.o
>  vmlinux-objs-$(CONFIG_EFI_STUB) +=
> $(objtree)/drivers/firmware/efi/libstub/lib.a
> 
> -quiet_cmd_wx_check = WXCHK   $<
> -cmd_wx_check = if $(OBJDUMP) -p $< | grep "flags .wx" > /dev/null; \
> -              then (echo >&2 "$<: Simultaneously writable and
> executable sections are prohibited"; \
> -                    /bin/false); fi
> +quiet_cmd_objcopy_and_wx_check = $(quiet_cmd_objcopy)
> +      cmd_objcopy_and_wx_check = if $(OBJDUMP) -p $< | grep "flags
> .wx" > /dev/null; then \
> +                                       (echo >&2 "$<: Simultaneously
> writable and executable sections are prohibited"; \
> +                                       /bin/false); else 
> $(cmd_objcopy); fi
> 
>  $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
>         $(call if_changed,ld)
> 
>  OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
>  $(obj)/vmlinux.bin: vmlinux FORCE
> -       $(call cmd,wx_check)
> -       $(call if_changed,objcopy)
> +       $(call if_changed,objcopy_and_wx_check)

Thank you for suggestion! I will fix it.
diff mbox series

Patch

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 1acff356d97a..4dcab38f5a38 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -112,11 +112,17 @@  vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi.o
 vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_mixed.o
 vmlinux-objs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
 
+quiet_cmd_wx_check = WXCHK   $<
+cmd_wx_check = if $(OBJDUMP) -p $< | grep "flags .wx" > /dev/null; \
+	       then (echo >&2 "$<: Simultaneously writable and executable sections are prohibited"; \
+		     /bin/false); fi
+
 $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
 	$(call if_changed,ld)
 
 OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
 $(obj)/vmlinux.bin: vmlinux FORCE
+	$(call cmd,wx_check)
 	$(call if_changed,objcopy)
 
 targets += $(patsubst $(obj)/%,%,$(vmlinux-objs-y)) vmlinux.bin.all vmlinux.relocs