* [Buildroot] [PATCH v3] Config.in: ban textrels on musl toolchains @ 2024-07-04 18:05 J. Neuschäfer via buildroot 2024-07-15 8:51 ` Arnout Vandecappelle via buildroot 0 siblings, 1 reply; 4+ messages in thread From: J. Neuschäfer via buildroot @ 2024-07-04 18:05 UTC (permalink / raw) To: buildroot; +Cc: Yann E. MORIN, Thomas Petazzoni, J. Neuschäfer 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, but adds an option to either forbid them on other libcs as well, or allow them on musl. [1]: https://www.openwall.com/lists/musl/2020/09/25/4 Signed-off-by: J. Neuschäfer <j.neuschaefer@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 --- I'm not sure about the Kconfig help text for BR2_LINK_ALLOW_TEXTREL. Perhaps it could be written a little nicer. v3: - rewrite to positive logic, as suggested by Yann E. MORIN - adjust the name and help text accordingly v2: - slightly different wording - default to yes on musl toolchains - fix check-package warnings - depends on BR2_SHARED_LIBS -> depends on !BR2_STATIC_LIBS --- Config.in | 14 ++++++++++++++ package/Makefile.in | 4 ++++ 2 files changed, 18 insertions(+) diff --git a/Config.in b/Config.in index bdf5fa8647..c8b52c0d74 100644 --- a/Config.in +++ b/Config.in @@ -910,6 +910,20 @@ endchoice comment "RELocation Read Only (RELRO) needs shared libraries" depends on !BR2_SHARED_LIBS +config BR2_LINK_ALLOW_TEXTREL + bool "Allow text section relocations (TEXTRELs)" + default y # Legacy + depends on !BR2_TOOLCHAIN_USES_MUSL + depends on !BR2_STATIC_LIBS + help + Enable this option to allow TEXTRELs. Otherwise "-z text" will be + passed to the linker to detect them and throw an error if they occur. + + TEXTRELs are not supported on musl-libc's dynamic loader and will + result in a run-time crash: + + 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 5476234c7e..3f1a1d3a35 100644 --- a/package/Makefile.in +++ b/package/Makefile.in @@ -149,6 +149,10 @@ endif TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS)) +ifeq ($(BR2_LINK_ALLOW_TEXTREL),) +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: 3ebc7c69d56430c34eba4c869d1d4fe4d1e8de55 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] 4+ messages in thread
* Re: [Buildroot] [PATCH v3] Config.in: ban textrels on musl toolchains 2024-07-04 18:05 [Buildroot] [PATCH v3] Config.in: ban textrels on musl toolchains J. Neuschäfer via buildroot @ 2024-07-15 8:51 ` Arnout Vandecappelle via buildroot 2024-07-15 9:06 ` Thomas Petazzoni via buildroot 2024-07-17 11:52 ` J. Neuschäfer via buildroot 0 siblings, 2 replies; 4+ messages in thread From: Arnout Vandecappelle via buildroot @ 2024-07-15 8:51 UTC (permalink / raw) To: J. Neuschäfer, buildroot; +Cc: Yann E. MORIN, Thomas Petazzoni Hi J., [BTW are you OK with me calling you "J."? It feels a bit weird...] On 04/07/2024 20:05, J. Neuschäfer via buildroot wrote: > 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, but adds an option to > either forbid them on other libcs as well, or allow them on musl. > > [1]: https://www.openwall.com/lists/musl/2020/09/25/4 > > Signed-off-by: J. Neuschäfer <j.neuschaefer@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 > --- > > I'm not sure about the Kconfig help text for BR2_LINK_ALLOW_TEXTREL. > Perhaps it could be written a little nicer. > > > v3: > - rewrite to positive logic, as suggested by Yann E. MORIN > - adjust the name and help text accordingly > > v2: > - slightly different wording > - default to yes on musl toolchains > - fix check-package warnings > - depends on BR2_SHARED_LIBS -> depends on !BR2_STATIC_LIBS > --- > Config.in | 14 ++++++++++++++ > package/Makefile.in | 4 ++++ > 2 files changed, 18 insertions(+) > > diff --git a/Config.in b/Config.in > index bdf5fa8647..c8b52c0d74 100644 > --- a/Config.in > +++ b/Config.in > @@ -910,6 +910,20 @@ endchoice > comment "RELocation Read Only (RELRO) needs shared libraries" > depends on !BR2_SHARED_LIBS > > +config BR2_LINK_ALLOW_TEXTREL > + bool "Allow text section relocations (TEXTRELs)" > + default y # Legacy > + depends on !BR2_TOOLCHAIN_USES_MUSL > + depends on !BR2_STATIC_LIBS > + help > + Enable this option to allow TEXTRELs. Otherwise "-z text" will be > + passed to the linker to detect them and throw an error if they occur. > + > + TEXTRELs are not supported on musl-libc's dynamic loader and will > + result in a run-time crash: > + > + 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 5476234c7e..3f1a1d3a35 100644 > --- a/package/Makefile.in > +++ b/package/Makefile.in > @@ -149,6 +149,10 @@ endif > > TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS)) > > +ifeq ($(BR2_LINK_ALLOW_TEXTREL),) > +TARGET_LDFLAGS += -z text > +endif After some discussion, we concluded that we don't see the point of making this a user-settable Config.in option. It should be completely in the Makefile.in, with a comment explaining why. So something like this (untested!): # 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. ifeq ($(BR2_TOOLCHAIN_USES_MUSL)$(BR2_STATIC_LIBS),yy) TARGET_LDFLAGS += -#l,-z,text endif Note also that we use -Wl,-z,... everywhere else to pass the -z options to the linker. Maybe that's because older GCC don't understand -z. But anyway, to be consistent, let's do the same here. Can you test the above version and resubmit? Oh, and please also in the commit message give an example of how you can test it (e.g. by removing the micropyton patch and building micropyton under musl?) Regards, Arnout > + > # 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: 3ebc7c69d56430c34eba4c869d1d4fe4d1e8de55 > 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 _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Buildroot] [PATCH v3] Config.in: ban textrels on musl toolchains 2024-07-15 8:51 ` Arnout Vandecappelle via buildroot @ 2024-07-15 9:06 ` Thomas Petazzoni via buildroot 2024-07-17 11:52 ` J. Neuschäfer via buildroot 1 sibling, 0 replies; 4+ messages in thread From: Thomas Petazzoni via buildroot @ 2024-07-15 9:06 UTC (permalink / raw) To: Arnout Vandecappelle; +Cc: J. Neuschäfer, Yann E. MORIN, buildroot On Mon, 15 Jul 2024 10:51:03 +0200 Arnout Vandecappelle <arnout@mind.be> wrote: > After some discussion, we concluded that we don't see the point of making this > a user-settable Config.in option. It should be completely in the Makefile.in, > with a comment explaining why. So something like this (untested!): > > # 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. > ifeq ($(BR2_TOOLCHAIN_USES_MUSL)$(BR2_STATIC_LIBS),yy) > TARGET_LDFLAGS += -#l,-z,text ^^^ this should have been -Wl,-z,text of course 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] 4+ messages in thread
* Re: [Buildroot] [PATCH v3] Config.in: ban textrels on musl toolchains 2024-07-15 8:51 ` Arnout Vandecappelle via buildroot 2024-07-15 9:06 ` Thomas Petazzoni via buildroot @ 2024-07-17 11:52 ` J. Neuschäfer via buildroot 1 sibling, 0 replies; 4+ messages in thread From: J. Neuschäfer via buildroot @ 2024-07-17 11:52 UTC (permalink / raw) To: Arnout Vandecappelle Cc: Thomas Petazzoni, J. Neuschäfer, Yann E. MORIN, buildroot On Mon, Jul 15, 2024 at 10:51:03AM +0200, Arnout Vandecappelle wrote: > Hi J., > > [BTW are you OK with me calling you "J."? It feels a bit weird...] It's ok, as well as jn ("jay-enn"). which is also ok. > > On 04/07/2024 20:05, J. Neuschäfer via buildroot wrote: [...] > > +ifeq ($(BR2_LINK_ALLOW_TEXTREL),) > > +TARGET_LDFLAGS += -z text > > +endif > > After some discussion, we concluded that we don't see the point of making > this a user-settable Config.in option. It should be completely in the > Makefile.in, with a comment explaining why. So something like this > (untested!): > > # 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. > ifeq ($(BR2_TOOLCHAIN_USES_MUSL)$(BR2_STATIC_LIBS),yy) > TARGET_LDFLAGS += -#l,-z,text > endif > > Note also that we use -Wl,-z,... everywhere else to pass the -z options to > the linker. Maybe that's because older GCC don't understand -z. But anyway, > to be consistent, let's do the same here. > > Can you test the above version and resubmit? Oh, and please also in the > commit message give an example of how you can test it (e.g. by removing the > micropyton patch and building micropyton under musl?) Yep, will do! Thanks again for your review. jn _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-07-17 11:52 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-04 18:05 [Buildroot] [PATCH v3] Config.in: ban textrels on musl toolchains J. Neuschäfer via buildroot 2024-07-15 8:51 ` Arnout Vandecappelle via buildroot 2024-07-15 9:06 ` Thomas Petazzoni via buildroot 2024-07-17 11:52 ` 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