[Buildroot] [PATCH 1/5 v2] toolchain: prepare to pass more additional CFLAGS via the wrapper

[Buildroot] [PATCH 1/5 v2] toolchain: prepare to pass more additional CFLAGS via the wrapper

[yann.morin at orange.com]

From: "Yann E. MORIN" <yann.morin@orange.com>

Currently, we pass the user-supplied so-called target optimisation flags
to the wrapper.

We're going to have additional such CFLAGS to pass, so push-back the
formatting loop to quote the options at the last moment.

Reported-by: Arnout Vandecappelle <arnout@mind.be>
Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 toolchain/toolchain-wrapper.mk | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/toolchain-wrapper.mk
index 613f5f6c56..c9e6fd3f3d 100644
--- a/toolchain/toolchain-wrapper.mk
+++ b/toolchain/toolchain-wrapper.mk
@@ -16,11 +16,13 @@ endif
 TOOLCHAIN_WRAPPER_ARGS = $($(PKG)_TOOLCHAIN_WRAPPER_ARGS)
 TOOLCHAIN_WRAPPER_ARGS += -DBR_SYSROOT='"$(STAGING_SUBDIR)"'
 
+TOOLCHAIN_WRAPPER_OPTS = \
+	$(call qstrip,$(BR2_TARGET_OPTIMIZATION))
+
 # We create a list like '"-mfoo", "-mbar", "-mbarfoo"' so that each flag is a
 # separate argument when used in execv() by the toolchain wrapper.
-TOOLCHAIN_WRAPPER_OPTS = \
-	$(foreach f,$(call qstrip,$(BR2_TARGET_OPTIMIZATION)),"$(f)"$(comma))
-TOOLCHAIN_WRAPPER_ARGS += -DBR_ADDITIONAL_CFLAGS='$(TOOLCHAIN_WRAPPER_OPTS)'
+TOOLCHAIN_WRAPPER_ARGS += \
+	-DBR_ADDITIONAL_CFLAGS='$(foreach f,$(TOOLCHAIN_WRAPPER_OPTS),"$(f)"$(comma))'
 
 ifeq ($(BR2_CCACHE),y)
 TOOLCHAIN_WRAPPER_ARGS += -DBR_CCACHE
-- 
2.17.1


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

[Buildroot] [PATCH 1/5 v2] toolchain: prepare to pass more additional CFLAGS via the wrapper

[Arnout Vandecappelle]

On 12/03/2019 13:09, yann.morin at orange.com wrote:
> From: "Yann E. MORIN" <yann.morin@orange.com>
> 
> Currently, we pass the user-supplied so-called target optimisation flags
> to the wrapper.
> 
> We're going to have additional such CFLAGS to pass, so push-back the
> formatting loop to quote the options at the last moment.
> 
> Reported-by: Arnout Vandecappelle <arnout@mind.be>
> Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

 Applied to master, thanks.

 Regards,
 Arnout

> ---
>  toolchain/toolchain-wrapper.mk | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/toolchain-wrapper.mk
> index 613f5f6c56..c9e6fd3f3d 100644
> --- a/toolchain/toolchain-wrapper.mk
> +++ b/toolchain/toolchain-wrapper.mk
> @@ -16,11 +16,13 @@ endif
>  TOOLCHAIN_WRAPPER_ARGS = $($(PKG)_TOOLCHAIN_WRAPPER_ARGS)
>  TOOLCHAIN_WRAPPER_ARGS += -DBR_SYSROOT='"$(STAGING_SUBDIR)"'
>  
> +TOOLCHAIN_WRAPPER_OPTS = \
> +	$(call qstrip,$(BR2_TARGET_OPTIMIZATION))
> +
>  # We create a list like '"-mfoo", "-mbar", "-mbarfoo"' so that each flag is a
>  # separate argument when used in execv() by the toolchain wrapper.
> -TOOLCHAIN_WRAPPER_OPTS = \
> -	$(foreach f,$(call qstrip,$(BR2_TARGET_OPTIMIZATION)),"$(f)"$(comma))
> -TOOLCHAIN_WRAPPER_ARGS += -DBR_ADDITIONAL_CFLAGS='$(TOOLCHAIN_WRAPPER_OPTS)'
> +TOOLCHAIN_WRAPPER_ARGS += \
> +	-DBR_ADDITIONAL_CFLAGS='$(foreach f,$(TOOLCHAIN_WRAPPER_OPTS),"$(f)"$(comma))'
>  
>  ifeq ($(BR2_CCACHE),y)
>  TOOLCHAIN_WRAPPER_ARGS += -DBR_CCACHE
> 

[Buildroot] [PATCH 2/5 v2] toolchain: set the ssp gcc option in kconfig

[yann.morin at orange.com]

From: "Yann E. MORIN" <yann.morin@orange.com>

Currently, we repeat all the SSP level selection deep down to the
toolchain wrapper itself, where we eventually translate it to the
actual SSP option to use. This is a bit redundant.

Additionally, we will want to check that the toolchain actually
supports that option (for those toolchain where it was backported).

So, move the translation into kconfig, and add the qstrip'ed value
to the additional flags passed to the wrapper. Add it before
user-supplied opitons, to keep the previous behaviour (and allow
anyone crazy-enough to override it with BR2_TARGET_OPTIMIZATION).

Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
Cc: Matt Weber <matthew.weber@rockwellcollins.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

---
Changes v1 -> v2:
  - use TOOLCHAIN_WRAPPER_OPTS  (Arnout)
---
 Config.in                      | 6 ++++++
 toolchain/toolchain-wrapper.c  | 9 ---------
 toolchain/toolchain-wrapper.mk | 9 +--------
 3 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/Config.in b/Config.in
index d58d8dc04a..757ad1ca40 100644
--- a/Config.in
+++ b/Config.in
@@ -764,6 +764,12 @@ config BR2_SSP_ALL
 
 endchoice
 
+config BR2_SSP_OPTION
+	string
+	default "-fstack-protector"        if BR2_SSP_REGULAR
+	default "-fstack-protector-strong" if BR2_SSP_STRONG
+	default "-fstack-protector-all"    if BR2_SSP_ALL
+
 comment "Stack Smashing Protection needs a toolchain w/ SSP"
 	depends on !BR2_TOOLCHAIN_HAS_SSP
 
diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
index e9c5cd9d32..c73a0cc079 100644
--- a/toolchain/toolchain-wrapper.c
+++ b/toolchain/toolchain-wrapper.c
@@ -98,15 +98,6 @@ static char *predef_args[] = {
 #if defined(BR_MIPS_TARGET_BIG_ENDIAN) || defined(BR_ARC_TARGET_BIG_ENDIAN)
 	"-EB",
 #endif
-#ifdef BR_SSP_REGULAR
-	"-fstack-protector",
-#endif
-#ifdef BR_SSP_STRONG
-	"-fstack-protector-strong",
-#endif
-#ifdef BR_SSP_ALL
-	"-fstack-protector-all",
-#endif
 #ifdef BR_ADDITIONAL_CFLAGS
 	BR_ADDITIONAL_CFLAGS
 #endif
diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/toolchain-wrapper.mk
index c9e6fd3f3d..ca66fa7ba4 100644
--- a/toolchain/toolchain-wrapper.mk
+++ b/toolchain/toolchain-wrapper.mk
@@ -17,6 +17,7 @@ TOOLCHAIN_WRAPPER_ARGS = $($(PKG)_TOOLCHAIN_WRAPPER_ARGS)
 TOOLCHAIN_WRAPPER_ARGS += -DBR_SYSROOT='"$(STAGING_SUBDIR)"'
 
 TOOLCHAIN_WRAPPER_OPTS = \
+	$(call qstrip,$(BR2_SSP_OPTION)) \
 	$(call qstrip,$(BR2_TARGET_OPTIMIZATION))
 
 # We create a list like '"-mfoo", "-mbar", "-mbarfoo"' so that each flag is a
@@ -53,14 +54,6 @@ else ifeq ($(BR2_RELRO_FULL),y)
 TOOLCHAIN_WRAPPER_ARGS += -DBR2_RELRO_FULL
 endif
 
-ifeq ($(BR2_SSP_REGULAR),y)
-TOOLCHAIN_WRAPPER_ARGS += -DBR_SSP_REGULAR
-else ifeq ($(BR2_SSP_STRONG),y)
-TOOLCHAIN_WRAPPER_ARGS += -DBR_SSP_STRONG
-else ifeq ($(BR2_SSP_ALL),y)
-TOOLCHAIN_WRAPPER_ARGS += -DBR_SSP_ALL
-endif
-
 define TOOLCHAIN_WRAPPER_BUILD
 	$(HOSTCC) $(HOST_CFLAGS) $(TOOLCHAIN_WRAPPER_ARGS) \
 		-s -Wl,--hash-style=$(TOOLCHAIN_WRAPPER_HASH_STYLE) \
-- 
2.17.1


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

[Buildroot] [PATCH 2/5 v2] toolchain: set the ssp gcc option in kconfig

[Matthew Weber]

Yann,


On Tue, Mar 12, 2019 at 7:10 AM <yann.morin@orange.com> wrote:
>
> From: "Yann E. MORIN" <yann.morin@orange.com>
>
> Currently, we repeat all the SSP level selection deep down to the
> toolchain wrapper itself, where we eventually translate it to the
> actual SSP option to use. This is a bit redundant.
>
> Additionally, we will want to check that the toolchain actually
> supports that option (for those toolchain where it was backported).
>
> So, move the translation into kconfig, and add the qstrip'ed value
> to the additional flags passed to the wrapper. Add it before
> user-supplied opitons, to keep the previous behaviour (and allow
> anyone crazy-enough to override it with BR2_TARGET_OPTIMIZATION).
>
> Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
> Cc: Matt Weber <matthew.weber@rockwellcollins.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>
> ---
> Changes v1 -> v2:
>   - use TOOLCHAIN_WRAPPER_OPTS  (Arnout)
> ---
>  Config.in                      | 6 ++++++
>  toolchain/toolchain-wrapper.c  | 9 ---------
>  toolchain/toolchain-wrapper.mk | 9 +--------
>  3 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/Config.in b/Config.in
> index d58d8dc04a..757ad1ca40 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -764,6 +764,12 @@ config BR2_SSP_ALL
>
>  endchoice
>
> +config BR2_SSP_OPTION
> +       string
> +       default "-fstack-protector"        if BR2_SSP_REGULAR
> +       default "-fstack-protector-strong" if BR2_SSP_STRONG
> +       default "-fstack-protector-all"    if BR2_SSP_ALL
> +
>  comment "Stack Smashing Protection needs a toolchain w/ SSP"
>         depends on !BR2_TOOLCHAIN_HAS_SSP
>

Much cleaner then when I pulled SSP into the wrapper :-)
Reviewed-by: Matthew Weber <matthew.weber@rockwellcollins.com>

[Buildroot] [PATCH 2/5 v2] toolchain: set the ssp gcc option in kconfig

[Arnout Vandecappelle]

On 12/03/2019 13:09, yann.morin at orange.com wrote:
> From: "Yann E. MORIN" <yann.morin@orange.com>
> 
> Currently, we repeat all the SSP level selection deep down to the
> toolchain wrapper itself, where we eventually translate it to the
> actual SSP option to use. This is a bit redundant.
> 
> Additionally, we will want to check that the toolchain actually
> supports that option (for those toolchain where it was backported).
> 
> So, move the translation into kconfig, and add the qstrip'ed value
> to the additional flags passed to the wrapper. Add it before
> user-supplied opitons, to keep the previous behaviour (and allow
> anyone crazy-enough to override it with BR2_TARGET_OPTIMIZATION).
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
> Cc: Matt Weber <matthew.weber@rockwellcollins.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

 Applied to master, thanks.

 Regards,
 Arnout

> 
> ---
> Changes v1 -> v2:
>   - use TOOLCHAIN_WRAPPER_OPTS  (Arnout)
> ---
>  Config.in                      | 6 ++++++
>  toolchain/toolchain-wrapper.c  | 9 ---------
>  toolchain/toolchain-wrapper.mk | 9 +--------
>  3 files changed, 7 insertions(+), 17 deletions(-)
> 
> diff --git a/Config.in b/Config.in
> index d58d8dc04a..757ad1ca40 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -764,6 +764,12 @@ config BR2_SSP_ALL
>  
>  endchoice
>  
> +config BR2_SSP_OPTION
> +	string
> +	default "-fstack-protector"        if BR2_SSP_REGULAR
> +	default "-fstack-protector-strong" if BR2_SSP_STRONG
> +	default "-fstack-protector-all"    if BR2_SSP_ALL
> +
>  comment "Stack Smashing Protection needs a toolchain w/ SSP"
>  	depends on !BR2_TOOLCHAIN_HAS_SSP
>  
> diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
> index e9c5cd9d32..c73a0cc079 100644
> --- a/toolchain/toolchain-wrapper.c
> +++ b/toolchain/toolchain-wrapper.c
> @@ -98,15 +98,6 @@ static char *predef_args[] = {
>  #if defined(BR_MIPS_TARGET_BIG_ENDIAN) || defined(BR_ARC_TARGET_BIG_ENDIAN)
>  	"-EB",
>  #endif
> -#ifdef BR_SSP_REGULAR
> -	"-fstack-protector",
> -#endif
> -#ifdef BR_SSP_STRONG
> -	"-fstack-protector-strong",
> -#endif
> -#ifdef BR_SSP_ALL
> -	"-fstack-protector-all",
> -#endif
>  #ifdef BR_ADDITIONAL_CFLAGS
>  	BR_ADDITIONAL_CFLAGS
>  #endif
> diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/toolchain-wrapper.mk
> index c9e6fd3f3d..ca66fa7ba4 100644
> --- a/toolchain/toolchain-wrapper.mk
> +++ b/toolchain/toolchain-wrapper.mk
> @@ -17,6 +17,7 @@ TOOLCHAIN_WRAPPER_ARGS = $($(PKG)_TOOLCHAIN_WRAPPER_ARGS)
>  TOOLCHAIN_WRAPPER_ARGS += -DBR_SYSROOT='"$(STAGING_SUBDIR)"'
>  
>  TOOLCHAIN_WRAPPER_OPTS = \
> +	$(call qstrip,$(BR2_SSP_OPTION)) \
>  	$(call qstrip,$(BR2_TARGET_OPTIMIZATION))
>  
>  # We create a list like '"-mfoo", "-mbar", "-mbarfoo"' so that each flag is a
> @@ -53,14 +54,6 @@ else ifeq ($(BR2_RELRO_FULL),y)
>  TOOLCHAIN_WRAPPER_ARGS += -DBR2_RELRO_FULL
>  endif
>  
> -ifeq ($(BR2_SSP_REGULAR),y)
> -TOOLCHAIN_WRAPPER_ARGS += -DBR_SSP_REGULAR
> -else ifeq ($(BR2_SSP_STRONG),y)
> -TOOLCHAIN_WRAPPER_ARGS += -DBR_SSP_STRONG
> -else ifeq ($(BR2_SSP_ALL),y)
> -TOOLCHAIN_WRAPPER_ARGS += -DBR_SSP_ALL
> -endif
> -
>  define TOOLCHAIN_WRAPPER_BUILD
>  	$(HOSTCC) $(HOST_CFLAGS) $(TOOLCHAIN_WRAPPER_ARGS) \
>  		-s -Wl,--hash-style=$(TOOLCHAIN_WRAPPER_HASH_STYLE) \
> 

[Buildroot] [PATCH 3/5 v2] toolchain: check the SSP option is known

[yann.morin at orange.com]

From: "Yann E. MORIN" <yann.morin@orange.com>

Some toolchain vendors may have backported those options to older gcc
versions, and we have no way to know, so we have to check that the
user's selection is acceptable.

Extend the macro that currently checks for SSP in the toolchain, with
a new test that the actual SSP option is recognised and accepted.

Note that the SSP option is either totaly empty, or an already-quoted
string, so we can safely and easily assign it to a shell variable to
test and use it.

Note that we do not introduce BR2_TOOLCHAIN_HAS_SSP_STRONG, because:

  - our internal toolchain infra only supports gcc >= 4.9, so it has
    SSP strong;

  - of the external pre-built toolchains, only the codesourcery-arm
    one has a gcc-4.8 which lacks SSP strong, all the others have a
    gcc >= 4.9;

  - we'd still have to do the actual check for custom external
    toolchains anyway.

So, we're not adding BR2_TOOLCHAIN_HAS_SSP_STRONG just for a single
case.

Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
Cc: Matt Weber <matthew.weber@rockwellcollins.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
Cc: Arnout Vandecappelle <arnout@mind.be>

---
Changes v1 -> v2:
  - expand the commit log to explain why we're not adding
    BR2_TOOLCHAIN_HAS_SSP_STRONG  (Arnout)
---
 toolchain/helpers.mk                                   | 8 ++++++++
 toolchain/toolchain-external/pkg-toolchain-external.mk | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
index e5520c00c3..ba097e83cf 100644
--- a/toolchain/helpers.mk
+++ b/toolchain/helpers.mk
@@ -415,6 +415,7 @@ check_unusable_toolchain = \
 # Check if the toolchain has SSP (stack smashing protector) support
 #
 # $1: cross-gcc path
+# $2: gcc ssp option
 #
 check_toolchain_ssp = \
 	__CROSS_CC=$(strip $1) ; \
@@ -427,6 +428,13 @@ check_toolchain_ssp = \
 		echo "SSP support not available in this toolchain, please disable BR2_TOOLCHAIN_EXTERNAL_HAS_SSP" ; \
 		exit 1 ; \
 	fi ; \
+	__SSP_OPTION=$(2); \
+	if [ -n "$${__SSP_OPTION}" ] ; then \
+		if ! echo 'void main(){}' | $${__CROSS_CC} -Werror $${__SSP_OPTION} -x c - -o $(BUILD_DIR)/.br-toolchain-test.tmp >/dev/null 2>&1 ; then \
+			echo "SSP option $${__SSP_OPTION} not available in this toolchain, please select another SSP level" ; \
+			exit 1 ; \
+		fi; \
+	fi; \
 	rm -f $(BUILD_DIR)/.br-toolchain-test.tmp*
 
 #
diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
index db3570d96f..00cbd7b17a 100644
--- a/toolchain/toolchain-external/pkg-toolchain-external.mk
+++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
@@ -549,7 +549,7 @@ define $(2)_CONFIGURE_CMDS
 	else \
 		$$(call check_glibc,$$$${SYSROOT_DIR}) ; \
 	fi
-	$$(Q)$$(call check_toolchain_ssp,$$(TOOLCHAIN_EXTERNAL_CC))
+	$$(Q)$$(call check_toolchain_ssp,$$(TOOLCHAIN_EXTERNAL_CC),$(BR2_SSP_OPTION))
 endef
 
 $(2)_TOOLCHAIN_WRAPPER_ARGS += $$(TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS)
-- 
2.17.1


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

[Buildroot] [PATCH 3/5 v2] toolchain: check the SSP option is known

[Arnout Vandecappelle]

On 12/03/2019 13:09, yann.morin at orange.com wrote:
> From: "Yann E. MORIN" <yann.morin@orange.com>
> 
> Some toolchain vendors may have backported those options to older gcc
> versions, and we have no way to know, so we have to check that the
> user's selection is acceptable.
> 
> Extend the macro that currently checks for SSP in the toolchain, with
> a new test that the actual SSP option is recognised and accepted.
> 
> Note that the SSP option is either totaly empty, or an already-quoted
> string, so we can safely and easily assign it to a shell variable to
> test and use it.
> 
> Note that we do not introduce BR2_TOOLCHAIN_HAS_SSP_STRONG, because:
> 
>   - our internal toolchain infra only supports gcc >= 4.9, so it has
>     SSP strong;
> 
>   - of the external pre-built toolchains, only the codesourcery-arm
>     one has a gcc-4.8 which lacks SSP strong, all the others have a
>     gcc >= 4.9;
> 
>   - we'd still have to do the actual check for custom external
>     toolchains anyway.
> 
> So, we're not adding BR2_TOOLCHAIN_HAS_SSP_STRONG just for a single
> case.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
> Cc: Matt Weber <matthew.weber@rockwellcollins.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> 
> ---
> Changes v1 -> v2:
>   - expand the commit log to explain why we're not adding
>     BR2_TOOLCHAIN_HAS_SSP_STRONG  (Arnout)
> ---
>  toolchain/helpers.mk                                   | 8 ++++++++
>  toolchain/toolchain-external/pkg-toolchain-external.mk | 2 +-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
> index e5520c00c3..ba097e83cf 100644
> --- a/toolchain/helpers.mk
> +++ b/toolchain/helpers.mk
> @@ -415,6 +415,7 @@ check_unusable_toolchain = \
>  # Check if the toolchain has SSP (stack smashing protector) support
>  #
>  # $1: cross-gcc path
> +# $2: gcc ssp option
>  #
>  check_toolchain_ssp = \
>  	__CROSS_CC=$(strip $1) ; \
> @@ -427,6 +428,13 @@ check_toolchain_ssp = \
>  		echo "SSP support not available in this toolchain, please disable BR2_TOOLCHAIN_EXTERNAL_HAS_SSP" ; \
>  		exit 1 ; \
>  	fi ; \
> +	__SSP_OPTION=$(2); \

 Is there any reason to pass this as an argument rather than using
$(BR2_SSP_OPTION) directly?

 The same goes for $(TOOLCHAIN_EXTERNAL_CC), but that ship has sailed.

 Regards,
 Arnout

> +	if [ -n "$${__SSP_OPTION}" ] ; then \
> +		if ! echo 'void main(){}' | $${__CROSS_CC} -Werror $${__SSP_OPTION} -x c - -o $(BUILD_DIR)/.br-toolchain-test.tmp >/dev/null 2>&1 ; then \
> +			echo "SSP option $${__SSP_OPTION} not available in this toolchain, please select another SSP level" ; \
> +			exit 1 ; \
> +		fi; \
> +	fi; \
>  	rm -f $(BUILD_DIR)/.br-toolchain-test.tmp*
>  
>  #
> diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
> index db3570d96f..00cbd7b17a 100644
> --- a/toolchain/toolchain-external/pkg-toolchain-external.mk
> +++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
> @@ -549,7 +549,7 @@ define $(2)_CONFIGURE_CMDS
>  	else \
>  		$$(call check_glibc,$$$${SYSROOT_DIR}) ; \
>  	fi
> -	$$(Q)$$(call check_toolchain_ssp,$$(TOOLCHAIN_EXTERNAL_CC))
> +	$$(Q)$$(call check_toolchain_ssp,$$(TOOLCHAIN_EXTERNAL_CC),$(BR2_SSP_OPTION))
>  endef
>  
>  $(2)_TOOLCHAIN_WRAPPER_ARGS += $$(TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS)
> 

[Buildroot] [PATCH 3/5 v2] toolchain: check the SSP option is known

[yann.morin at orange.com]

Arnout, All,

On 2019-03-13 00:25 +0100, Arnout Vandecappelle spake thusly:
> On 12/03/2019 13:09, yann.morin at orange.com wrote:
> > From: "Yann E. MORIN" <yann.morin@orange.com>
[--SNIP--]
> > Note that the SSP option is either totaly empty, or an already-quoted
> > string, so we can safely and easily assign it to a shell variable to
> > test and use it.

Please note this paragraph above, and...

[--SNIP--]
> > @@ -427,6 +428,13 @@ check_toolchain_ssp = \
> >  		echo "SSP support not available in this toolchain, please disable BR2_TOOLCHAIN_EXTERNAL_HAS_SSP" ; \
> >  		exit 1 ; \
> >  	fi ; \
> > +	__SSP_OPTION=$(2); \
> 
>  Is there any reason to pass this as an argument rather than using
> $(BR2_SSP_OPTION) directly?

... correlate it with your question here.

There are two cases:

  - no SSP      BR2_SSP_OPTION is not defined, so it is not even the
                empty quoted string ""

  - some SSP    BR2_SSP_OPTION is a non-empty quoted string.

So...

> > +	if [ -n "$${__SSP_OPTION}" ] ; then \

... this test would have to use the qstrip-then-quote-anyway dance.

And finally, I tried to mimick the existing code, which lead to the
current patch: pass the option as a parameter to the macro, and use an
intermediate shell variable.

Regards,
Yann E. MORIN.

> > +		if ! echo 'void main(){}' | $${__CROSS_CC} -Werror $${__SSP_OPTION} -x c - -o $(BUILD_DIR)/.br-toolchain-test.tmp >/dev/null 2>&1 ; then \
> > +			echo "SSP option $${__SSP_OPTION} not available in this toolchain, please select another SSP level" ; \
> > +			exit 1 ; \
> > +		fi; \
> > +	fi; \
> >  	rm -f $(BUILD_DIR)/.br-toolchain-test.tmp*
> >  
> >  #
> > diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
> > index db3570d96f..00cbd7b17a 100644
> > --- a/toolchain/toolchain-external/pkg-toolchain-external.mk
> > +++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
> > @@ -549,7 +549,7 @@ define $(2)_CONFIGURE_CMDS
> >  	else \
> >  		$$(call check_glibc,$$$${SYSROOT_DIR}) ; \
> >  	fi
> > -	$$(Q)$$(call check_toolchain_ssp,$$(TOOLCHAIN_EXTERNAL_CC))
> > +	$$(Q)$$(call check_toolchain_ssp,$$(TOOLCHAIN_EXTERNAL_CC),$(BR2_SSP_OPTION))
> >  endef
> >  
> >  $(2)_TOOLCHAIN_WRAPPER_ARGS += $$(TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS)
> > 

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
| +33 534.541.179 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
|      yann.morin (at) orange.com      |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

[Buildroot] [PATCH 3/5 v2] toolchain: check the SSP option is known

[Thomas Petazzoni]

Hello,

On Tue, 12 Mar 2019 13:09:34 +0100
<yann.morin@orange.com> wrote:

>  # $1: cross-gcc path
> +# $2: gcc ssp option
>  #
>  check_toolchain_ssp = \
>  	__CROSS_CC=$(strip $1) ; \
> @@ -427,6 +428,13 @@ check_toolchain_ssp = \
>  		echo "SSP support not available in this toolchain, please disable BR2_TOOLCHAIN_EXTERNAL_HAS_SSP" ; \
>  		exit 1 ; \
>  	fi ; \
> +	__SSP_OPTION=$(2); \
> +	if [ -n "$${__SSP_OPTION}" ] ; then \
> +		if ! echo 'void main(){}' | $${__CROSS_CC} -Werror $${__SSP_OPTION} -x c - -o $(BUILD_DIR)/.br-toolchain-test.tmp >/dev/null 2>&1 ; then \
> +			echo "SSP option $${__SSP_OPTION} not available in this toolchain, please select another SSP level" ; \
> +			exit 1 ; \
> +		fi; \
> +	fi; \

Does it really make sense to check for both -fstack-protector (in the
code that already exists) *and* BR2_SSP_OPTION ?

Wouldn't it make more sense to just check for BR2_SSP_OPTION ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 3/5 v2] toolchain: check the SSP option is known

[yann.morin at orange.com]

Thomas, All,

On 2019-03-26 20:26 +0100, Thomas Petazzoni spake thusly:
> On Tue, 12 Mar 2019 13:09:34 +0100
> <yann.morin@orange.com> wrote:
> 
> >  # $1: cross-gcc path
> > +# $2: gcc ssp option
> >  #
> >  check_toolchain_ssp = \
> >  	__CROSS_CC=$(strip $1) ; \
> > @@ -427,6 +428,13 @@ check_toolchain_ssp = \
> >  		echo "SSP support not available in this toolchain, please disable BR2_TOOLCHAIN_EXTERNAL_HAS_SSP" ; \
> >  		exit 1 ; \
> >  	fi ; \
> > +	__SSP_OPTION=$(2); \
> > +	if [ -n "$${__SSP_OPTION}" ] ; then \
> > +		if ! echo 'void main(){}' | $${__CROSS_CC} -Werror $${__SSP_OPTION} -x c - -o $(BUILD_DIR)/.br-toolchain-test.tmp >/dev/null 2>&1 ; then \
> > +			echo "SSP option $${__SSP_OPTION} not available in this toolchain, please select another SSP level" ; \
> > +			exit 1 ; \
> > +		fi; \
> > +	fi; \
> 
> Does it really make sense to check for both -fstack-protector (in the
> code that already exists) *and* BR2_SSP_OPTION ?
> 
> Wouldn't it make more sense to just check for BR2_SSP_OPTION ?

My reasoning behind this additional test, is that there are two things
we need to test:

  * does the toolchain have SSP support (or not) as claimed by the user?

  * does the toolchain actually support the SSP level requested by the
    user?

There are packages that, wrongly or rightfully so, are not concerned
with te SSP level, but just the fact that there is SSP or not.

So, I decided to keep the existing test, and expand it. It means that we
may indeed check for -fstack-protector twice.

Regards,
Yann E. MORIN.

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
| +33 534.541.179 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
|      yann.morin (at) orange.com      |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

[Buildroot] [PATCH 3/5 v2] toolchain: check the SSP option is known

[Arnout Vandecappelle]

On 12/03/2019 13:09, yann.morin at orange.com wrote:
> From: "Yann E. MORIN" <yann.morin@orange.com>
> 
> Some toolchain vendors may have backported those options to older gcc
> versions, and we have no way to know, so we have to check that the
> user's selection is acceptable.
> 
> Extend the macro that currently checks for SSP in the toolchain, with
> a new test that the actual SSP option is recognised and accepted.
> 
> Note that the SSP option is either totaly empty, or an already-quoted
> string, so we can safely and easily assign it to a shell variable to
> test and use it.
> 
> Note that we do not introduce BR2_TOOLCHAIN_HAS_SSP_STRONG, because:
> 
>   - our internal toolchain infra only supports gcc >= 4.9, so it has
>     SSP strong;
> 
>   - of the external pre-built toolchains, only the codesourcery-arm
>     one has a gcc-4.8 which lacks SSP strong, all the others have a
>     gcc >= 4.9;
> 
>   - we'd still have to do the actual check for custom external
>     toolchains anyway.
> 
> So, we're not adding BR2_TOOLCHAIN_HAS_SSP_STRONG just for a single
> case.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
> Cc: Matt Weber <matthew.weber@rockwellcollins.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>

 You replied to all concerns, so I've applied as-is.

 Regards,
 Arnout

[Buildroot] [PATCH 4/5 v2] toolchain: -fstack-protector-strong can be back-ported

[yann.morin at orange.com]

From: "Yann E. MORIN" <yann.morin@orange.com>

Currently, use of -fstack-protector-strong is only available for gcc
starting with 4.9, on the assumption that it appeared with that version.

Although this is true, it happens that quite a few vendors will have
back-ported -fstack-protector-strong to older gcc versions (at least 4.8
seen in the wild).

Remove the guard against gcc>=4.9, and expand the help text.

Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
Cc: Matt Weber <matthew.weber@rockwellcollins.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

---
Notes:

We could have changed the guard to something like:
    depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_9 || BR2_TOOLCHAIN_EXTERNAL

However, the latest gcc we support in the internal toolchain *is*
gcc-4.9, so the condition would have always been true. Hence, we just
drop the condition.
---
 Config.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Config.in b/Config.in
index 757ad1ca40..d5a0460f98 100644
--- a/Config.in
+++ b/Config.in
@@ -746,14 +746,14 @@ config BR2_SSP_REGULAR
 
 config BR2_SSP_STRONG
 	bool "-fstack-protector-strong"
-	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_9
 	help
 	  Like -fstack-protector but includes additional functions to be
 	  protected - those that have local array definitions, or have
 	  references to local frame addresses.
 
-comment "Stack Smashing Protection strong needs a toolchain w/ gcc >= 4.9"
-	depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_9
+	  -fstack-protector-strong officially appeared in gcc 4.9, but
+	  some vendors have backported -fstack-protector-strong to older
+	  versions of gcc.
 
 config BR2_SSP_ALL
 	bool "-fstack-protector-all"
-- 
2.17.1


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

[Buildroot] [PATCH 4/5 v2] toolchain: -fstack-protector-strong can be back-ported

[Arnout Vandecappelle]

On 12/03/2019 13:09, yann.morin at orange.com wrote:
> From: "Yann E. MORIN" <yann.morin@orange.com>
> 
> Currently, use of -fstack-protector-strong is only available for gcc
> starting with 4.9, on the assumption that it appeared with that version.
> 
> Although this is true, it happens that quite a few vendors will have
> back-ported -fstack-protector-strong to older gcc versions (at least 4.8
> seen in the wild).
> 
> Remove the guard against gcc>=4.9, and expand the help text.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
> Cc: Matt Weber <matthew.weber@rockwellcollins.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> ---
> Notes:
> 
> We could have changed the guard to something like:
>     depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_9 || BR2_TOOLCHAIN_EXTERNAL
> 
> However, the latest gcc we support in the internal toolchain *is*
> gcc-4.9, so the condition would have always been true. Hence, we just
> drop the condition.

 This note is in fact interesting, so I've added it to the commit message.
However, it would have been more appropriate (if we would have had the
condition) to make it

 	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_9 || BR2_TOOLCHAIN_EXTERNAL_CUSTOM

(because we know our predefined external toolchains <4.9 *don't* suport this
option). In fact, we only have one 4.8 predefined external toolchain: Sourcery
ARM. So for that one toolchain, it's not worth adding that condition. The
problem will anyway be caught by the check you just added, because it's also
executed for predefined external toolchains.

 Regards,
 Arnout

> ---
>  Config.in | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Config.in b/Config.in
> index 757ad1ca40..d5a0460f98 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -746,14 +746,14 @@ config BR2_SSP_REGULAR
>  
>  config BR2_SSP_STRONG
>  	bool "-fstack-protector-strong"
> -	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_9
>  	help
>  	  Like -fstack-protector but includes additional functions to be
>  	  protected - those that have local array definitions, or have
>  	  references to local frame addresses.
>  
> -comment "Stack Smashing Protection strong needs a toolchain w/ gcc >= 4.9"
> -	depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_9
> +	  -fstack-protector-strong officially appeared in gcc 4.9, but
> +	  some vendors have backported -fstack-protector-strong to older
> +	  versions of gcc.
>  
>  config BR2_SSP_ALL
>  	bool "-fstack-protector-all"
> 

[Buildroot] [PATCH 4/5 v2] toolchain: -fstack-protector-strong can be back-ported

[yann.morin at orange.com]

Arnout, All,

On 2019-08-03 23:16 +0200, Arnout Vandecappelle spake thusly:
> On 12/03/2019 13:09, yann.morin at orange.com wrote:
> > From: "Yann E. MORIN" <yann.morin@orange.com>
> > 
> > Currently, use of -fstack-protector-strong is only available for gcc
> > starting with 4.9, on the assumption that it appeared with that version.
> > 
> > Although this is true, it happens that quite a few vendors will have
> > back-ported -fstack-protector-strong to older gcc versions (at least 4.8
> > seen in the wild).
> > 
> > Remove the guard against gcc>=4.9, and expand the help text.
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
> > Cc: Matt Weber <matthew.weber@rockwellcollins.com>
> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > 
> > ---
> > Notes:
> > 
> > We could have changed the guard to something like:
> >     depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_9 || BR2_TOOLCHAIN_EXTERNAL
> > 
> > However, the latest gcc we support in the internal toolchain *is*
> > gcc-4.9, so the condition would have always been true. Hence, we just
> > drop the condition.
> 
>  This note is in fact interesting, so I've added it to the commit message.
> However, it would have been more appropriate (if we would have had the
> condition) to make it
> 
>  	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_9 || BR2_TOOLCHAIN_EXTERNAL_CUSTOM

Actually, this would no longer be true, since we recently merged support
for preconfigured toolchains from br2-external trees, and such toolchain
may well be a gcc-4.8 or earlier with back-ported SSP strong.

Regards,
Yann E. MORIN.

> (because we know our predefined external toolchains <4.9 *don't* suport this
> option). In fact, we only have one 4.8 predefined external toolchain: Sourcery
> ARM. So for that one toolchain, it's not worth adding that condition. The
> problem will anyway be caught by the check you just added, because it's also
> executed for predefined external toolchains.
> 
>  Regards,
>  Arnout
> 
> > ---
> >  Config.in | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Config.in b/Config.in
> > index 757ad1ca40..d5a0460f98 100644
> > --- a/Config.in
> > +++ b/Config.in
> > @@ -746,14 +746,14 @@ config BR2_SSP_REGULAR
> >  
> >  config BR2_SSP_STRONG
> >  	bool "-fstack-protector-strong"
> > -	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_9
> >  	help
> >  	  Like -fstack-protector but includes additional functions to be
> >  	  protected - those that have local array definitions, or have
> >  	  references to local frame addresses.
> >  
> > -comment "Stack Smashing Protection strong needs a toolchain w/ gcc >= 4.9"
> > -	depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_9
> > +	  -fstack-protector-strong officially appeared in gcc 4.9, but
> > +	  some vendors have backported -fstack-protector-strong to older
> > +	  versions of gcc.
> >  
> >  config BR2_SSP_ALL
> >  	bool "-fstack-protector-all"
> > 

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
| +33 534.541.179 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
|      yann.morin (at) orange.com      |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

[Buildroot] [PATCH 5/5 v2] toolchain: allow PIC/PIE without RELRO

[yann.morin at orange.com]

From: "Yann E. MORIN" <yann.morin@orange.com>

In commit 7484c1c3b806 (toolchain/toolchain-wrapper: add BR2_RELRO_),
we added the PIC/PIE flags, but based on the RELRO_FULL condition.

It is however totally possible to do a PIC/PIE executable without
RELRO_FULL, as it is also valid to do a PIC/PIE build with RELRO_PARTIAL.

Add a new option that now governs the PIC/PIE flags.

Note: it is unknown if RELRO_FULL really needs PIC/PIE or not, so we
keep the current situation, where RELRO-FULL forces PIC/PIE compilation.
Decoupling can come later from an interested party.

Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
Cc: Matt Weber <matthew.weber@rockwellcollins.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 Config.in                      | 8 ++++++++
 toolchain/toolchain-wrapper.c  | 2 +-
 toolchain/toolchain-wrapper.mk | 4 ++++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Config.in b/Config.in
index d5a0460f98..31fea3ab34 100644
--- a/Config.in
+++ b/Config.in
@@ -712,6 +712,13 @@ endmenu
 
 comment "Security Hardening Options"
 
+config BR2_PIC_PIE
+	bool "Build code with PIC/PIE"
+	depends on BR2_SHARED_LIBS
+	help
+	  Generate Position-Independent Code (PIC) and link
+	  Position-Independent Executables (PIE).
+
 choice
 	bool "Stack Smashing Protection"
 	default BR2_SSP_ALL if BR2_ENABLE_SSP # legacy
@@ -794,6 +801,7 @@ config BR2_RELRO_PARTIAL
 
 config BR2_RELRO_FULL
 	bool "Full"
+	select BR2_PIC_PIE
 	help
 	  This option includes the partial configuration, but also marks
 	  the GOT as read-only at the cost of initialization time during
diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
index c73a0cc079..7a4b9c4007 100644
--- a/toolchain/toolchain-wrapper.c
+++ b/toolchain/toolchain-wrapper.c
@@ -367,7 +367,7 @@ int main(int argc, char **argv)
 		*cur++ = "-Wno-builtin-macro-redefined";
 	}
 
-#ifdef BR2_RELRO_FULL
+#ifdef BR2_PIC_PIE
 	/* Patterned after Fedora/Gentoo hardening approaches.
 	 * https://fedoraproject.org/wiki/Changes/Harden_All_Packages
 	 * https://wiki.gentoo.org/wiki/Hardened/Toolchain#Position_Independent_Executables_.28PIEs.29
diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/toolchain-wrapper.mk
index ca66fa7ba4..3c42146cea 100644
--- a/toolchain/toolchain-wrapper.mk
+++ b/toolchain/toolchain-wrapper.mk
@@ -48,6 +48,10 @@ ifeq ($(BR2_CCACHE_USE_BASEDIR),y)
 TOOLCHAIN_WRAPPER_ARGS += -DBR_CCACHE_BASEDIR='"$(BASE_DIR)"'
 endif
 
+ifeq ($(BR2_PIC_PIE),y)
+TOOLCHAIN_WRAPPER_ARGS += -DBR2_PIC_PIE
+endif
+
 ifeq ($(BR2_RELRO_PARTIAL),y)
 TOOLCHAIN_WRAPPER_ARGS += -DBR2_RELRO_PARTIAL
 else ifeq ($(BR2_RELRO_FULL),y)
-- 
2.17.1


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

[Buildroot] [PATCH 5/5 v2] toolchain: allow PIC/PIE without RELRO

[Matthew Weber]

Yann,
On Tue, Mar 12, 2019 at 7:09 AM <yann.morin@orange.com> wrote:
>
> From: "Yann E. MORIN" <yann.morin@orange.com>
>
> In commit 7484c1c3b806 (toolchain/toolchain-wrapper: add BR2_RELRO_),
> we added the PIC/PIE flags, but based on the RELRO_FULL condition.
>
> It is however totally possible to do a PIC/PIE executable without
> RELRO_FULL, as it is also valid to do a PIC/PIE build with RELRO_PARTIAL.
>
> Add a new option that now governs the PIC/PIE flags.
>
> Note: it is unknown if RELRO_FULL really needs PIC/PIE or not, so we
> keep the current situation, where RELRO-FULL forces PIC/PIE compilation.
> Decoupling can come later from an interested party.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
> Cc: Matt Weber <matthew.weber@rockwellcollins.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  Config.in                      | 8 ++++++++
>  toolchain/toolchain-wrapper.c  | 2 +-
>  toolchain/toolchain-wrapper.mk | 4 ++++
>  3 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/Config.in b/Config.in
> index d5a0460f98..31fea3ab34 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -712,6 +712,13 @@ endmenu
>
>  comment "Security Hardening Options"
>
> +config BR2_PIC_PIE
> +       bool "Build code with PIC/PIE"
> +       depends on BR2_SHARED_LIBS
> +       help
> +         Generate Position-Independent Code (PIC) and link
> +         Position-Independent Executables (PIE).
> +
>  choice
>         bool "Stack Smashing Protection"
>         default BR2_SSP_ALL if BR2_ENABLE_SSP # legacy
> @@ -794,6 +801,7 @@ config BR2_RELRO_PARTIAL
>
>  config BR2_RELRO_FULL
>         bool "Full"
> +       select BR2_PIC_PIE

In the previous email chain it was being discussed if PIC/PIE was
required for full RELRO.  Like you guys mentioned, I believe it was
always just lumped into the configuration the other distros called
"full".  However, I'm pretty sure they are independent and you could
have full RELRO without PIC/PIE  (I did not test this theory, just
checked some docs and the theory holds).  I'd be on the fence if we
should remove this select and keep the BR2_PIC_PIE as something you'd
need to independently set if you want it with BR2_RELRO_FULL.  I
guess, better to make that sort of change now then after more time has
gone by.

>         help
>           This option includes the partial configuration, but also marks
>           the GOT as read-only at the cost of initialization time during
> diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
> index c73a0cc079..7a4b9c4007 100644
> --- a/toolchain/toolchain-wrapper.c
> +++ b/toolchain/toolchain-wrapper.c
> @@ -367,7 +367,7 @@ int main(int argc, char **argv)
>                 *cur++ = "-Wno-builtin-macro-redefined";
>         }
>
> -#ifdef BR2_RELRO_FULL
> +#ifdef BR2_PIC_PIE

Reviewed-by: Matthew Weber <matthew.weber@rockwellcollins.com>

[Buildroot] [PATCH 5/5 v2] toolchain: allow PIC/PIE without RELRO

[yann.morin at orange.com]

Matt, All,

On 2019-03-12 08:50 -0500, Matthew Weber spake thusly:
> On Tue, Mar 12, 2019 at 7:09 AM <yann.morin@orange.com> wrote:
> > From: "Yann E. MORIN" <yann.morin@orange.com>
> > In commit 7484c1c3b806 (toolchain/toolchain-wrapper: add BR2_RELRO_),
> > we added the PIC/PIE flags, but based on the RELRO_FULL condition.
[--SNIP--]
> >  config BR2_RELRO_FULL
> >         bool "Full"
> > +       select BR2_PIC_PIE
> 
> In the previous email chain it was being discussed if PIC/PIE was
> required for full RELRO.  Like you guys mentioned, I believe it was
> always just lumped into the configuration the other distros called
> "full".  However, I'm pretty sure they are independent and you could
> have full RELRO without PIC/PIE  (I did not test this theory, just
> checked some docs and the theory holds).

Arnout did test it, and it indeed works.

>  I'd be on the fence if we
> should remove this select and keep the BR2_PIC_PIE as something you'd
> need to independently set if you want it with BR2_RELRO_FULL.  I
> guess, better to make that sort of change now then after more time has
> gone by.

I did not do that in this patch, to introduce the minimal disruption
possible. I.e. a configuration which had relro-full will still get
PIC/PIE. This patch just adds the possiblity to do PIC/PIE without
relro-full.

If we really want to decouple the two, then I think we should do that in
a separate patch, which just drops this new select, to allow relro-full
without PIC/PIE.

I'll do that in a followup patch.

Regards,
Yann E. MORIN.

> >         help
> >           This option includes the partial configuration, but also marks
> >           the GOT as read-only at the cost of initialization time during
> > diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
> > index c73a0cc079..7a4b9c4007 100644
> > --- a/toolchain/toolchain-wrapper.c
> > +++ b/toolchain/toolchain-wrapper.c
> > @@ -367,7 +367,7 @@ int main(int argc, char **argv)
> >                 *cur++ = "-Wno-builtin-macro-redefined";
> >         }
> >
> > -#ifdef BR2_RELRO_FULL
> > +#ifdef BR2_PIC_PIE
> 
> Reviewed-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
| +33 534.541.179 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
|      yann.morin (at) orange.com      |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

[Buildroot] [PATCH 5/5 v2] toolchain: allow PIC/PIE without RELRO

[Arnout Vandecappelle]

On 12/03/2019 13:09, yann.morin at orange.com wrote:
> From: "Yann E. MORIN" <yann.morin@orange.com>
> 
> In commit 7484c1c3b806 (toolchain/toolchain-wrapper: add BR2_RELRO_),
> we added the PIC/PIE flags, but based on the RELRO_FULL condition.
> 
> It is however totally possible to do a PIC/PIE executable without
> RELRO_FULL, as it is also valid to do a PIC/PIE build with RELRO_PARTIAL.
> 
> Add a new option that now governs the PIC/PIE flags.
> 
> Note: it is unknown if RELRO_FULL really needs PIC/PIE or not, so we
> keep the current situation, where RELRO-FULL forces PIC/PIE compilation.
> Decoupling can come later from an interested party.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
> Cc: Matt Weber <matthew.weber@rockwellcollins.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

 Applied to master, thanks.

 Regards,
 Arnout