1. Patchew
  2. Xen
  3. [PATCH v2 0/4] Add stack protector
  4. View patch

[PATCH v2 1/4] common: remove -fno-stack-protector from EMBEDDED_EXTRA_CFLAGS

Volodymyr Babchuk posted 4 patches 3 weeks, 5 days ago
There is a newer version of this series
[PATCH v2 1/4] common: remove -fno-stack-protector from EMBEDDED_EXTRA_CFLAGS
Posted by Volodymyr Babchuk 3 weeks, 5 days ago 
This patch is preparation for making stack protector
configurable. First step is to remove -fno-stack-protector flag from
EMBEDDED_EXTRA_CFLAGS so separate projects (Hypervisor in this case)
can enable/disable this feature by themselves.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

---

Changes in v2:
 - New in v2
---
 Config.mk                            | 2 +-
 stubdom/Makefile                     | 2 ++
 tools/firmware/Rules.mk              | 2 ++
 tools/tests/x86_emulator/testcase.mk | 2 ++
 xen/Makefile                         | 2 ++
 5 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/Config.mk b/Config.mk
index fa0414055b..c9fef4659f 100644
--- a/Config.mk
+++ b/Config.mk
@@ -190,7 +190,7 @@ endif
 APPEND_LDFLAGS += $(foreach i, $(APPEND_LIB), -L$(i))
 APPEND_CFLAGS += $(foreach i, $(APPEND_INCLUDES), -I$(i))
 
-EMBEDDED_EXTRA_CFLAGS := -fno-pie -fno-stack-protector
+EMBEDDED_EXTRA_CFLAGS := -fno-pie
 EMBEDDED_EXTRA_CFLAGS += -fno-exceptions -fno-asynchronous-unwind-tables
 
 XEN_EXTFILES_URL ?= https://xenbits.xen.org/xen-extfiles
diff --git a/stubdom/Makefile b/stubdom/Makefile
index 2a81af28a1..41424f6aca 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -54,6 +54,8 @@ TARGET_CFLAGS += $(CFLAGS)
 TARGET_CPPFLAGS += $(CPPFLAGS)
 $(call cc-options-add,TARGET_CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 
+$(call cc-option-add,TARGET_CFLAGS,CC,-fno-stack-protector)
+
 # Do not use host headers and libs
 GCC_INSTALL = $(shell LANG=C gcc -print-search-dirs | sed -n -e 's/install: \(.*\)/\1/p')
 TARGET_CPPFLAGS += -U __linux__ -U __FreeBSD__ -U __sun__
diff --git a/tools/firmware/Rules.mk b/tools/firmware/Rules.mk
index d3482c9ec4..b3f29556b7 100644
--- a/tools/firmware/Rules.mk
+++ b/tools/firmware/Rules.mk
@@ -15,6 +15,8 @@ $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 
 $(call cc-option-add,CFLAGS,CC,-fcf-protection=none)
 
+$(call cc-option-add,CFLAGS,CC,-fno-stack-protector)
+
 # Do not add the .note.gnu.property section to any of the firmware objects: it
 # breaks the rombios binary and is not useful for firmware anyway.
 $(call cc-option-add,CFLAGS,CC,-Wa$$(comma)-mx86-used-note=no)
diff --git a/tools/tests/x86_emulator/testcase.mk b/tools/tests/x86_emulator/testcase.mk
index fc95e24589..49a7a8dee9 100644
--- a/tools/tests/x86_emulator/testcase.mk
+++ b/tools/tests/x86_emulator/testcase.mk
@@ -4,6 +4,8 @@ include $(XEN_ROOT)/tools/Rules.mk
 
 $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 
+$(call cc-option-add,CFLAGS,CC,-fno-stack-protector)
+
 CFLAGS += -fno-builtin -g0 $($(TESTCASE)-cflags)
 
 LDFLAGS_DIRECT += $(shell { $(LD) -v --warn-rwx-segments; } >/dev/null 2>&1 && echo --no-warn-rwx-segments)
diff --git a/xen/Makefile b/xen/Makefile
index 2e1a925c84..34ed8c0fc7 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -432,6 +432,8 @@ else
 CFLAGS_UBSAN :=
 endif
 
+CFLAGS += -fno-stack-protector
+
 ifeq ($(CONFIG_LTO),y)
 CFLAGS += -flto
 LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so
-- 
2.47.1
Re: [PATCH v2 1/4] common: remove -fno-stack-protector from EMBEDDED_EXTRA_CFLAGS
Posted by Jan Beulich 3 weeks, 3 days ago 
On 30.11.2024 02:10, Volodymyr Babchuk wrote:
> This patch is preparation for making stack protector
> configurable. First step is to remove -fno-stack-protector flag from
> EMBEDDED_EXTRA_CFLAGS so separate projects (Hypervisor in this case)
> can enable/disable this feature by themselves.

s/projects/components/ ?

> --- a/stubdom/Makefile
> +++ b/stubdom/Makefile
> @@ -54,6 +54,8 @@ TARGET_CFLAGS += $(CFLAGS)
>  TARGET_CPPFLAGS += $(CPPFLAGS)
>  $(call cc-options-add,TARGET_CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>  
> +$(call cc-option-add,TARGET_CFLAGS,CC,-fno-stack-protector)
> +
>  # Do not use host headers and libs
>  GCC_INSTALL = $(shell LANG=C gcc -print-search-dirs | sed -n -e 's/install: \(.*\)/\1/p')
>  TARGET_CPPFLAGS += -U __linux__ -U __FreeBSD__ -U __sun__
> --- a/tools/firmware/Rules.mk
> +++ b/tools/firmware/Rules.mk
> @@ -15,6 +15,8 @@ $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>  
>  $(call cc-option-add,CFLAGS,CC,-fcf-protection=none)
>  
> +$(call cc-option-add,CFLAGS,CC,-fno-stack-protector)
> +
>  # Do not add the .note.gnu.property section to any of the firmware objects: it
>  # breaks the rombios binary and is not useful for firmware anyway.
>  $(call cc-option-add,CFLAGS,CC,-Wa$$(comma)-mx86-used-note=no)
> --- a/tools/tests/x86_emulator/testcase.mk
> +++ b/tools/tests/x86_emulator/testcase.mk
> @@ -4,6 +4,8 @@ include $(XEN_ROOT)/tools/Rules.mk
>  
>  $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>  
> +$(call cc-option-add,CFLAGS,CC,-fno-stack-protector)

Is use of cc-option-add really necessary throughout here, when ...

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -432,6 +432,8 @@ else
>  CFLAGS_UBSAN :=
>  endif
>  
> +CFLAGS += -fno-stack-protector

... is isn't needed here? Iirc the compiler version ranges supported don't
vary between components. Then again afaics $(EMBEDDED_EXTRA_CFLAGS) is used
by x86 only right now, and with cc-options-add, so perhaps it (a) needs
using cc-options-add here, too, and (b) it wants explaining why this needs
generalizing from x86 to all architectures. Quite possibly hypervisor use
of $(EMBEDDED_EXTRA_CFLAGS) may want generalizing separately, up front?

Jan
Re: [PATCH v2 1/4] common: remove -fno-stack-protector from EMBEDDED_EXTRA_CFLAGS
Posted by Jan Beulich 3 weeks, 3 days ago 
On 02.12.2024 09:06, Jan Beulich wrote:
> On 30.11.2024 02:10, Volodymyr Babchuk wrote:
>> This patch is preparation for making stack protector
>> configurable. First step is to remove -fno-stack-protector flag from
>> EMBEDDED_EXTRA_CFLAGS so separate projects (Hypervisor in this case)
>> can enable/disable this feature by themselves.
> 
> s/projects/components/ ?
> 
>> --- a/stubdom/Makefile
>> +++ b/stubdom/Makefile
>> @@ -54,6 +54,8 @@ TARGET_CFLAGS += $(CFLAGS)
>>  TARGET_CPPFLAGS += $(CPPFLAGS)
>>  $(call cc-options-add,TARGET_CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>>  
>> +$(call cc-option-add,TARGET_CFLAGS,CC,-fno-stack-protector)
>> +
>>  # Do not use host headers and libs
>>  GCC_INSTALL = $(shell LANG=C gcc -print-search-dirs | sed -n -e 's/install: \(.*\)/\1/p')
>>  TARGET_CPPFLAGS += -U __linux__ -U __FreeBSD__ -U __sun__
>> --- a/tools/firmware/Rules.mk
>> +++ b/tools/firmware/Rules.mk
>> @@ -15,6 +15,8 @@ $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>>  
>>  $(call cc-option-add,CFLAGS,CC,-fcf-protection=none)
>>  
>> +$(call cc-option-add,CFLAGS,CC,-fno-stack-protector)
>> +
>>  # Do not add the .note.gnu.property section to any of the firmware objects: it
>>  # breaks the rombios binary and is not useful for firmware anyway.
>>  $(call cc-option-add,CFLAGS,CC,-Wa$$(comma)-mx86-used-note=no)
>> --- a/tools/tests/x86_emulator/testcase.mk
>> +++ b/tools/tests/x86_emulator/testcase.mk
>> @@ -4,6 +4,8 @@ include $(XEN_ROOT)/tools/Rules.mk
>>  
>>  $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>>  
>> +$(call cc-option-add,CFLAGS,CC,-fno-stack-protector)
> 
> Is use of cc-option-add really necessary throughout here, when ...
> 
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -432,6 +432,8 @@ else
>>  CFLAGS_UBSAN :=
>>  endif
>>  
>> +CFLAGS += -fno-stack-protector
> 
> ... is isn't needed here? Iirc the compiler version ranges supported don't
> vary between components. Then again afaics $(EMBEDDED_EXTRA_CFLAGS) is used
> by x86 only right now, and with cc-options-add, so perhaps it (a) needs
> using cc-options-add here, too, and (b) it wants explaining why this needs
> generalizing from x86 to all architectures. Quite possibly hypervisor use
> of $(EMBEDDED_EXTRA_CFLAGS) may want generalizing separately, up front?

Correction: Except for PPC all architectures consume $(EMBEDDED_EXTRA_CFLAGS)
right now. So the moving is less of a generalization than I first thought. I
still need to get used to passing -R (rather than -r) to grep, to find all
instances I'm after ...

Jan
Re: [PATCH v2 1/4] common: remove -fno-stack-protector from EMBEDDED_EXTRA_CFLAGS
Posted by Andrew Cooper 3 weeks, 3 days ago 
On 02/12/2024 8:06 am, Jan Beulich wrote:
> On 30.11.2024 02:10, Volodymyr Babchuk wrote:
>> This patch is preparation for making stack protector
>> configurable. First step is to remove -fno-stack-protector flag from
>> EMBEDDED_EXTRA_CFLAGS so separate projects (Hypervisor in this case)
>> can enable/disable this feature by themselves.
> s/projects/components/ ?
>
>> --- a/stubdom/Makefile
>> +++ b/stubdom/Makefile
>> @@ -54,6 +54,8 @@ TARGET_CFLAGS += $(CFLAGS)
>>  TARGET_CPPFLAGS += $(CPPFLAGS)
>>  $(call cc-options-add,TARGET_CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>>  
>> +$(call cc-option-add,TARGET_CFLAGS,CC,-fno-stack-protector)
>> +
>>  # Do not use host headers and libs
>>  GCC_INSTALL = $(shell LANG=C gcc -print-search-dirs | sed -n -e 's/install: \(.*\)/\1/p')
>>  TARGET_CPPFLAGS += -U __linux__ -U __FreeBSD__ -U __sun__
>> --- a/tools/firmware/Rules.mk
>> +++ b/tools/firmware/Rules.mk
>> @@ -15,6 +15,8 @@ $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>>  
>>  $(call cc-option-add,CFLAGS,CC,-fcf-protection=none)
>>  
>> +$(call cc-option-add,CFLAGS,CC,-fno-stack-protector)
>> +
>>  # Do not add the .note.gnu.property section to any of the firmware objects: it
>>  # breaks the rombios binary and is not useful for firmware anyway.
>>  $(call cc-option-add,CFLAGS,CC,-Wa$$(comma)-mx86-used-note=no)
>> --- a/tools/tests/x86_emulator/testcase.mk
>> +++ b/tools/tests/x86_emulator/testcase.mk
>> @@ -4,6 +4,8 @@ include $(XEN_ROOT)/tools/Rules.mk
>>  
>>  $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>>  
>> +$(call cc-option-add,CFLAGS,CC,-fno-stack-protector)
> Is use of cc-option-add really necessary throughout here, when ...
>
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -432,6 +432,8 @@ else
>>  CFLAGS_UBSAN :=
>>  endif
>>  
>> +CFLAGS += -fno-stack-protector
> ... is isn't needed here? Iirc the compiler version ranges supported don't
> vary between components. Then again afaics $(EMBEDDED_EXTRA_CFLAGS) is used
> by x86 only right now, and with cc-options-add, so perhaps it (a) needs
> using cc-options-add here, too, and (b) it wants explaining why this needs
> generalizing from x86 to all architectures. Quite possibly hypervisor use
> of $(EMBEDDED_EXTRA_CFLAGS) may want generalizing separately, up front?

EMBEDDED_EXTRA_CFLAGS uses cc-*-add because some options are (/were) not
accepted by compilers.  Notably -fno-stack-protector-all (found from v1
of this series), and prior to that, -no-pie which as I recall is an LD
option not a CC option.

All supported compilers know -fno-stack-protector (found when checking
-fno-stack-protector-all) so it can be added to plain CFLAGS everywhere,
not only in xen/

~Andrew

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.