Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] Config.in: add option to ban textrels
@ 2024-04-19 14:52 J. Neuschäfer via buildroot
  2024-05-09 15:40 ` Thomas Petazzoni via buildroot
  0 siblings, 1 reply; 3+ messages in thread
From: J. Neuschäfer via buildroot @ 2024-04-19 14:52 UTC (permalink / raw)
  To: buildroot; +Cc: J. Neuschäfer

Ideally, this option would default to yes when a musl-based toolchain is
used, as musl doesn't support TEXTRELs[1] and programs with TEXTRELs
will crash on start-up under musl.

However, to avoid a large build fallout, it defaults to "no" for now.
After this option has been enabled in build tests for a while and some
of the fallout has been fixed, the default can be changed.

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

Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>
---
 Config.in           | 13 +++++++++++++
 package/Makefile.in |  4 ++++
 2 files changed, 17 insertions(+)

diff --git a/Config.in b/Config.in
index e4f58f3f66..26964a6bac 100644
--- a/Config.in
+++ b/Config.in
@@ -906,6 +906,19 @@ endchoice
 comment "RELocation Read Only (RELRO) needs shared libraries"
 	depends on !BR2_SHARED_LIBS

+config BR2_LINK_ZTEXT
+	bool "Disallow text section relocations (TEXTRELs)"
+	depends on BR2_SHARED_LIBS
+	default n # ideally: BR2_TOOLCHAIN_USES_MUSL
+	help
+	  Pass "-z text" to the linker to detect TEXTRELs and throw an error if
+	  they occur.
+
+	  This is recommended when building a system with musl-libc, because
+	  TEXTRELs are not supported on musl-libc:
+
+	    https://www.openwall.com/lists/musl/2020/09/25/4
+
 config BR2_FORTIFY_SOURCE_ARCH_SUPPORTS
 	bool
 	default y
diff --git a/package/Makefile.in b/package/Makefile.in
index 3e276d23d6..de93defb3b 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -149,6 +149,10 @@ endif

 TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS))

+ifeq ($(BR2_LINK_ZTEXT),y)
+TARGET_LDFLAGS += -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

---
base-commit: 256aa8ed85f8fd65ea0f0f242adb55f95a13eb2b
change-id: 20240417-ztext-5accbab61c0a

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

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Buildroot] [PATCH] Config.in: add option to ban textrels
  2024-04-19 14:52 [Buildroot] [PATCH] Config.in: add option to ban textrels J. Neuschäfer via buildroot
@ 2024-05-09 15:40 ` Thomas Petazzoni via buildroot
  2024-05-10 13:26   ` J. Neuschäfer via buildroot
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Petazzoni via buildroot @ 2024-05-09 15:40 UTC (permalink / raw)
  To: J. Neuschäfer via buildroot; +Cc: J. Neuschäfer

Hello,

On Fri, 19 Apr 2024 16:52:50 +0200
J. Neuschäfer via buildroot <buildroot@buildroot.org> wrote:

> Ideally, this option would default to yes when a musl-based toolchain is
> used, as musl doesn't support TEXTRELs[1] and programs with TEXTRELs
> will crash on start-up under musl.
> 
> However, to avoid a large build fallout, it defaults to "no" for now.
> After this option has been enabled in build tests for a while and some
> of the fallout has been fixed, the default can be changed.
> 
> [1]: https://www.openwall.com/lists/musl/2020/09/25/4
> 
> Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>

Thanks for the patch. I am not fully sure I totally grasp the issue,
but I'm wondering if making it an option is really the right thing to
do. If indeed TEXTRELs don't work with musl, then we should simply
disable them.

One thing I'm not entirely clear is that
https://www.openwall.com/lists/musl/2020/09/25/4 states "I recently
discovered that musl doesn't support DT/DF_TEXTREL in the main
executable", while passing -z text will disallow TEXTREL everywhere,
not only "main executable" but also shared libraries. Is that intended?

Do you have an idea of the amount of breakage enabling -z text would
cause? Which particular package/situation did you encounter that was
using TEXTREL and shouldn't have in order to be compatible with musl?

Thanks,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Buildroot] [PATCH] Config.in: add option to ban textrels
  2024-05-09 15:40 ` Thomas Petazzoni via buildroot
@ 2024-05-10 13:26   ` J. Neuschäfer via buildroot
  0 siblings, 0 replies; 3+ messages in thread
From: J. Neuschäfer via buildroot @ 2024-05-10 13:26 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: J. Neuschäfer, J. Neuschäfer via buildroot


[-- Attachment #1.1: Type: text/plain, Size: 3607 bytes --]

On Thu, May 09, 2024 at 05:40:57PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri, 19 Apr 2024 16:52:50 +0200
> J. Neuschäfer via buildroot <buildroot@buildroot.org> wrote:
> 
> > Ideally, this option would default to yes when a musl-based toolchain is
> > used, as musl doesn't support TEXTRELs[1] and programs with TEXTRELs
> > will crash on start-up under musl.
> > 
> > However, to avoid a large build fallout, it defaults to "no" for now.
> > After this option has been enabled in build tests for a while and some
> > of the fallout has been fixed, the default can be changed.
> > 
> > [1]: https://www.openwall.com/lists/musl/2020/09/25/4
> > 
> > Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>
> 
> Thanks for the patch. I am not fully sure I totally grasp the issue,
> but I'm wondering if making it an option is really the right thing to
> do. If indeed TEXTRELs don't work with musl, then we should simply
> disable them.

Fair enough. My intention here was to soften the blow somewhat with
regards to how much breakage will be seen in the autobuilders, but two
points run counter to that idea:

- Any binaries built with textrels on musl would be broken anyway, and
  the current situation just hides the breakage by deferring it until
  runtime.
- I'm considering to try an allyesconfig build locally to see what breaks.
  Not sure if it's a good idea though, given the sheer number of packages :)

> 
> One thing I'm not entirely clear is that
> https://www.openwall.com/lists/musl/2020/09/25/4 states "I recently
> discovered that musl doesn't support DT/DF_TEXTREL in the main
> executable", while passing -z text will disallow TEXTREL everywhere,
> not only "main executable" but also shared libraries. Is that intended?

Hm, that's a detail that I haven't thought about.
I'm not sure if textrels in shared libraries even happen in practice.

musl's situtation is as follows, as far as I can piece it together:

- Version 0.7.12 (released in 2011) claims support for textrels in
  shared objects[1]. The code that added it (in commit v0.7.11-1-g9f17413c
  "textrel support, cheap and ugly"[2]) is still present essentially
  unchanged, but I don't understand how it works[3].
- The email from Rick Felker states it's only for legacy reasons and on
  a few architectures.
- Commit v1.1.9-20-gdc031ee0 ("add rcrt1 start file for fully static-linked PIE")[4]
  states: "all libraries being linked must be built as PIC/PIE; TEXTRELs
  are not supported at this time."

In summary, I think it's safe to forbid textrels on musl in general.


> Do you have an idea of the amount of breakage enabling -z text would
> cause? Which particular package/situation did you encounter that was
> using TEXTREL and shouldn't have in order to be compatible with musl?

I haven't investigated the total amount of breakage. The package that
prompted this change was micropython, which I recently fixed upstream[5].


Best regards,
- jn

[1]: https://git.musl-libc.org/cgit/musl/tree/WHATSNEW#n272
[2]: https://git.musl-libc.org/cgit/musl/commit/?id=9f17413c753030811761d576fdb95f200bd818d7
[3]:    for (i=0; ((size_t *)(base+dyn))[i]; i+=2)
                if (((size_t *)(base+dyn))[i]==DT_TEXTREL) {
                        mprotect(map, map_len, PROT_READ|PROT_WRITE|PROT_EXEC);
                        break;
                }
[4]: https://git.musl-libc.org/cgit/musl/commit/?id=dc031ee0b1ba11baa00cd7f0769e461a5f396c71
[5]: https://github.com/micropython/micropython/commit/7b050b366b7dacfb43779c51702a892d8f1873d0

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-05-10 13:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-19 14:52 [Buildroot] [PATCH] Config.in: add option to ban textrels J. Neuschäfer via buildroot
2024-05-09 15:40 ` Thomas Petazzoni via buildroot
2024-05-10 13:26   ` J. Neuschäfer via buildroot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox