[PATCH] ppc: add "-z notext" flag to disable diagnostic

[PATCH] ppc: add "-z notext" flag to disable diagnostic

[Bill Wendling]

The "-z notext" flag disables reporting an error if DT_TEXTREL is set on
PPC with CONFIG=kdump:

  ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against
    local symbol in readonly segment; recompile object files with -fPIC
    or pass '-Wl,-z,notext' to allow text relocations in the output
  >>> defined in built-in.a(arch/powerpc/kernel/misc.o)
  >>> referenced by arch/powerpc/kernel/misc.o:(.text+0x20) in archive
      built-in.a

The BFD linker disables this by default (though it's configurable in
current versions). LLD enables this by default. So we add the flag to
keep LLD from emitting the error.

Signed-off-by: Bill Wendling <morbo@google.com>

---
The output of the "get_maintainer.pl" run just in case no one believes me. :-)

$ ./scripts/get_maintainer.pl arch/powerpc/Makefile
Michael Ellerman <mpe@ellerman.id.au> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
Benjamin Herrenschmidt <benh@kernel.crashing.org> (reviewer:LINUX FOR POWERPC (32-BIT AND 64-BIT))
Paul Mackerras <paulus@samba.org> (reviewer:LINUX FOR POWERPC (32-BIT AND 64-BIT))
Nathan Chancellor <nathan@kernel.org> (supporter:CLANG/LLVM BUILD SUPPORT)
Nick Desaulniers <ndesaulniers@google.com> (supporter:CLANG/LLVM BUILD SUPPORT)
linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC (32-BIT AND 64-BIT))
linux-kernel@vger.kernel.org (open list)
clang-built-linux@googlegroups.com (open list:CLANG/LLVM BUILD SUPPORT)
---
 arch/powerpc/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 6505d66f1193..17a9fbf9b789 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -122,6 +122,7 @@ endif
 
 LDFLAGS_vmlinux-y := -Bstatic
 LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
+LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) += -z notext
 LDFLAGS_vmlinux	:= $(LDFLAGS_vmlinux-y)
 
 ifdef CONFIG_PPC64
-- 
2.33.0.rc1.237.g0d66db33f3-goog

Re: [PATCH] ppc: add "-z notext" flag to disable diagnostic

[Nick Desaulniers]

On Thu, Aug 12, 2021 at 1:49 PM Bill Wendling <morbo@google.com> wrote:
>
> The "-z notext" flag disables reporting an error if DT_TEXTREL is set on
> PPC with CONFIG=kdump:
>
>   ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against
>     local symbol in readonly segment; recompile object files with -fPIC
>     or pass '-Wl,-z,notext' to allow text relocations in the output
>   >>> defined in built-in.a(arch/powerpc/kernel/misc.o)
>   >>> referenced by arch/powerpc/kernel/misc.o:(.text+0x20) in archive
>       built-in.a
>
> The BFD linker disables this by default (though it's configurable in
> current versions). LLD enables this by default. So we add the flag to
> keep LLD from emitting the error.
>
> Signed-off-by: Bill Wendling <morbo@google.com>

Link: https://github.com/ClangBuiltLinux/linux/issues/811
Reported-by: Itaru Kitayama <itaru.kitayama@riken.jp>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> ---
> The output of the "get_maintainer.pl" run just in case no one believes me. :-)

LOL!

>
> $ ./scripts/get_maintainer.pl arch/powerpc/Makefile
> Michael Ellerman <mpe@ellerman.id.au> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Benjamin Herrenschmidt <benh@kernel.crashing.org> (reviewer:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Paul Mackerras <paulus@samba.org> (reviewer:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Nathan Chancellor <nathan@kernel.org> (supporter:CLANG/LLVM BUILD SUPPORT)
> Nick Desaulniers <ndesaulniers@google.com> (supporter:CLANG/LLVM BUILD SUPPORT)
> linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> linux-kernel@vger.kernel.org (open list)
> clang-built-linux@googlegroups.com (open list:CLANG/LLVM BUILD SUPPORT)
> ---
>  arch/powerpc/Makefile | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 6505d66f1193..17a9fbf9b789 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -122,6 +122,7 @@ endif
>
>  LDFLAGS_vmlinux-y := -Bstatic
>  LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
> +LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) += -z notext
>  LDFLAGS_vmlinux        := $(LDFLAGS_vmlinux-y)
>
>  ifdef CONFIG_PPC64
> --
> 2.33.0.rc1.237.g0d66db33f3-goog
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] ppc: add "-z notext" flag to disable diagnostic

[Daniel Axtens]

Bill Wendling <morbo@google.com> writes:

> The "-z notext" flag disables reporting an error if DT_TEXTREL is set on
> PPC with CONFIG=kdump:
>
>   ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against
>     local symbol in readonly segment; recompile object files with -fPIC
>     or pass '-Wl,-z,notext' to allow text relocations in the output
>   >>> defined in built-in.a(arch/powerpc/kernel/misc.o)
>   >>> referenced by arch/powerpc/kernel/misc.o:(.text+0x20) in archive
>       built-in.a
>
> The BFD linker disables this by default (though it's configurable in
> current versions). LLD enables this by default. So we add the flag to
> keep LLD from emitting the error.

You didn't provide a huge amount of context but I was able to reproduce
a similar set of errors with pseries_le_defconfig and

make ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu- CC="ccache clang-11" LD=ld.lld-11 AR=llvm-ar-11 -j4 vmlinux

I also checked the manpage, and indeed the system ld does not issue this
warning/error by default.

> ---
>  arch/powerpc/Makefile | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 6505d66f1193..17a9fbf9b789 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -122,6 +122,7 @@ endif
>  
>  LDFLAGS_vmlinux-y := -Bstatic
>  LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
> +LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) += -z notext

Is there any reason this should be gated on CONFIG_RELOCATABLE? (I tried
without it and got different but possibly related linker errors...)

Also, is this behaviour new?

Kind regards,
Daniel

>  LDFLAGS_vmlinux	:= $(LDFLAGS_vmlinux-y)
>  
>  ifdef CONFIG_PPC64
> -- 
> 2.33.0.rc1.237.g0d66db33f3-goog

Re: [PATCH] ppc: add "-z notext" flag to disable diagnostic

[Bill Wendling]

On Fri, Aug 13, 2021 at 7:13 AM Daniel Axtens <dja@axtens.net> wrote:
>
> Bill Wendling <morbo@google.com> writes:
>
> > The "-z notext" flag disables reporting an error if DT_TEXTREL is set on
> > PPC with CONFIG=kdump:
> >
> >   ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against
> >     local symbol in readonly segment; recompile object files with -fPIC
> >     or pass '-Wl,-z,notext' to allow text relocations in the output
> >   >>> defined in built-in.a(arch/powerpc/kernel/misc.o)
> >   >>> referenced by arch/powerpc/kernel/misc.o:(.text+0x20) in archive
> >       built-in.a
> >
> > The BFD linker disables this by default (though it's configurable in
> > current versions). LLD enables this by default. So we add the flag to
> > keep LLD from emitting the error.
>
> You didn't provide a huge amount of context but I was able to reproduce
> a similar set of errors with pseries_le_defconfig and
>
My apologies for the lack of context. We discovered this issue when
moving to use LLD instead of BFD for our linker. This change suggested
by Fangrui Song. Here's his comments from
https://github.com/ClangBuiltLinux/linux/issues/811. (Nick Desaulniers
added a "Link" tag with his review.)

```
The text relocations are real. lld can link .tmp_vmlinux1 smoothly if
you pass -z notext. Though, it will still be insightful to investigate
where these text relocations come from. I believe there are only 2
categories.

(a) For a CONFIG_HAVE_ARCH_PREL32_RELOCATIONS=y build (x86 and arm64
select the option by default), ___ksymtab+* sections (non-SHF_WRITE)
contains entries relocated by PC relative relocations. These entries
do not need dynamic relocations.

out/powerpc64le/.config generated by ppc64le_defconfig does not enable
CONFIG_HAVE_ARCH_PREL32_RELOCATIONS=y.

% readelf -r out/x86_64/entry/entry_64.o
...
Relocation section '.rela___ksymtab+native_load_gs_index' at offset
0x6460 contains 2 entries:
    Offset             Info             Type               Symbol's
Value  Symbol's Name + Addend
0000000000000000  0000007a00000002 R_X86_64_PC32
0000000000000ea0 native_load_gs_index + 0
0000000000000004  0000001d00000002 R_X86_64_PC32
0000000000000000 __ksymtab_strings + 0

include/asm-generic/export.h

ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against
local symbol in readonly segment; recompile object files with -fPIC or
pass '-Wl
,-z,notext' to allow text relocations in the output
>>> defined in init/built-in.a(do_mounts.o)
>>> referenced by do_mounts.c
>>>               do_mounts.o:(__ksymtab_name_to_dev_t) in archive init/built-in.a

(b) R_PPC64_ADDR64 in __mcount_loc sections.

ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against
local symbol in readonly segment; recompile object files with -fPIC or
pass '-Wl
,-z,notext' to allow text relocations in the output
>>> defined in init/built-in.a(do_mounts_rd.o)
>>> referenced by do_mounts_rd.c
>>>               do_mounts_rd.o:(__mcount_loc+0x0) in archive init/built-in.a

This section is generated by ./scripts/recordmcount
"init/do_mounts_rd.o". The tool hard codes R_PPC64_ADDR64.

% grep PPC64 scripts/recordmcount.c
        case EM_PPC64:  reltype = R_PPC64_ADDR64; break;
```

> make ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu- CC="ccache clang-11" LD=ld.lld-11 AR=llvm-ar-11 -j4 vmlinux
>
> I also checked the manpage, and indeed the system ld does not issue this
> warning/error by default.
>
It should be noted that the BFD linker can emit this warning/error if
binutils is configured with
"--enable-textrel-check={warning,error,yes}".

> > ---
> >  arch/powerpc/Makefile | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> > index 6505d66f1193..17a9fbf9b789 100644
> > --- a/arch/powerpc/Makefile
> > +++ b/arch/powerpc/Makefile
> > @@ -122,6 +122,7 @@ endif
> >
> >  LDFLAGS_vmlinux-y := -Bstatic
> >  LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
> > +LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) += -z notext
>
> Is there any reason this should be gated on CONFIG_RELOCATABLE? (I tried
> without it and got different but possibly related linker errors...)
>
My understanding is that '-z notext' allows (i.e. doesn't emit
warnings/errors for) relocations against read-only segments. So it's
not really relevant to non-relocatable builds.

Unrelated question: Should the "-pie" flag be added with "+= -pie"
(note the plus sign)?

> Also, is this behaviour new?
>
I don't believe so. It's only when we wanted to use LLD that it was noticed.

BTW, this patch should more properly be attributed to Fangrui Song. I
can send a follow-up patch that reflects this and adds more context to
the commit message.

-bw

> Kind regards,
> Daniel
>
> >  LDFLAGS_vmlinux      := $(LDFLAGS_vmlinux-y)
> >
> >  ifdef CONFIG_PPC64
> > --
> > 2.33.0.rc1.237.g0d66db33f3-goog

Re: [PATCH] ppc: add "-z notext" flag to disable diagnostic

[Nick Desaulniers]

On Fri, Aug 13, 2021 at 11:24 AM Bill Wendling <morbo@google.com> wrote:
>
> BTW, this patch should more properly be attributed to Fangrui Song. I
> can send a follow-up patch that reflects this and adds more context to
> the commit message.

Sounds good to me. The TL;DR is that LLD has a different implicit
default for `-z text` vs `-z notext` than BFD.  We can emulate the
behavior or BFD by simply being explicit about `-z notext` always.

Or we can dig through why there are relocations in read only sections,
fix those, then enable `-z text` for all linkers.  My recommendation
would be get the thing building, then go digging time permitting.
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] ppc: add "-z notext" flag to disable diagnostic

[Segher Boessenkool]

On Fri, Aug 13, 2021 at 11:59:21AM -0700, Nick Desaulniers wrote:
> Or we can dig through why there are relocations in read only sections,
> fix those, then enable `-z text` for all linkers.  My recommendation
> would be get the thing building, then go digging time permitting.

It is not always a bug.  You can get much more efficient code if you
have text relocations than if you don't.  This "read-only" memory is
perfectly writable until after relocation, a la relro.

But you no doubt will find some non-optimalities (or even straight out
bugs) if you build with -ztext sometimes :-)


Segher

Re: [PATCH] ppc: add "-z notext" flag to disable diagnostic

[Michael Ellerman]

Bill Wendling <morbo@google.com> writes:
> On Fri, Aug 13, 2021 at 7:13 AM Daniel Axtens <dja@axtens.net> wrote:
>> Bill Wendling <morbo@google.com> writes:
...
>> > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> > index 6505d66f1193..17a9fbf9b789 100644
>> > --- a/arch/powerpc/Makefile
>> > +++ b/arch/powerpc/Makefile
>> > @@ -122,6 +122,7 @@ endif
>> >
>> >  LDFLAGS_vmlinux-y := -Bstatic
>> >  LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
>> > +LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) += -z notext
...
>
> Unrelated question: Should the "-pie" flag be added with "+= -pie"
> (note the plus sign)?

I noticed that too.

It's been like that since the original relocatable support was added in
2008, commit 549e8152de80 ("powerpc: Make the 64-bit kernel as a
position-independent executable"), which did:

-LDFLAGS_vmlinux	:= -Bstatic
+LDFLAGS_vmlinux-yy := -Bstatic
+LDFLAGS_vmlinux-$(CONFIG_PPC64)$(CONFIG_RELOCATABLE) := -pie
+LDFLAGS_vmlinux	:= $(LDFLAGS_vmlinux-yy)


There's no mention of those flags in the change log. But the way it's
written suggests the intention was to not pass -Bstatic for relocatable
builds, otherwise it could have been more simply:

+LDFLAGS_vmlinux-$(CONFIG_PPC64)$(CONFIG_RELOCATABLE) := -pie
+LDFLAGS_vmlinux	:= -Bstatic $(LDFLAGS_vmlinux-yy)


So I think it was deliberate to not use +=, but whether that's actually
correct I can't say. Maybe in the past -Bstatic and -pie were
incompatible?

cheers

Re: [PATCH] ppc: add "-z notext" flag to disable diagnostic

[Fangrui Song]

On 2021-08-14, Daniel Axtens wrote:
>Bill Wendling <morbo@google.com> writes:
>
>> The "-z notext" flag disables reporting an error if DT_TEXTREL is set on
>> PPC with CONFIG=kdump:
>>
>>   ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against
>>     local symbol in readonly segment; recompile object files with -fPIC
>>     or pass '-Wl,-z,notext' to allow text relocations in the output
>>   >>> defined in built-in.a(arch/powerpc/kernel/misc.o)
>>   >>> referenced by arch/powerpc/kernel/misc.o:(.text+0x20) in archive
>>       built-in.a
>>
>> The BFD linker disables this by default (though it's configurable in
>> current versions). LLD enables this by default. So we add the flag to
>> keep LLD from emitting the error.
>
>You didn't provide a huge amount of context but I was able to reproduce
>a similar set of errors with pseries_le_defconfig and
>
>make ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu- CC="ccache clang-11" LD=ld.lld-11 AR=llvm-ar-11 -j4 vmlinux
>
>I also checked the manpage, and indeed the system ld does not issue this
>warning/error by default.
>
>> ---
>>  arch/powerpc/Makefile | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> index 6505d66f1193..17a9fbf9b789 100644
>> --- a/arch/powerpc/Makefile
>> +++ b/arch/powerpc/Makefile
>> @@ -122,6 +122,7 @@ endif
>>
>>  LDFLAGS_vmlinux-y := -Bstatic
>>  LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
>> +LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) += -z notext
>
>Is there any reason this should be gated on CONFIG_RELOCATABLE? (I tried
>without it and got different but possibly related linker errors...)
>
>Also, is this behaviour new?

This is a longstanding behavior.

https://maskray.me/blog/2020-12-19-lld-and-gnu-linker-incompatibilities
See "Text relocations"

.o files used to link .tmp_vmlinux.kallsyms1 have many R_PPC64_ADDR64
relocations in non-SHF_WRITE sections. There are many text relocations (e.g. in
.rela___ksymtab_gpl+* and .rela__mcount_loc sections) in a -pie link and are
disallowed by LLD:

   ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against local symbol in readonly segment; recompile object files with -fPIC or pass '-Wl,-z,notext' to allow text relocations in the output
   >>> defined in arch/powerpc/kernel/head_64.o
   >>> referenced by arch/powerpc/kernel/head_64.o:(__restart_table+0x10)

Newer GNU ld configured with --enable-textrel-check=error will report an error
as well:

   ld/ld-new: read-only segment has dynamic relocations



Text relocations are considered very awful by linker developers.
binutils 2.35 added --enable-textrel-check={no,warn,error}
https://sourceware.org/bugzilla/show_bug.cgi?id=20824

I can imagine that in the future some Linux distributions (especially those
focusing on security) will default their binutils to use
--enable-textrel-check={no,warn,error}.  CONFIG_RELOCATABLE build will break
sooner or later.


In -no-pie links, R_PPC64_ADDR64 relocations are link-time constants.
There are no text relocations, therefore no need for -z notext.

>Kind regards,
>Daniel
>
>>  LDFLAGS_vmlinux	:= $(LDFLAGS_vmlinux-y)
>>
>>  ifdef CONFIG_PPC64
>> --
>> 2.33.0.rc1.237.g0d66db33f3-goog
>
>-- 
>You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
>To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
>To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/87sfzde8lk.fsf%40linkitivity.dja.id.au.

Re: [PATCH] ppc: add "-z notext" flag to disable diagnostic

[Segher Boessenkool]

On Fri, Aug 13, 2021 at 01:05:08PM -0700, Fangrui Song wrote:
> Text relocations are considered very awful by linker developers.

By very few linker developers.

> binutils 2.35 added --enable-textrel-check={no,warn,error}
> https://sourceware.org/bugzilla/show_bug.cgi?id=20824

Yes, some people wanted the default to be configurable.  So now we have
a default default that is sane, so most people get to reap the benefits
of having defaults at all, but we also allow other people to shoot
themselves (and people who have to deal with them) in the foot.
"Progress".  Changing the defaults should be a one-time event, only done
when the benefits strongly outweigh the costs.  Defaults should never be
configurable (by the user).

> I can imagine that in the future some Linux distributions (especially those
> focusing on security) will default their binutils to use
> --enable-textrel-check={no,warn,error}.

How would this be a benefit to security?

> In -no-pie links, R_PPC64_ADDR64 relocations are link-time constants.

Where "link" includes dynamic links as well.  There are no constants.

> There are no text relocations, therefore no need for -z notext.

This is a choice by the compiler, nothing more.  It saves some process
startup time, and allows slightly more maps to be shared by processes
that run the same images.  But it is a tradeoff, so it might change; and
of course it is not an ABI requirement.


Segher

Re: [PATCH] ppc: add "-z notext" flag to disable diagnostic

[Fāng-ruì Sòng]

On Sat, Aug 14, 2021 at 5:59 AM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Fri, Aug 13, 2021 at 01:05:08PM -0700, Fangrui Song wrote:
> > Text relocations are considered very awful by linker developers.
>
> By very few linker developers.

https://groups.google.com/g/generic-abi/c/Ckq19PfLxyk/m/uW29sgkoAgAJ
Ali Bahrami: "My opinion is that no one wants text relocations, but
not labeling them if they do exist doesn't seem right. I find the
presence of DF_TEXTREL very interesting when diagnosing various
issues."

https://gcc.gnu.org/legacy-ml/gcc/2016-04/msg00138.html
( "So why not simply create 'dynamic text relocations' then?  Is that
possible with a pure linker change?" )
Cary Coutant: "Ugh. Besides being a bad idea from a performance point
of view, it's not even always possible to do. Depending on the
architecture, a direct reference from an executable to a variable in a
shared library may not have the necessary reach."

binutils-gdb commit "Add linker option: --warn-shared-textrel to
produce warnings when adding a DT_TEXTREL to a shared object."
Nick Clifton

https://www.openwall.com/lists/musl/2020/09/26/3
Szabolcs Nagy: "nice.  and gcc passes -z text for static pie code so
that case should not end up with text rels."

Someone wrote "Overall, the overhead of processing text relocations
can cause serious performance degradation." in Solaris' Linker and
Libraries Guide.

Me :)
(I wrote lld/ELF 9.0.0~13.0.0 release notes and filed dozen of GNU
ld/gold bugs/feature requests)

> > binutils 2.35 added --enable-textrel-check={no,warn,error}
> > https://sourceware.org/bugzilla/show_bug.cgi?id=20824
>
> Yes, some people wanted the default to be configurable.  So now we have
> a default default that is sane, so most people get to reap the benefits
> of having defaults at all, but we also allow other people to shoot
> themselves (and people who have to deal with them) in the foot.
> "Progress".  Changing the defaults should be a one-time event, only done
> when the benefits strongly outweigh the costs.  Defaults should never be
> configurable (by the user).

ld.lld has such a non-configurable model and thus caught the issue
(which the patch intends to address).

Future --enable-textrel-check={yes,error} configured GNU ld will be similar.

> > I can imagine that in the future some Linux distributions (especially those
> > focusing on security) will default their binutils to use
> > --enable-textrel-check={no,warn,error}.
>
> How would this be a benefit to security?

https://flameeyes.blog/2016/01/16/textrels-text-relocations-and-their-impact-on-hardening-techniques/

https://github.com/golang/go/issues/9210
Android: "libexample.so has text relocations. This is wasting memory
and prevents security hardening. Please fix."

FWIW I contributed a glibc patch allowing TEXTREL to co-exist with ifunc.
It requires temporary mapping the text segment W^X.

> > In -no-pie links, R_PPC64_ADDR64 relocations are link-time constants.
>
> Where "link" includes dynamic links as well.  There are no constants.

"Link-time" usually refers to the processing of the static linker.

Dynamic links can use load-time or run-time.

> > There are no text relocations, therefore no need for -z notext.
>
> This is a choice by the compiler, nothing more.  It saves some process
> startup time, and allows slightly more maps to be shared by processes
> that run the same images.  But it is a tradeoff, so it might change; and
> of course it is not an ABI requirement.
>
>
> Segher

Text relocations are generally awful.
GNU ld and gold's traditional "add DF_TEXTREL on-demand" behavior made
such user errors easy to make.
I understand that kernels are special applications where we apply
relocations once and many user-space objection can be less of a
concern/ignored.
However, the Linux kernel is already in a position where many linker
options are controlled and thus should specify -z notext to make
the intention explicit, or fix the problems (I think x86-64 is good;
that said, powerpc
has a higher cost using PC-relative instructions so pay the oneshot relocation
time cost probably isn't a bad choice)



--
宋方睿

Re: [PATCH] ppc: add "-z notext" flag to disable diagnostic

[Segher Boessenkool]

Hi!

On Sat, Aug 14, 2021 at 12:34:15PM -0700, Fāng-ruì Sòng wrote:
> On Sat, Aug 14, 2021 at 5:59 AM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> >
> > On Fri, Aug 13, 2021 at 01:05:08PM -0700, Fangrui Song wrote:
> > > Text relocations are considered very awful by linker developers.
> >
> > By very few linker developers.

> https://groups.google.com/g/generic-abi/c/Ckq19PfLxyk/m/uW29sgkoAgAJ
> Ali Bahrami: "My opinion is that no one wants text relocations, but
> not labeling them if they do exist doesn't seem right. I find the
> presence of DF_TEXTREL very interesting when diagnosing various
> issues."

I don't know who that is, and that post has no context.

> https://gcc.gnu.org/legacy-ml/gcc/2016-04/msg00138.html
> ( "So why not simply create 'dynamic text relocations' then?  Is that
> possible with a pure linker change?" )
> Cary Coutant: "Ugh. Besides being a bad idea from a performance point
> of view, it's not even always possible to do. Depending on the
> architecture, a direct reference from an executable to a variable in a
> shared library may not have the necessary reach."

That is about a very specific kind of relocation.

> binutils-gdb commit "Add linker option: --warn-shared-textrel to
> produce warnings when adding a DT_TEXTREL to a shared object."
> Nick Clifton

That does not say text relocations are bad.  Of course you want to know
if they are there, for various reasons, like, if they are disallowed on
some systems.

> https://www.openwall.com/lists/musl/2020/09/26/3
> Szabolcs Nagy: "nice.  and gcc passes -z text for static pie code so
> that case should not end up with text rels."

That does not say text relocations are bad.

> Someone wrote "Overall, the overhead of processing text relocations
> can cause serious performance degradation." in Solaris' Linker and
> Libraries Guide.

In process startup, sure.  And it can make those processes run faster
as well.  That is the tradeoff with *all* relocations; you can make any
code without any relocations.  Relocations are a tradeoff, like most
things.

> > How would this be a benefit to security?
> 
> https://flameeyes.blog/2016/01/16/textrels-text-relocations-and-their-impact-on-hardening-techniques/

This means that those "hardening techniques" have some serious
weaknesses, that is all.  And hardening is not part of security
anyway; it is impact mitigation.

> FWIW I contributed a glibc patch allowing TEXTREL to co-exist with ifunc.
> It requires temporary mapping the text segment W^X.

What does W^X mean here?  It normally means no mapping is both writable
and executable at the same time.

> > > There are no text relocations, therefore no need for -z notext.
> >
> > This is a choice by the compiler, nothing more.  It saves some process
> > startup time, and allows slightly more maps to be shared by processes
> > that run the same images.  But it is a tradeoff, so it might change; and
> > of course it is not an ABI requirement.

> Text relocations are generally awful.

Great arguments, thanks!  :-P

> GNU ld and gold's traditional "add DF_TEXTREL on-demand" behavior made
> such user errors easy to make.

That has no bearing on if text relocations are useful or not.

> I understand that kernels are special applications where we apply
> relocations once and many user-space objection can be less of a
> concern/ignored.
> However, the Linux kernel is already in a position where many linker
> options are controlled and thus should specify -z notext to make
> the intention explicit, or fix the problems (I think x86-64 is good;
> that said, powerpc
> has a higher cost using PC-relative instructions so pay the oneshot relocation
> time cost probably isn't a bad choice)

I have no idea what you mean.


Segher

[PATCH v2] ppc: add "-z notext" flag to disable diagnostic

[Bill Wendling]

From: Fangrui Song <maskray@google.com>

Object files used to link .tmp_vmlinux.kallsyms1 have many R_PPC64_ADDR64
relocations in non-SHF_WRITE sections. There are many text relocations (e.g. in
.rela___ksymtab_gpl+* and .rela__mcount_loc sections) in a -pie link and are
disallowed by LLD:

  ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against local symbol in readonly segment; recompile object files with -fPIC or pass '-Wl,-z,notext' to allow text relocations in the output
  >>> defined in arch/powerpc/kernel/head_64.o
  >>> referenced by arch/powerpc/kernel/head_64.o:(__restart_table+0x10)

Newer GNU ld configured with "--enable-textrel-check=error" will report an
error as well:

  $ ld-new -EL -m elf64lppc -pie ... -o .tmp_vmlinux.kallsyms1 ...
  ld-new: read-only segment has dynamic relocations

Add "-z notext" to suppress the errors. Non-CONFIG_RELOCATABLE builds use the
default -no-pie mode and thus R_PPC64_ADDR64 relocations can be resolved at
link-time.

Link: https://github.com/ClangBuiltLinux/linux/issues/811
Signed-off-by: Fangrui Song <maskray@google.com>
Co-developed-by: Bill Wendling <morbo@google.com>
Signed-off-by: Bill Wendling <morbo@google.com>
Reported-by: Itaru Kitayama <itaru.kitayama@riken.jp>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---
v2:
  - Assign "Fangrui Song" as the proper author.
  - Improve the commit message to add more context.
  - Appending tags from original patch's review.
---
 arch/powerpc/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 6505d66f1193..17a9fbf9b789 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -122,6 +122,7 @@ endif
 
 LDFLAGS_vmlinux-y := -Bstatic
 LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
+LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) += -z notext
 LDFLAGS_vmlinux	:= $(LDFLAGS_vmlinux-y)
 
 ifdef CONFIG_PPC64
-- 
2.33.0.rc1.237.g0d66db33f3-goog

Re: [PATCH v2] ppc: add "-z notext" flag to disable diagnostic

[Michael Ellerman]

On Fri, 13 Aug 2021 13:05:11 -0700, Bill Wendling wrote:
> Object files used to link .tmp_vmlinux.kallsyms1 have many R_PPC64_ADDR64
> relocations in non-SHF_WRITE sections. There are many text relocations (e.g. in
> .rela___ksymtab_gpl+* and .rela__mcount_loc sections) in a -pie link and are
> disallowed by LLD:
> 
>   ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against local symbol in readonly segment; recompile object files with -fPIC or pass '-Wl,-z,notext' to allow text relocations in the output
>   >>> defined in arch/powerpc/kernel/head_64.o
>   >>> referenced by arch/powerpc/kernel/head_64.o:(__restart_table+0x10)
> 
> [...]

Applied to powerpc/next.

[1/1] ppc: add "-z notext" flag to disable diagnostic
      https://git.kernel.org/powerpc/c/0355785313e2191be4e1108cdbda94ddb0238c48

cheers