diff mbox series

[2/6] package/Makefile.in: Add missing options to LDFLAGS for full RELRO build

Message ID 20180711143113.11927-3-matthew.weber@rockwellcollins.com
State Accepted
Headers show
Series Hardening Flag Bugfix/Enhancement | expand

Commit Message

Matt Weber July 11, 2018, 2:31 p.m. UTC 
From: Stefan Sørensen <stefan.sorensen@spectralink.com>

The options for a full RELRO build should also be added to LDFLAGS.

Originally submitted as
http://patchwork.ozlabs.org/patch/904034/

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
---
 package/Makefile.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Arnout Vandecappelle July 11, 2018, 9:26 p.m. UTC | #1 
On 11-07-18 16:31, Matt Weber wrote:
> From: Stefan Sørensen <stefan.sorensen@spectralink.com>
> 
> The options for a full RELRO build should also be added to LDFLAGS.
> 
> Originally submitted as
> http://patchwork.ozlabs.org/patch/904034/
> 
> Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
> Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
> ---
>  package/Makefile.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/Makefile.in b/package/Makefile.in
> index 5e0ff8c841..14b3bbd243 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -159,7 +159,7 @@ TARGET_HARDENED += $(TARGET_CFLAGS_RELRO)
>  TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO)
>  else ifeq ($(BR2_RELRO_FULL),y)
>  TARGET_HARDENED += -fPIE $(TARGET_CFLAGS_RELRO_FULL)
> -TARGET_LDFLAGS += -pie
> +TARGET_LDFLAGS += -pie $(TARGET_CFLAGS_RELRO_FULL)

 Actually, those flags (-Wl,-z,now,-z,relo) really are link-time flags only. So
really there is no reason why we would have them in TARGET_CFLAGS (i.e. in
TARGET_HARDENED). However, it is likely that there are packages that only apply
TARGET_CFLAGS to linking, not TARGET_LDFLAGS (we currently have no way of
knowing, since all of the LDFLAGS are also in CFLAGS, except for the hardening
ones).

 I have also considered to add TARGET_HARDENING to TARGET_LDFLAGS. However, it
is really only this -Wl,-z,... option which is relevant in LDFLAGS; the
-fstack-protector and -D_FORTIFY options can never have any effect. So this
looks good.

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 I have one little gripe, not directly related to this patch: I think the
variables TARGET_CFLAGS_RELRO and TARGET_CFLAGS_RELRO_FULL have little value,
they make IMO the code harder to read.

 Regards,
 Arnout
>  endif
>  
>  ifeq ($(BR2_FORTIFY_SOURCE_1),y)
>
Thomas Petazzoni Aug. 10, 2018, 8:33 p.m. UTC | #2 
Hello,

On Wed, 11 Jul 2018 09:31:09 -0500, Matt Weber wrote:
> From: Stefan Sørensen <stefan.sorensen@spectralink.com>
> 
> The options for a full RELRO build should also be added to LDFLAGS.
> 
> Originally submitted as
> http://patchwork.ozlabs.org/patch/904034/
> 
> Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
> Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
> ---
>  package/Makefile.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to next, thanks.

Thomas
diff mbox series

Patch

diff --git a/package/Makefile.in b/package/Makefile.in
index 5e0ff8c841..14b3bbd243 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -159,7 +159,7 @@  TARGET_HARDENED += $(TARGET_CFLAGS_RELRO)
 TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO)
 else ifeq ($(BR2_RELRO_FULL),y)
 TARGET_HARDENED += -fPIE $(TARGET_CFLAGS_RELRO_FULL)
-TARGET_LDFLAGS += -pie
+TARGET_LDFLAGS += -pie $(TARGET_CFLAGS_RELRO_FULL)
 endif
 
 ifeq ($(BR2_FORTIFY_SOURCE_1),y)