Re: [PATCH] x86: Make protected symbols local for -shared

[Fangrui Song <i@maskray.me>]

On 2022-06-26, Fangrui Song 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).

By removing elf_backend_extern_protected_data as in the current patch,
no port will use elf_backend_extern_protected_data. We can just remove
elf_backend_extern_protected_data, and probably make -z (no)?extern-protected-data 
ignored options in the future (I believe nobody uses this option).

>>>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.