[Buildroot] [PATCH] valgrind: Force -fno-stack-protector in CFLAGS

[Buildroot] [PATCH] valgrind: Force -fno-stack-protector in CFLAGS

[Matt Weber]

From: Brandon Maier <brandon.maier@rockwellcollins.com>

Valgrind must be compiled with no stack protection. Valgrind defaults
CFLAGS to -fno-stack-protector, but Buildroot's CFLAGS overrides it.

Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com>
Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
---
 package/valgrind/valgrind.mk | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/package/valgrind/valgrind.mk b/package/valgrind/valgrind.mk
index 087a381..b9cd947 100644
--- a/package/valgrind/valgrind.mk
+++ b/package/valgrind/valgrind.mk
@@ -27,7 +27,11 @@ VALGRIND_AUTORECONF = YES
 # and pass the right -march option, so they take precedence over
 # Valgrind's wrongfully detected value.
 ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el),y)
-VALGRIND_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -march=$(BR2_GCC_TARGET_ARCH)"
+VALGRIND_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -fno-stack-protector -march=$(BR2_GCC_TARGET_ARCH)"
+else
+# Valgrind must be compiled with no stack protection. Valgrind defaults
+# CFLAGS to -fno-stack-protector, but Buildroot's CFLAGS overrides it.
+VALGRIND_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -fno-stack-protector"
 endif
 
 # On ARM, Valgrind only supports ARMv7, and uses the arch part of the
-- 
1.9.1

[Buildroot] [PATCH] valgrind: Force -fno-stack-protector in CFLAGS

[Thomas Petazzoni]

Hello,

On Tue, 20 Dec 2016 22:10:24 -0600, Matt Weber wrote:
> From: Brandon Maier <brandon.maier@rockwellcollins.com>
> 
> Valgrind must be compiled with no stack protection. Valgrind defaults
> CFLAGS to -fno-stack-protector, but Buildroot's CFLAGS overrides it.
> 
> Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com>
> Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
> ---
>  package/valgrind/valgrind.mk | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/package/valgrind/valgrind.mk b/package/valgrind/valgrind.mk
> index 087a381..b9cd947 100644
> --- a/package/valgrind/valgrind.mk
> +++ b/package/valgrind/valgrind.mk
> @@ -27,7 +27,11 @@ VALGRIND_AUTORECONF = YES
>  # and pass the right -march option, so they take precedence over
>  # Valgrind's wrongfully detected value.
>  ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el),y)
> -VALGRIND_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -march=$(BR2_GCC_TARGET_ARCH)"
> +VALGRIND_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -fno-stack-protector -march=$(BR2_GCC_TARGET_ARCH)"
> +else
> +# Valgrind must be compiled with no stack protection. Valgrind defaults
> +# CFLAGS to -fno-stack-protector, but Buildroot's CFLAGS overrides it.
> +VALGRIND_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -fno-stack-protector"
>  endif

I think you should state "Buildroot *may* override it". Indeed, we
only pass -fstack-protector if you have
BR2_SSP_{REGULAR,STRONG,BR2_SSP_ALL} enabled.

Other than that, could you refactor this the following way:

# Valgrind must be compiled with no stack protection, so forcefully
# pass -fno-stack-protector to override what Buildroot may have in
# TARGET_CFLAGS if SSP support is enabled.
VALGRIND_CFLAGS = \
	$(TARGET_CFLAGS) \
	-fno-stack-protector

# blblabla mips stuff
ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el),y)
VALGRIND_CFLAGS += -march=$(BR2_GCC_TARGET_ARCH)
endif

VALGRIND_CONF_ENV = CFLAGS="$(VALGRIND_CFLAGS)"

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH] valgrind: Force -fno-stack-protector in CFLAGS

[Matthew Weber]

Thomas,

On Wed, Dec 21, 2016 at 3:09 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Tue, 20 Dec 2016 22:10:24 -0600, Matt Weber wrote:
>> From: Brandon Maier <brandon.maier@rockwellcollins.com>
>>
>> Valgrind must be compiled with no stack protection. Valgrind defaults
>> CFLAGS to -fno-stack-protector, but Buildroot's CFLAGS overrides it.
>>
>> Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com>
>> Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
>> ---
>>  package/valgrind/valgrind.mk | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/package/valgrind/valgrind.mk b/package/valgrind/valgrind.mk
>> index 087a381..b9cd947 100644
>> --- a/package/valgrind/valgrind.mk
>> +++ b/package/valgrind/valgrind.mk
>> @@ -27,7 +27,11 @@ VALGRIND_AUTORECONF = YES
>>  # and pass the right -march option, so they take precedence over
>>  # Valgrind's wrongfully detected value.
>>  ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el),y)
>> -VALGRIND_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -march=$(BR2_GCC_TARGET_ARCH)"
>> +VALGRIND_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -fno-stack-protector -march=$(BR2_GCC_TARGET_ARCH)"
>> +else
>> +# Valgrind must be compiled with no stack protection. Valgrind defaults
>> +# CFLAGS to -fno-stack-protector, but Buildroot's CFLAGS overrides it.
>> +VALGRIND_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -fno-stack-protector"
>>  endif
>
> I think you should state "Buildroot *may* override it". Indeed, we
> only pass -fstack-protector if you have
> BR2_SSP_{REGULAR,STRONG,BR2_SSP_ALL} enabled.
>
> Other than that, could you refactor this the following way:
>
> # Valgrind must be compiled with no stack protection, so forcefully
> # pass -fno-stack-protector to override what Buildroot may have in
> # TARGET_CFLAGS if SSP support is enabled.
> VALGRIND_CFLAGS = \
>         $(TARGET_CFLAGS) \
>         -fno-stack-protector
>
> # blblabla mips stuff
> ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el),y)
> VALGRIND_CFLAGS += -march=$(BR2_GCC_TARGET_ARCH)
> endif
>
> VALGRIND_CONF_ENV = CFLAGS="$(VALGRIND_CFLAGS)"
>

Thanks for the review, we'll update the description and these couple changes.

-- 
Matthew L Weber / Pr Software Engineer
Airborne Information Systems / Security Systems and Software / Secure Platforms
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
www.rockwellcollins.com

Note: Any Export License Required Information and License Restricted
Third Party Intellectual Property (TPIP) content must be encrypted and
sent to matthew.weber at corp.rockwellcollins.com.

[Buildroot] [PATCH] valgrind: Force -fno-stack-protector in CFLAGS

[Matt Weber]

From: Brandon Maier <brandon.maier@rockwellcollins.com>

Valgrind must be compiled with no stack protection. Valgrind defaults
CFLAGS to -fno-stack-protector, but Buildroot's CFLAGS may override
if SSP is enabled.

Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com>
---

v1 -> v2
[Thomas P
  - Update language to state Buildroot may enable SSP
  - Use a local temp VALGRIND_CFLAGS to build out CFLAGS used
    in VALGRIND_CONF_ENV

Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
---
 package/valgrind/valgrind.mk | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/package/valgrind/valgrind.mk b/package/valgrind/valgrind.mk
index 087a381..652432f 100644
--- a/package/valgrind/valgrind.mk
+++ b/package/valgrind/valgrind.mk
@@ -17,6 +17,13 @@ VALGRIND_INSTALL_STAGING = YES
 # patch 0004-Fixes-for-musl-libc.patch touching configure.ac
 VALGRIND_AUTORECONF = YES
 
+# Valgrind must be compiled with no stack protection, so forcefully
+# pass -fno-stack-protector to override what Buildroot may have in
+# TARGET_CFLAGS if BR2_SSP_* support is enabled.
+VALGRIND_CFLAGS = \
+	$(TARGET_CFLAGS) \
+	-fno-stack-protector
+
 # When Valgrind detects a 32-bit MIPS architecture, it forcibly adds
 # -march=mips32 to CFLAGS; when it detects a 64-bit MIPS architecture,
 # it forcibly adds -march=mips64. This causes Valgrind to be built
@@ -27,9 +34,11 @@ VALGRIND_AUTORECONF = YES
 # and pass the right -march option, so they take precedence over
 # Valgrind's wrongfully detected value.
 ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el),y)
-VALGRIND_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -march=$(BR2_GCC_TARGET_ARCH)"
+VALGRIND_CFLAGS += -march=$(BR2_GCC_TARGET_ARCH)
 endif
 
+VALGRIND_CONF_ENV = CFLAGS="$(VALGRIND_CFLAGS)"
+
 # On ARM, Valgrind only supports ARMv7, and uses the arch part of the
 # host tuple to determine whether it's being built for ARMv7 or
 # not. Therefore, we adjust the host tuple to specify we're on
-- 
1.9.1