| Message ID | 20240419-ztext-v1-1-a8d5c2cfcf57@gmx.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Config.in: add option to ban textrels | expand |
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
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
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
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(+) --- base-commit: 256aa8ed85f8fd65ea0f0f242adb55f95a13eb2b change-id: 20240417-ztext-5accbab61c0a Best regards, -- J. Neuschäfer <j.neuschaefer@gmx.net>