2022-12-15 12:42:37

by Evgeniy Baskov

[permalink] [raw]
Subject: [PATCH v4 07/26] x86/build: Check W^X of vmlinux during build

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 <[email protected]>
Tested-by: Peter Jones <[email protected]>
Signed-off-by: Evgeniy Baskov <[email protected]>
---
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)
$(call if_changed,objcopy)

targets += $(patsubst $(obj)/%,%,$(vmlinux-objs-y)) vmlinux.bin.all vmlinux.relocs
--
2.37.4


2023-03-08 09:34:22

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v4 07/26] x86/build: Check W^X of vmlinux during build

On Thu, 15 Dec 2022 at 13:38, Evgeniy Baskov <[email protected]> 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 <[email protected]>
> Tested-by: Peter Jones <[email protected]>
> Signed-off-by: Evgeniy Baskov <[email protected]>
> ---
> 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)

2023-03-08 16:07:01

by Evgeniy Baskov

[permalink] [raw]
Subject: Re: [PATCH v4 07/26] x86/build: Check W^X of vmlinux during build

On 2023-03-08 12:34, Ard Biesheuvel wrote:
> On Thu, 15 Dec 2022 at 13:38, Evgeniy Baskov <[email protected]> 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 <[email protected]>
>> Tested-by: Peter Jones <[email protected]>
>> Signed-off-by: Evgeniy Baskov <[email protected]>
>> ---
>> 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.