diff mbox series

[3/3] toolchain/wrapper: move -ztext from LDFLAGS to toolchain wrapper

Message ID 8402ba3c5551505159a155c5cf0fc5b58c6a3614.1723543467.git.yann.morin@orange.com
State New
Headers show
Series [1/3] toolchain/wrapper: check unsafe paths earlier | expand

Commit Message

Yann E. MORIN Aug. 13, 2024, 10:04 a.m. UTC 
From: "Yann E. MORIN" <yann.morin@orange.com>

Passing linker flags via LDFLAGS is notoriously fragile: packages either
use LDFLAGS with the gcc frontend, or directly with ld. This means that
care must be taken to only pass flags that are recognised by both gcc
and ld, which usually proves a bit challenging, as bd035872be99
(package/Makefile.in: Use "-z text" instead of "-Wl, -z, text")
demonstrates (later ammended by 3a39e706a91a).

It turns out that we already have a better, robust way of passing
LDFLAGS via the gcc frotned: the toolchain wrapper, which we already use
to pass relro for example.

We can revert back to using -Wl,-z,text, as we're sure whis is going to
be a call to the gcc frontend (the wrapper is not used for ld), which
makes it consistent with the other similar args for -z relro and -z now.

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
Cc: J. Neuschäfer <j.neuschaefer@gmx.net>
Cc: Romain Naour <romain.naour@gmail.com>
Cc: Giulio Benetti <giulio.benetti@benettiengineering.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 package/Makefile.in            | 19 -------------------
 toolchain/toolchain-wrapper.c  |  6 +++++-
 toolchain/toolchain-wrapper.mk |  9 +++++++++
 3 files changed, 14 insertions(+), 20 deletions(-)

Comments

Yann E. MORIN Aug. 14, 2024, 5:54 a.m. UTC | #1 
All,

On 2024-08-13 12:04 +0200, yann.morin@orange.com spake thusly:
> From: "Yann E. MORIN" <yann.morin@orange.com>
> 
> Passing linker flags via LDFLAGS is notoriously fragile: packages either
> use LDFLAGS with the gcc frontend, or directly with ld. This means that
> care must be taken to only pass flags that are recognised by both gcc
> and ld, which usually proves a bit challenging, as bd035872be99
> (package/Makefile.in: Use "-z text" instead of "-Wl, -z, text")
> demonstrates (later ammended by 3a39e706a91a).
> 
> It turns out that we already have a better, robust way of passing
> LDFLAGS via the gcc frotned: the toolchain wrapper, which we already use
> to pass relro for example.
> 
> We can revert back to using -Wl,-z,text, as we're sure whis is going to
> be a call to the gcc frontend (the wrapper is not used for ld), which
> makes it consistent with the other similar args for -z relro and -z now.
> 
> Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
> Cc: J. Neuschäfer <j.neuschaefer@gmx.net>
> Cc: Romain Naour <romain.naour@gmail.com>
> Cc: Giulio Benetti <giulio.benetti@benettiengineering.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Markus provided their tested-by tag in another thread:
    https://lore.kernel.org/buildroot/CAGt4E5u3UeGDLp_D1SgedQgXvcN2mGxSF3ExX+e0KKqDMMTVzw@mail.gmail.com/

Tested-by: Markus Mayer <mmayer@broadcom.com>

Regards,
Yann E. MORIN.
diff mbox series

Patch

diff --git a/package/Makefile.in b/package/Makefile.in
index 808b71a93e..47a89f1ae1 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -149,25 +149,6 @@  endif
 
 TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS))
 
-# musl's dynamic loader doesn't support DT_TEXTREL, which results in a runtime
-# crash if it gets used. The "-z text" linker option issues a build-time error
-# when DT_TEXREL is used, so we capture the problem earlier.
-#
-# See also: https://www.openwall.com/lists/musl/2020/09/25/4
-#
-# NOTE: We're using "-ztext" instead of "-Wl,-z,text" here, because some
-# packages pass TARGET_LDFLAGS directly to ld rather than gcc, and ld doesn't
-# support -Wl,[...]. -z is supported by both gcc and clang, so it probably
-# won't cause us problems.
-#
-# We're using "-ztext" instead of "-z text" here, because some buildsystems
-# (like scons, for gpsd) will reorder and/or drop LDFLAGS, causing a lone
-# "-z" to be passed and the "text" keyword to be dropped otherwise. Both
-# gcc and ld supports that, so it probably won't cause us problems.
-ifeq ($(BR2_TOOLCHAIN_USES_MUSL):$(BR2_STATIC_LIBS),y:)
-TARGET_LDFLAGS += -ztext
-endif
-
 # By design, _FORTIFY_SOURCE requires gcc optimization to be enabled.
 # Therefore, we need to pass _FORTIFY_SOURCE and the optimization level
 # through the same mechanism, i.e currently through CFLAGS. Passing
diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
index 7647a1a12d..de54f838ec 100644
--- a/toolchain/toolchain-wrapper.c
+++ b/toolchain/toolchain-wrapper.c
@@ -51,10 +51,11 @@  static char _date_[sizeof("-D__DATE__=\"MMM DD YYYY\"")];
  * 	-Wno-builtin-macro-redefined
  * 	-Wl,-z,now
  * 	-Wl,-z,relro
+ * 	-Wl,-z,text
  * 	-fPIE
  * 	-pie
  */
-#define EXCLUSIVE_ARGS	10
+#define EXCLUSIVE_ARGS	11
 
 static char *predef_args[] = {
 #ifdef BR_CCACHE
@@ -486,6 +487,9 @@  int main(int argc, char **argv)
 			break;
 	}
 	if (i == argc) {
+#ifdef BR2_ZTEXT
+		*cur++ = "-Wl,-z,text";
+#endif
 		/* https://wiki.gentoo.org/wiki/Hardened/Toolchain#Mark_Read-Only_Appropriate_Sections */
 #ifdef BR2_RELRO_PARTIAL
 		*cur++ = "-Wl,-z,relro";
diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/toolchain-wrapper.mk
index cbf46f15fa..d605e41f2c 100644
--- a/toolchain/toolchain-wrapper.mk
+++ b/toolchain/toolchain-wrapper.mk
@@ -83,6 +83,15 @@  else ifeq ($(BR2_RELRO_FULL),y)
 TOOLCHAIN_WRAPPER_ARGS += -DBR2_RELRO_FULL
 endif
 
+# musl's dynamic loader doesn't support DT_TEXTREL, which results in a runtime
+# crash if it gets used. The "-z text" linker option issues a build-time error
+# when DT_TEXREL is used, so we capture the problem earlier.
+#
+# See also: https://www.openwall.com/lists/musl/2020/09/25/4
+ifeq ($(BR2_TOOLCHAIN_USES_MUSL):$(BR2_STATIC_LIBS),y:)
+TOOLCHAIN_WRAPPER_ARGS += -DBR2_ZTEXT
+endif
+
 define TOOLCHAIN_WRAPPER_BUILD
 	$(HOSTCC) $(HOST_CFLAGS) $(TOOLCHAIN_WRAPPER_ARGS) \
 		-s -Wl,--hash-style=$(TOOLCHAIN_WRAPPER_HASH_STYLE) \