diff mbox series

[v4] Config.in: ban textrels on musl toolchains

Message ID 20240718-ztext-v4-1-15a9744f1edb@gmx.net
State Superseded
Headers show
Series [v4] Config.in: ban textrels on musl toolchains | expand

Commit Message

J. Neuschäfer July 18, 2024, 8:33 p.m. UTC 
musl-libc doesn't support TEXTRELs[1] and programs with TEXTRELs will
crash on start-up under musl.

This patch forbids the use of TEXTRELs on musl toolchains with dynamic
linking.

To verify this patch:

- Delete package/micropython/0001-py-nlrthumb-Make-non-Thumb2-long-jump-workaround-opt.patch
- Build micropython (before v1.23) with a musl toolchain and
  BR2_SHARED_LIBS. The build should abort while linking micropython.

[1]: https://www.openwall.com/lists/musl/2020/09/25/4

Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>
---
Changes in v4:
- Remove Config.in option, implement the whole logic in
  package/Makefile.in
- Document how to verify
- Link to v3: https://lore.kernel.org/r/20240704-ztext-v3-1-9195d06c8bd0@gmx.net

Changes in v3:
- drop micropython patch (already merged)
- rewrite to positive logic, as suggested by Yann E. MORIN
- Link to v2: https://lore.kernel.org/r/20240529-ztext-v2-0-82985032f169@gmx.net

Changes in v2:
- Slightly different wording
- Enable the option by default on musl toolchains
- Add patch to fix build of micropython

Link to v1:
- https://lore.kernel.org/r/20240419-ztext-v1-1-a8d5c2cfcf57@gmx.net
---
---
 package/Makefile.in | 9 +++++++++
 1 file changed, 9 insertions(+)


---
base-commit: c624eee120b6ee0c81e636bd47abe92452610cf7
change-id: 20240417-ztext-5accbab61c0a

Best regards,
--
J. Neuschäfer <j.neuschaefer@gmx.net>

Comments

J. Neuschäfer July 19, 2024, 6:49 a.m. UTC | #1 
On Thu, Jul 18, 2024 at 10:33:23PM +0200, J. Neuschäfer wrote:
> +ifeq ($(BR2_TOOLCHAIN_USES_MUSL)$(BR2_BINFMT_SUPPORTS_SHARED),yy)

I just noticed that BR2_BINFMT_SUPPORTS_SHARED is incorrect, because
it's true regardless of whether shared libraries are actually used.
What I was aiming for is "BR2_SHARED_LIBS or BR2_SHARED_STATIC_LIBS".

I suppose this should work:

ifeq ($(BR2_TOOLCHAIN_USES_MUSL)$(BR2_SHARED_LIBS)$(BR2_SHARED_STATIC_LIBS),yy)

... but I find it a bit unclear because it technically doesn't limit
which two of the three options have to be true (this is done elsewhere,
in the Kconfig logic that makes BR2_SHARED_LIBS and BR2_SHARED_STATIC_LIBS
mutually exclusive). I'll rather go for two ifeqs in this case:

ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y)
ifeq ($(BR2_SHARED_LIBS)$(BR2_SHARED_STATIC_LIBS),y)


-- jn
Arnout Vandecappelle July 19, 2024, 7:19 a.m. UTC | #2 
On 19/07/2024 08:49, J. Neuschäfer wrote:
> On Thu, Jul 18, 2024 at 10:33:23PM +0200, J. Neuschäfer wrote:
>> +ifeq ($(BR2_TOOLCHAIN_USES_MUSL)$(BR2_BINFMT_SUPPORTS_SHARED),yy)
> 
> I just noticed that BR2_BINFMT_SUPPORTS_SHARED is incorrect, because
> it's true regardless of whether shared libraries are actually used.
> What I was aiming for is "BR2_SHARED_LIBS or BR2_SHARED_STATIC_LIBS".
> 
> I suppose this should work:
> 
> ifeq ($(BR2_TOOLCHAIN_USES_MUSL)$(BR2_SHARED_LIBS)$(BR2_SHARED_STATIC_LIBS),yy)

  What we do is:

ifeq ($(BR2_TOOLCHAIN_USES_MUSL):$(BR2_STATIC_LIBS),y:)

  This says that musl must be y and static must not be.

  Regards,
  Arnout

> 
> ... but I find it a bit unclear because it technically doesn't limit
> which two of the three options have to be true (this is done elsewhere,
> in the Kconfig logic that makes BR2_SHARED_LIBS and BR2_SHARED_STATIC_LIBS
> mutually exclusive). I'll rather go for two ifeqs in this case:
> 
> ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y)
> ifeq ($(BR2_SHARED_LIBS)$(BR2_SHARED_STATIC_LIBS),y)
> 
> 
> -- jn
J. Neuschäfer July 19, 2024, 1:43 p.m. UTC | #3 
On Fri, Jul 19, 2024 at 09:19:22AM +0200, Arnout Vandecappelle wrote:
> On 19/07/2024 08:49, J. Neuschäfer wrote:
> > On Thu, Jul 18, 2024 at 10:33:23PM +0200, J. Neuschäfer wrote:
> > > +ifeq ($(BR2_TOOLCHAIN_USES_MUSL)$(BR2_BINFMT_SUPPORTS_SHARED),yy)
> >
> > I just noticed that BR2_BINFMT_SUPPORTS_SHARED is incorrect, because
> > it's true regardless of whether shared libraries are actually used.
> > What I was aiming for is "BR2_SHARED_LIBS or BR2_SHARED_STATIC_LIBS".
> >
> > I suppose this should work:
> >
> > ifeq ($(BR2_TOOLCHAIN_USES_MUSL)$(BR2_SHARED_LIBS)$(BR2_SHARED_STATIC_LIBS),yy)
>
>  What we do is:
>
> ifeq ($(BR2_TOOLCHAIN_USES_MUSL):$(BR2_STATIC_LIBS),y:)
>
>  This says that musl must be y and static must not be.

Nice trick, I'll use it.
diff mbox series

Patch

diff --git a/package/Makefile.in b/package/Makefile.in
index 47a89f1ae1..3587662014 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -149,6 +149,15 @@  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
+ifeq ($(BR2_TOOLCHAIN_USES_MUSL)$(BR2_BINFMT_SUPPORTS_SHARED),yy)
+TARGET_LDFLAGS += -Wl,-z,text
+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