RFC: PATCHES: Properly handle reference to protected data on x86
Commit Message
On Wed, Mar 4, 2015 at 3:26 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> Protected symbol means that it can't be pre-emptied. It
> doesn't mean its address won't be external. This is true
> for pointer to protected function. With copy relocation,
> address of protected data defined in the shared library may
> also be external. We only know that for sure at run-time.
> Here are patches for glibc, binutils and GCC to handle it
> properly.
>
> Any comments?
This is the binutils patch I checked in. It basically reverted
the change for
https://sourceware.org/bugzilla/show_bug.cgi?id=15228
on x86. Copy relocations against protected symbols should
work.
Comments
On Thu, Mar 05, 2015 at 06:39:10AM -0800, H.J. Lu wrote:
> On Wed, Mar 4, 2015 at 3:26 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > Protected symbol means that it can't be pre-emptied. It
> > doesn't mean its address won't be external. This is true
> > for pointer to protected function. With copy relocation,
> > address of protected data defined in the shared library may
> > also be external. We only know that for sure at run-time.
> > Here are patches for glibc, binutils and GCC to handle it
> > properly.
> >
> > Any comments?
>
> This is the binutils patch I checked in. It basically reverted
> the change for
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=15228
>
> on x86. Copy relocations against protected symbols should
> work.
Does it actually work now though? Last I checked gcc was generating
wrong code too -- GOT-relative accesses rather than accessing them
through the GOT. If that's the case, ld has no way to fix the problem.
Rich
On Thu, Mar 5, 2015 at 9:32 AM, Rich Felker <dalias@libc.org> wrote:
> On Thu, Mar 05, 2015 at 06:39:10AM -0800, H.J. Lu wrote:
>> On Wed, Mar 4, 2015 at 3:26 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> > Protected symbol means that it can't be pre-emptied. It
>> > doesn't mean its address won't be external. This is true
>> > for pointer to protected function. With copy relocation,
>> > address of protected data defined in the shared library may
>> > also be external. We only know that for sure at run-time.
>> > Here are patches for glibc, binutils and GCC to handle it
>> > properly.
>> >
>> > Any comments?
>>
>> This is the binutils patch I checked in. It basically reverted
>> the change for
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=15228
>>
>> on x86. Copy relocations against protected symbols should
>> work.
>
> Does it actually work now though? Last I checked gcc was generating
> wrong code too -- GOT-relative accesses rather than accessing them
> through the GOT. If that's the case, ld has no way to fix the problem.
>
You need to apply both my GCC and glibc patches in
https://gcc.gnu.org/ml/gcc-patches/2015-03/msg00257.html
On Thu, Mar 5, 2015 at 6:39 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Mar 4, 2015 at 3:26 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> Protected symbol means that it can't be pre-emptied. It
>> doesn't mean its address won't be external. This is true
>> for pointer to protected function. With copy relocation,
>> address of protected data defined in the shared library may
>> also be external. We only know that for sure at run-time.
>> Here are patches for glibc, binutils and GCC to handle it
>> properly.
>>
>> Any comments?
>
> This is the binutils patch I checked in. It basically reverted
> the change for
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=15228
>
> on x86. Copy relocations against protected symbols should
> work.
>
> --
> H.J.
> ---
> bfd/
>
> PR ld/pr15228
> PR ld/pr17709
> * elf-bfd.h (elf_backend_data): Add extern_protected_data.
> * elf32-i386.c (elf_backend_extern_protected_data): New.
> Defined to 1.
> * elf64-x86-64.c (elf_backend_extern_protected_data): Likewise.
> * elflink.c (_bfd_elf_adjust_dynamic_copy): Don't error on
> copy relocs against protected symbols if extern_protected_data
> is true.
> (_bfd_elf_symbol_refs_local_p): Don't return true on protected
> non-function symbols if extern_protected_data is true.
> * elfxx-target.h (elf_backend_extern_protected_data): New.
> Default to 0.
> (elfNN_bed): Initialize extern_protected_data with
> elf_backend_extern_protected_data.
>
> ld/testsuite/
>
> PR ld/pr15228
> PR ld/pr17709
> * ld-i386/i386.exp (i386tests): Add a test for PR ld/17709.
> * ld-i386/pr17709-nacl.rd: New file.
> * ld-i386/pr17709.rd: Likewise.
> * ld-i386/pr17709a.s: Likewise.
> * ld-i386/pr17709b.s: Likewise.
> * ld-i386/protected3.d: Updated.
> * ld-i386/protected3.s: Likewise.
> * ld-x86-64/pr17709-nacl.rd: New file.
> * ld-x86-64/pr17709.rd: Likewise.
> * ld-x86-64/pr17709a.s: Likewise.
> * ld-x86-64/pr17709b.s: Likewise.
> * ld-x86-64/protected3.d: Updated.
> * ld-x86-64/protected3.s: Likewise.
> * ld-x86-64/x86-64.exp (x86_64tests): Add a test for PR ld/17709.
I back ported it to 2.25 branch.
From ca3fe95e469b9daec153caa2c90665f5daaec2b5 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 5 Mar 2015 06:34:39 -0800
Subject: [PATCH] Add extern_protected_data and set it for x86
With copy relocation, address of protected data defined in the shared
library may be external. This patch adds extern_protected_data and
changes _bfd_elf_symbol_refs_local_p to return false for protected data
if extern_protected_data is true.
bfd/
PR ld/pr15228
PR ld/pr17709
* elf-bfd.h (elf_backend_data): Add extern_protected_data.
* elf32-i386.c (elf_backend_extern_protected_data): New.
Defined to 1.
* elf64-x86-64.c (elf_backend_extern_protected_data): Likewise.
* elflink.c (_bfd_elf_adjust_dynamic_copy): Don't error on
copy relocs against protected symbols if extern_protected_data
is true.
(_bfd_elf_symbol_refs_local_p): Don't return true on protected
non-function symbols if extern_protected_data is true.
* elfxx-target.h (elf_backend_extern_protected_data): New.
Default to 0.
(elfNN_bed): Initialize extern_protected_data with
elf_backend_extern_protected_data.
ld/testsuite/
PR ld/pr15228
PR ld/pr17709
* ld-i386/i386.exp (i386tests): Add a test for PR ld/17709.
* ld-i386/pr17709-nacl.rd: New file.
* ld-i386/pr17709.rd: Likewise.
* ld-i386/pr17709a.s: Likewise.
* ld-i386/pr17709b.s: Likewise.
* ld-i386/protected3.d: Updated.
* ld-i386/protected3.s: Likewise.
* ld-x86-64/pr17709-nacl.rd: New file.
* ld-x86-64/pr17709.rd: Likewise.
* ld-x86-64/pr17709a.s: Likewise.
* ld-x86-64/pr17709b.s: Likewise.
* ld-x86-64/protected3.d: Updated.
* ld-x86-64/protected3.s: Likewise.
* ld-x86-64/x86-64.exp (x86_64tests): Add a test for PR ld/17709.
---
bfd/ChangeLog | 18 ++++++++++++++++++
bfd/elf-bfd.h | 4 ++++
bfd/elf32-i386.c | 1 +
bfd/elf64-x86-64.c | 1 +
bfd/elflink.c | 9 ++++++---
bfd/elfxx-target.h | 6 +++++-
ld/testsuite/ChangeLog | 19 +++++++++++++++++++
ld/testsuite/ld-i386/i386.exp | 4 ++++
ld/testsuite/ld-i386/pr17709-nacl.rd | 4 ++++
ld/testsuite/ld-i386/pr17709.rd | 4 ++++
ld/testsuite/ld-i386/pr17709a.s | 8 ++++++++
ld/testsuite/ld-i386/pr17709b.s | 5 +++++
ld/testsuite/ld-i386/protected3.d | 3 ++-
ld/testsuite/ld-i386/protected3.s | 3 ++-
ld/testsuite/ld-x86-64/pr17709-nacl.rd | 4 ++++
ld/testsuite/ld-x86-64/pr17709.rd | 4 ++++
ld/testsuite/ld-x86-64/pr17709a.s | 8 ++++++++
ld/testsuite/ld-x86-64/pr17709b.s | 5 +++++
ld/testsuite/ld-x86-64/protected3.d | 3 ++-
ld/testsuite/ld-x86-64/protected3.s | 3 ++-
ld/testsuite/ld-x86-64/x86-64.exp | 4 ++++
21 files changed, 112 insertions(+), 8 deletions(-)
create mode 100644 ld/testsuite/ld-i386/pr17709-nacl.rd
create mode 100644 ld/testsuite/ld-i386/pr17709.rd
create mode 100644 ld/testsuite/ld-i386/pr17709a.s
create mode 100644 ld/testsuite/ld-i386/pr17709b.s
create mode 100644 ld/testsuite/ld-x86-64/pr17709-nacl.rd
create mode 100644 ld/testsuite/ld-x86-64/pr17709.rd
create mode 100644 ld/testsuite/ld-x86-64/pr17709a.s
create mode 100644 ld/testsuite/ld-x86-64/pr17709b.s
@@ -1,3 +1,21 @@
+2015-03-05 H.J. Lu <hongjiu.lu@intel.com>
+
+ PR ld/pr15228
+ PR ld/pr17709
+ * elf-bfd.h (elf_backend_data): Add extern_protected_data.
+ * elf32-i386.c (elf_backend_extern_protected_data): New.
+ Defined to 1.
+ * elf64-x86-64.c (elf_backend_extern_protected_data): Likewise.
+ * elflink.c (_bfd_elf_adjust_dynamic_copy): Don't error on
+ copy relocs against protected symbols if extern_protected_data
+ is true.
+ (_bfd_elf_symbol_refs_local_p): Don't return true on protected
+ non-function symbols if extern_protected_data is true.
+ * elfxx-target.h (elf_backend_extern_protected_data): New.
+ Default to 0.
+ (elfNN_bed): Initialize extern_protected_data with
+ elf_backend_extern_protected_data.
+
2015-03-05 Nick Clifton <nickc@redhat.com>
PR binutils/18025
@@ -1359,6 +1359,10 @@ struct elf_backend_data
in length rather than sec->size in length, if sec->rawsize is
non-zero and smaller than sec->size. */
unsigned caches_rawsize : 1;
+
+ /* Address of protected data defined in the shared library may be
+ external, i.e., due to copy relocation. */
+ unsigned extern_protected_data : 1;
};
/* Information about reloc sections associated with a bfd_elf_section_data
@@ -5292,6 +5292,7 @@ elf_i386_add_symbol_hook (bfd * abfd,
#define elf_backend_want_plt_sym 0
#define elf_backend_got_header_size 12
#define elf_backend_plt_alignment 4
+#define elf_backend_extern_protected_data 1
/* Support RELA for objdump of prelink objects. */
#define elf_info_to_howto elf_i386_info_to_howto_rel
@@ -5868,6 +5868,7 @@ static const struct bfd_elf_special_section
#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_info_to_howto elf_x86_64_info_to_howto
@@ -2671,7 +2671,9 @@ _bfd_elf_adjust_dynamic_copy (struct bfd_link_info *info,
/* Increment the size of DYNBSS to make room for the symbol. */
dynbss->size += h->size;
- if (h->protected_def)
+ /* No error if extern_protected_data is true. */
+ if (h->protected_def
+ && !get_elf_backend_data (dynbss->owner)->extern_protected_data)
{
info->callbacks->einfo
(_("%P: copy reloc against protected `%T' is invalid\n"),
@@ -2835,8 +2837,9 @@ _bfd_elf_symbol_refs_local_p (struct elf_link_hash_entry *h,
bed = get_elf_backend_data (hash_table->dynobj);
- /* STV_PROTECTED non-function symbols are local. */
- if (!bed->is_function_type (h->type))
+ /* If extern_protected_data is false, STV_PROTECTED non-function
+ symbols are local. */
+ if (!bed->extern_protected_data && !bed->is_function_type (h->type))
return TRUE;
/* Function pointer equality tests may require that STV_PROTECTED
@@ -117,6 +117,9 @@
#ifndef elf_backend_caches_rawsize
#define elf_backend_caches_rawsize 0
#endif
+#ifndef elf_backend_extern_protected_data
+#define elf_backend_extern_protected_data 0
+#endif
#ifndef elf_backend_stack_align
#define elf_backend_stack_align 16
#endif
@@ -801,7 +804,8 @@ static struct elf_backend_data elfNN_bed =
elf_backend_want_dynbss,
elf_backend_want_p_paddr_set_to_zero,
elf_backend_default_execstack,
- elf_backend_caches_rawsize
+ elf_backend_caches_rawsize,
+ elf_backend_extern_protected_data
};
/* Forward declaration for use when initialising alternative_target field. */
@@ -1,3 +1,22 @@
+2015-03-05 H.J. Lu <hongjiu.lu@intel.com>
+
+ PR ld/pr15228
+ PR ld/pr17709
+ * ld-i386/i386.exp (i386tests): Add a test for PR ld/17709.
+ * ld-i386/pr17709-nacl.rd: New file.
+ * ld-i386/pr17709.rd: Likewise.
+ * ld-i386/pr17709a.s: Likewise.
+ * ld-i386/pr17709b.s: Likewise.
+ * ld-i386/protected3.d: Updated.
+ * ld-i386/protected3.s: Likewise.
+ * ld-x86-64/pr17709-nacl.rd: New file.
+ * ld-x86-64/pr17709.rd: Likewise.
+ * ld-x86-64/pr17709a.s: Likewise.
+ * ld-x86-64/pr17709b.s: Likewise.
+ * ld-x86-64/protected3.d: Updated.
+ * ld-x86-64/protected3.s: Likewise.
+ * ld-x86-64/x86-64.exp (x86_64tests): Add a test for PR ld/17709.
+
2015-03-04 Richard Sandiford <richard.sandiford@arm.com>
PR gas/17843
@@ -191,6 +191,10 @@ set i386tests {
"--32" {pr17306b.s} {} ""}
{"PR ld/17306 (2)" "-melf_i386 -shared -Bsymbolic --just-symbols=tmpdir/pr17306b.o" ""
"--32" {pr17306a.s} {} "libpr17306.so"}
+ {"PR ld/17709 (1)" "-melf_i386 -shared" ""
+ "--32" {pr17709a.s} {} "libpr17709.so"}
+ {"PR ld/17709 (2)" "-melf_i386 tmpdir/libpr17709.so" ""
+ "--32" {pr17709b.s} {{readelf -r pr17709.rd}} "pr17709"}
}
# So as to avoid rewriting every last test case here in a nacl variant,
new file mode 100644
@@ -0,0 +1,4 @@
+
+Relocation section '.rel\..*' at offset .* contains 1 entries:
+ Offset Info Type Sym\.Value Sym\. Name
+[0-9a-f ]+R_386_COPY +[0-9a-f]+ +foo
new file mode 100644
@@ -0,0 +1,4 @@
+
+Relocation section '.rel\..*' at offset .* contains 1 entries:
+ Offset Info Type Sym\.Value Sym\. Name
+[0-9a-f ]+R_386_COPY +[0-9a-f]+ +foo
new file mode 100644
@@ -0,0 +1,8 @@
+ .protected foo
+.globl foo
+ .data
+ .align 4
+ .type foo, @object
+ .size foo, 4
+foo:
+ .long 1
new file mode 100644
@@ -0,0 +1,5 @@
+ .text
+ .globl _start
+ .type _start, @function
+_start:
+ movl foo, %eax
@@ -8,6 +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] 00 00 mov 0x[a-f0-9]+\(%ecx\),%eax
+[ ]*[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]+: 8b 00 mov \(%eax\),%eax
[ ]*[a-f0-9]+: c3 ret
#pass
@@ -10,6 +10,7 @@ foo:
.globl bar
.type bar, @function
bar:
- movl foo@GOTOFF(%ecx), %eax
+ movl foo@GOT(%ecx), %eax
+ movl (%eax), %eax
ret
.size bar, .-bar
new file mode 100644
@@ -0,0 +1,4 @@
+
+Relocation section '.rela\..*' at offset .* contains 1 entries:
+ +Offset +Info +Type +Symbol's Value +Symbol's Name \+ Addend
+[0-9a-f ]+R_X86_64_COPY+[0-9a-f ]+ +foo \+ 0
new file mode 100644
@@ -0,0 +1,4 @@
+
+Relocation section '.rela\..*' at offset .* contains 1 entries:
+ +Offset +Info +Type +Symbol's Value +Symbol's Name \+ Addend
+[0-9a-f ]+R_X86_64_COPY+[0-9a-f ]+ +foo \+ 0
new file mode 100644
@@ -0,0 +1,8 @@
+ .protected foo
+.globl foo
+ .data
+ .align 4
+ .type foo, @object
+ .size foo, 4
+foo:
+ .long 1
new file mode 100644
@@ -0,0 +1,5 @@
+ .text
+ .globl _start
+ .type _start, @function
+_start:
+ movl foo, %eax
@@ -8,6 +8,7 @@
Disassembly of section .text:
0+[a-f0-9]+ <bar>:
-[ ]*[a-f0-9]+: 8b 05 ([0-9a-f]{2} ){4} * mov 0x[a-f0-9]+\(%rip\),%eax # [a-f0-9]+ <foo>
+[ ]*[a-f0-9]+: 48 8b 05 ([0-9a-f]{2} ){4} * mov 0x[a-f0-9]+\(%rip\),%rax # [a-f0-9]+ <_DYNAMIC\+0x[a-f0-9]+>
+[ ]*[a-f0-9]+: 8b 00 mov \(%rax\),%eax
[ ]*[a-f0-9]+: c3 retq *
#pass
@@ -10,6 +10,7 @@ foo:
.globl bar
.type bar, @function
bar:
- movl foo(%rip), %eax
+ movq foo@GOTPCREL(%rip), %rax
+ movl (%rax), %eax
ret
.size bar, .-bar
@@ -144,6 +144,10 @@ set x86_64tests {
"--64" {pr17306b.s} {} ""}
{"PR ld/17306 (2)" "-melf_x86_64 -shared -Bsymbolic --just-symbols=tmpdir/pr17306b.o" ""
"--64" {pr17306a.s} {} "libpr17306.so"}
+ {"PR ld/17709 (1)" "-melf_x86_64 -shared" ""
+ "--64" {pr17709a.s} {} "libpr17709.so"}
+ {"PR ld/17709 (2)" "-melf_x86_64 tmpdir/libpr17709.so" ""
+ "--64" {pr17709b.s} {{readelf -rW pr17709.rd}} "pr17709"}
}
# So as to avoid rewriting every last test case here in a nacl variant,
--
2.1.0