From: Fangrui Song <i@maskray.me>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: Binutils <binutils@sourceware.org>
Subject: Re: [PATCH] x86: Make protected symbols local for -shared
Date: Mon, 27 Jun 2022 10:09:03 -0700 [thread overview]
Message-ID: <20220627170903.nyi5lyjs4jubwbxs@gmail.com> (raw)
In-Reply-To: <CAMe9rOrnZaDB926S3kuQPpNoJEy6R_FA2Sc3gDJQW_GkCZh4zg@mail.gmail.com>
On 2022-06-27, H.J. Lu wrote:
>On Sun, Jun 26, 2022 at 12:03 PM Fangrui Song <i@maskray.me> wrote:
>>
>> On 2022-06-26, H.J. Lu wrote:
>> >On Sat, Jun 25, 2022 at 10:44 AM Fangrui Song <i@maskray.me> wrote:
>> >>
>> >> Call _bfd_elf_symbol_refs_local_p with local_protected==true. This has
>> >> 2 noticeable effects for -shared:
>> >>
>> >> * GOT-generating relocations referencing a protected data symbol no
>> >> longer lead to a GLOB_DAT (similar to a hidden symbol).
>> >> * Direct access relocations (e.g. R_X86_64_PC32) no longer has the
>> >> confusing diagnostic below.
>> >>
>> >> __attribute__((visibility("protected"))) void *foo() {
>> >> return (void *)foo;
>> >> }
>> >>
>> >> // gcc -fpic -shared -fuse-ld=bfd
>> >> relocation R_X86_64_PC32 against protected symbol `foo' can not be used when making a shared object
>> >>
>> >> The new behavior matches arm, aarch64 (commit
>> >> 83c325007c5599fa9b60b8d5f7b84842160e1d1b), and powerpc ports, and other
>> >> linkers: gold and ld.lld.
>> >>
>> >> Note: if some code tries to use direct access relocations to take the
>> >> address of foo, the pointer equality will break, but the error should be
>> >> reported on the executable link, not on the innocent shared object link.
>> >> glibc 2.36 will give a warning at relocation resolving time.
>> >
>> >It should be controlled by -z [no]indirect-extern-access. Can you enable
>> >-z indirect-extern-access with -shared by default instead?
>>
>> If I set `link_info.indirect_extern_access = 1;` in ld/ldmain.c,
>> bfd/elf-properties.c:654 will create a
>> GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS note.
>> This will probably be unexpected (and check-ld will have 280+ failures).
>
>This is normal when the default behavior is changed. You can pass
>-z noindirect-extern-access to these testcases.
Adding GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS will be a
significant behavior change and may unnecessarily break user programs
(glibc will report an error instead of a warning).
If the executable takes the address of a protected function defined in a
shared object, it may or may not cause a pointer equality problem (the
shared object may not take the address) and the problem (if exists) may or
may not be a broken invariance to the program (it may not expect pointer
equality).
All of aarch64/arm/powerpc64/riscv (likely most except x86, but I
haven't enumerated) consider a protected data symbol local in -shared
links. x86 did so a while ago (before 2015?). (For
aarch64/arm/powerpc64/riscv, I wish that we never need
GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS. The property will just
waste some bytes in every shared object without carrying much
information.)
The 280+ failures in check-ld due to the default
GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS need to be considered as
well.
>> >> With this change, `#define elf_backend_extern_protected_data 1` is no
>> >> longer effective. Just remove it.
>> >>
>> >> Remove the test "Run protected-func-1 without PIE" since -fno-pic
>> >> address taken operation in the executable doesn't work with protected
>> >> symbol in a shared object by default. Similarly, remove
>> >> protected-data-1a and protected-data-1b. protected-data-1b can be made
>> >> working by removing HAVE_LD_PIE_COPYRELOC from GCC
>> >> (https://sourceware.org/pipermail/gcc-patches/2022-June/596678.html).
>> >> ---
>> >> bfd/elf32-i386.c | 1 -
>> >> bfd/elf64-x86-64.c | 1 -
>> >> bfd/elfxx-x86.c | 2 +-
>> >> ld/testsuite/ld-i386/protected1.d | 4 +++-
>> >> ld/testsuite/ld-i386/protected3.d | 2 +-
>> >> ld/testsuite/ld-i386/protected6a.d | 4 +++-
>> >> ld/testsuite/ld-x86-64/pr24151a-x32.d | 4 +++-
>> >> ld/testsuite/ld-x86-64/pr24151a.d | 4 +++-
>> >> ld/testsuite/ld-x86-64/protected1.d | 4 +++-
>> >> ld/testsuite/ld-x86-64/protected3.d | 2 +-
>> >> ld/testsuite/ld-x86-64/protected6a.d | 4 +++-
>> >> ld/testsuite/ld-x86-64/protected7a.d | 4 +++-
>> >> ld/testsuite/ld-x86-64/x86-64.exp | 27 ---------------------------
>> >> 13 files changed, 24 insertions(+), 39 deletions(-)
>> >>
>> >> diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
>> >> index e4106d9fd3b..c3c46795731 100644
>> >> --- a/bfd/elf32-i386.c
>> >> +++ b/bfd/elf32-i386.c
>> >> @@ -4424,7 +4424,6 @@ elf_i386_link_setup_gnu_properties (struct bfd_link_info *info)
>> >> #define elf_backend_got_header_size 12
>> >> #define elf_backend_plt_alignment 4
>> >> #define elf_backend_dtrel_excludes_plt 1
>> >> -#define elf_backend_extern_protected_data 1
>> >> #define elf_backend_caches_rawsize 1
>> >> #define elf_backend_want_dynrelro 1
>> >>
>> >> diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
>> >> index 6154a70bdd7..aaa5f1496b9 100644
>> >> --- a/bfd/elf64-x86-64.c
>> >> +++ b/bfd/elf64-x86-64.c
>> >> @@ -5275,7 +5275,6 @@ elf_x86_64_special_sections[]=
>> >> #define elf_backend_got_header_size (GOT_ENTRY_SIZE*3)
>> >> #define elf_backend_rela_normal 1
>> >> #define elf_backend_plt_alignment 4
>> >> -#define elf_backend_extern_protected_data 1
>> >> #define elf_backend_caches_rawsize 1
>> >> #define elf_backend_dtrel_excludes_plt 1
>> >> #define elf_backend_want_dynrelro 1
>> >> diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
>> >> index acb2cc8528d..18f3d335458 100644
>> >> --- a/bfd/elfxx-x86.c
>> >> +++ b/bfd/elfxx-x86.c
>> >> @@ -3094,7 +3094,7 @@ _bfd_x86_elf_link_symbol_references_local (struct bfd_link_info *info,
>> >> 2. When building executable, there is no dynamic linker. Or
>> >> 3. or "-z nodynamic-undefined-weak" is used.
>> >> */
>> >> - if (SYMBOL_REFERENCES_LOCAL (info, h)
>> >> + if (_bfd_elf_symbol_refs_local_p (h, info, 1)
>> >> || (h->root.type == bfd_link_hash_undefweak
>> >> && (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
>> >> || (bfd_link_executable (info)
>> >> diff --git a/ld/testsuite/ld-i386/protected1.d b/ld/testsuite/ld-i386/protected1.d
>> >> index a3cb5cef140..531645b8fe8 100644
>> >> --- a/ld/testsuite/ld-i386/protected1.d
>> >> +++ b/ld/testsuite/ld-i386/protected1.d
>> >> @@ -1,3 +1,5 @@
>> >> #as: --32
>> >> #ld: -shared -melf_i386
>> >> -#error: .*relocation R_386_GOTOFF against protected function `foo' can not be used when making a shared object
>> >> +#readelf: -rW
>> >> +#...
>> >> +There are no relocations in this file.
>> >> diff --git a/ld/testsuite/ld-i386/protected3.d b/ld/testsuite/ld-i386/protected3.d
>> >> index c3a6888d900..77367c4738f 100644
>> >> --- a/ld/testsuite/ld-i386/protected3.d
>> >> +++ b/ld/testsuite/ld-i386/protected3.d
>> >> @@ -8,7 +8,7 @@
>> >> Disassembly of section .text:
>> >>
>> >> 0+[a-f0-9]+ <bar>:
>> >> -[ ]*[a-f0-9]+: 8b 81 [a-f0-9][a-f0-9] [a-f0-9][a-f0-9] ff ff mov -0x[a-f0-9]+\(%ecx\),%eax
>> >> +[ ]*[a-f0-9]+: 8d 81 00 00 00 00 lea 0x0\(%ecx\),%eax
>> >> [ ]*[a-f0-9]+: 8b 00 mov \(%eax\),%eax
>> >> [ ]*[a-f0-9]+: c3 ret
>> >> #pass
>> >> diff --git a/ld/testsuite/ld-i386/protected6a.d b/ld/testsuite/ld-i386/protected6a.d
>> >> index 7dc350432f4..4d3873239f9 100644
>> >> --- a/ld/testsuite/ld-i386/protected6a.d
>> >> +++ b/ld/testsuite/ld-i386/protected6a.d
>> >> @@ -1,4 +1,6 @@
>> >> #source: protected6.s
>> >> #as: --32
>> >> #ld: -shared -melf_i386
>> >> -#error: .*relocation R_386_GOTOFF against protected data `foo' can not be used when making a shared object
>> >> +#readelf: -rW
>> >> +#...
>> >> +There are no relocations in this file.
>> >> diff --git a/ld/testsuite/ld-x86-64/pr24151a-x32.d b/ld/testsuite/ld-x86-64/pr24151a-x32.d
>> >> index 130611ddf49..1f49b655f7d 100644
>> >> --- a/ld/testsuite/ld-x86-64/pr24151a-x32.d
>> >> +++ b/ld/testsuite/ld-x86-64/pr24151a-x32.d
>> >> @@ -1,4 +1,6 @@
>> >> #source: pr24151a.s
>> >> #as: --x32
>> >> #ld: -shared -melf32_x86_64
>> >> -#error: .*relocation R_X86_64_PC32 against protected symbol `foo' can not be used when making a shared object
>> >> +#readelf: -rW
>> >> +#...
>> >> +There are no relocations in this file.
>> >> diff --git a/ld/testsuite/ld-x86-64/pr24151a.d b/ld/testsuite/ld-x86-64/pr24151a.d
>> >> index 783b85a1a6f..6c48e383e01 100644
>> >> --- a/ld/testsuite/ld-x86-64/pr24151a.d
>> >> +++ b/ld/testsuite/ld-x86-64/pr24151a.d
>> >> @@ -1,3 +1,5 @@
>> >> #as: --64
>> >> #ld: -shared -melf_x86_64
>> >> -#error: .*relocation R_X86_64_PC32 against protected symbol `foo' can not be used when making a shared object
>> >> +#readelf: -rW
>> >> +#...
>> >> +There are no relocations in this file.
>> >> diff --git a/ld/testsuite/ld-x86-64/protected1.d b/ld/testsuite/ld-x86-64/protected1.d
>> >> index 783b85a1a6f..6c48e383e01 100644
>> >> --- a/ld/testsuite/ld-x86-64/protected1.d
>> >> +++ b/ld/testsuite/ld-x86-64/protected1.d
>> >> @@ -1,3 +1,5 @@
>> >> #as: --64
>> >> #ld: -shared -melf_x86_64
>> >> -#error: .*relocation R_X86_64_PC32 against protected symbol `foo' can not be used when making a shared object
>> >> +#readelf: -rW
>> >> +#...
>> >> +There are no relocations in this file.
>> >> diff --git a/ld/testsuite/ld-x86-64/protected3.d b/ld/testsuite/ld-x86-64/protected3.d
>> >> index 57950e4d6b6..ba63991582f 100644
>> >> --- a/ld/testsuite/ld-x86-64/protected3.d
>> >> +++ b/ld/testsuite/ld-x86-64/protected3.d
>> >> @@ -8,7 +8,7 @@
>> >> Disassembly of section .text:
>> >>
>> >> 0+[a-f0-9]+ <bar>:
>> >> -[ ]*[a-f0-9]+: 48 8b 05 ([0-9a-f]{2} ){4} * mov 0x[a-f0-9]+\(%rip\),%rax # [a-f0-9]+ <.*>
>> >> +[ ]*[a-f0-9]+: 48 8d 05 ([0-9a-f]{2} ){4} * lea 0x[a-f0-9]+\(%rip\),%rax # [a-f0-9]+ <.*>
>> >> [ ]*[a-f0-9]+: 8b 00 mov \(%rax\),%eax
>> >> [ ]*[a-f0-9]+: c3 ret
>> >> #pass
>> >> diff --git a/ld/testsuite/ld-x86-64/protected6a.d b/ld/testsuite/ld-x86-64/protected6a.d
>> >> index 3a7963ffd2f..50d6430b577 100644
>> >> --- a/ld/testsuite/ld-x86-64/protected6a.d
>> >> +++ b/ld/testsuite/ld-x86-64/protected6a.d
>> >> @@ -1,4 +1,6 @@
>> >> #source: protected6.s
>> >> #as: --64
>> >> #ld: -shared -melf_x86_64
>> >> -#error: .*relocation R_X86_64_GOTOFF64 against protected data `foo' can not be used when making a shared object
>> >> +#readelf: -rW
>> >> +#...
>> >> +There are no relocations in this file.
>> >> diff --git a/ld/testsuite/ld-x86-64/protected7a.d b/ld/testsuite/ld-x86-64/protected7a.d
>> >> index 3082084a7b8..3974246a2a8 100644
>> >> --- a/ld/testsuite/ld-x86-64/protected7a.d
>> >> +++ b/ld/testsuite/ld-x86-64/protected7a.d
>> >> @@ -1,4 +1,6 @@
>> >> #source: protected7.s
>> >> #as: --64
>> >> #ld: -shared -melf_x86_64
>> >> -#error: .*relocation R_X86_64_GOTOFF64 against protected function `foo' can not be used when making a shared object
>> >> +#readelf: -rW
>> >> +#...
>> >> +There are no relocations in this file.
>> >> diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
>> >> index 5e5636bcebe..a096c0b9d0f 100644
>> >> --- a/ld/testsuite/ld-x86-64/x86-64.exp
>> >> +++ b/ld/testsuite/ld-x86-64/x86-64.exp
>> >> @@ -1832,15 +1832,6 @@ if { [isnative] && [check_compiler_available] } {
>> >> "pr23997" \
>> >> "pass.out" \
>> >> ] \
>> >> - [list \
>> >> - "Run protected-func-1 without PIE" \
>> >> - "$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libprotected-func-1.so" \
>> >> - "-Wa,-mx86-used-note=yes" \
>> >> - { protected-func-1b.c } \
>> >> - "protected-func-1a" \
>> >> - "pass.out" \
>> >> - "$NOPIE_CFLAGS" \
>> >> - ] \
>> >> [list \
>> >> "Run protected-func-1 with PIE" \
>> >> "-Wl,--no-as-needed -pie tmpdir/libprotected-func-1.so" \
>> >> @@ -1904,24 +1895,6 @@ if { [isnative] && [check_compiler_available] } {
>> >> "pass.out" \
>> >> "-fPIE" \
>> >> ] \
>> >> - [list \
>> >> - "Run protected-data-1a without PIE" \
>> >> - "$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libprotected-data-1a.so" \
>> >> - "-Wa,-mx86-used-note=yes" \
>> >> - { protected-data-1b.c } \
>> >> - "protected-data-1a" \
>> >> - "pass.out" \
>> >> - "$NOPIE_CFLAGS" \
>> >> - ] \
>> >> - [list \
>> >> - "Run protected-data-1b with PIE" \
>> >> - "-Wl,--no-as-needed -pie tmpdir/libprotected-data-1a.so" \
>> >> - "-Wa,-mx86-used-note=yes" \
>> >> - { protected-data-1b.c } \
>> >> - "protected-data-1b" \
>> >> - "pass.out" \
>> >> - "-fPIE" \
>> >> - ] \
>> >> [list \
>> >> "Run protected-data-2a without PIE" \
>> >> "$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libprotected-data-2a.so" \
>> >> --
>> >> 2.37
>> >>
>> >
>> >
>> >--
>> >H.J.
>
>
>
>--
>H.J.
next prev parent reply other threads:[~2022-06-27 17:09 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-25 17:44 Fangrui Song
2022-06-26 18:13 ` H.J. Lu
2022-06-26 19:03 ` Fangrui Song
2022-06-26 19:07 ` Fangrui Song
2022-06-27 13:30 ` H.J. Lu
2022-06-27 13:24 ` H.J. Lu
2022-06-27 17:09 ` Fangrui Song [this message]
2022-06-27 17:43 ` H.J. Lu
2022-06-27 17:53 ` Fangrui Song
2022-06-27 18:26 ` H.J. Lu
2022-06-27 18:46 ` Fangrui Song
2022-06-27 18:57 ` H.J. Lu
2022-06-28 3:07 ` Fangrui Song
2022-06-28 3:24 ` H.J. Lu
2022-06-28 3:43 ` Fangrui Song
2022-06-28 3:51 ` H.J. Lu
2022-06-28 4:18 ` Fangrui Song
2022-07-19 1:44 ` H.J. Lu
2022-07-19 3:13 ` Fangrui Song
2022-07-19 3:38 ` H.J. Lu
2022-07-19 4:02 ` Fangrui Song
2022-07-19 15:40 ` H.J. Lu
2022-07-25 14:07 ` H.J. Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220627170903.nyi5lyjs4jubwbxs@gmail.com \
--to=i@maskray.me \
--cc=binutils@sourceware.org \
--cc=hjl.tools@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).