Bug 15228 - copy relocations against protected symbols should be disallowed
Summary: copy relocations against protected symbols should be disallowed
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: 2.26
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-05 00:53 UTC by Andy Lutomirski
Modified: 2015-04-03 23:31 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Lutomirski 2013-03-05 00:53:59 UTC
Take this code:

-- begin lib.cc --
struct A
{
  A() : x(1) {}
  int x;
};

__attribute__((visibility("protected"))) A a;
-- end lib.cc --

--begin main.cc--
struct A
{
  A() : x(1) {}
  int x;
};

extern A a;

extern "C" void abort(void);

int main()
{
  if (a.x != 1)
    abort();
}
--end main.cc--

and build it with:

$ g++ -shared -fPIC -o lib.so lib.cc
$ g++ lib.so main.cc

ld sees a protected defined object called a of size 4 in lib.so and an undefined object called a in main.o.  It helpfully generates a copy relocation.  The resulting code cannot possibly work, and, in fact, it aborts.  Everything works fine with -fPIE.

IMO it would be much better to fail to link than to produce a binary that ld.so is happy to load (maybe that's an ld bug) but that is more or less guaranteed to execute incorrectly.


(FWIW, Windows has solved this problem using dllimport for many years, but that's another story.)
Comment 1 cvs-commit@gcc.gnu.org 2014-12-12 13:07:48 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  6cabe1ea460c54c17ac877b2541eccf91d6b4b9c (commit)
      from  21daaaaffcbda47b724858dd99ee2082043ef2da (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6cabe1ea460c54c17ac877b2541eccf91d6b4b9c

commit 6cabe1ea460c54c17ac877b2541eccf91d6b4b9c
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Dec 12 22:53:46 2014 +1030

    Copy relocations against protected symbols
    
    Copy relocs are used in a scheme to avoid dynamic text relocations in
    non-PIC executables that refer to variables defined in shared
    libraries.  The idea is to have the linker define any such variable in
    the executable, with a copy reloc copying the initial value, then have
    both the executable and shared library refer to the executable copy.
    If the shared library defines the variable as protected then we have
    two copies of the variable being used.
    
    	PR 15228
    	* elflink.c (_bfd_elf_adjust_dynamic_copy): Add "info" param.
    	Error on copy relocs against protected symbols.
    	(elf_merge_st_other): Set h->protected_def.
    	* elf-bfd.h (struct elf_link_hash_entry): Add "protected_def".
    	(_bfd_elf_adjust_dynamic_copy): Update prototype.
    	* elf-m10300.c (_bfd_mn10300_elf_adjust_dynamic_symbol): Update
    	_bfd_elf_adjust_dynamic_copy call.
    	* elf32-arm.c (elf32_arm_adjust_dynamic_symbol): Likewise.
    	* elf32-cr16.c (_bfd_cr16_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-cris.c (elf_cris_adjust_dynamic_symbol): Likewise.
    	* elf32-hppa.c (elf32_hppa_adjust_dynamic_symbol): Likewise.
    	* elf32-i370.c (i370_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-i386.c (elf_i386_adjust_dynamic_symbol): Likewise.
    	* elf32-lm32.c (lm32_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-m32r.c (m32r_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-m68k.c (elf_m68k_adjust_dynamic_symbol): Likewise.
    	* elf32-metag.c (elf_metag_adjust_dynamic_symbol): Likewise.
    	* elf32-or1k.c (or1k_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-ppc.c (ppc_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-s390.c (elf_s390_adjust_dynamic_symbol): Likewise.
    	* elf32-sh.c (sh_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-tic6x.c (elf32_tic6x_adjust_dynamic_symbol): Likewise.
    	* elf32-tilepro.c (tilepro_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-vax.c (elf_vax_adjust_dynamic_symbol): Likewise.
    	* elf64-ppc.c (ppc64_elf_adjust_dynamic_symbol): Likewise.
    	* elf64-s390.c (elf_s390_adjust_dynamic_symbol): Likewise.
    	* elf64-sh64.c (sh64_elf64_adjust_dynamic_symbol): Likewise.
    	* elf64-x86-64.c (elf_x86_64_adjust_dynamic_symbol): Likewise.
    	* elfnn-aarch64.c (elfNN_aarch64_adjust_dynamic_symbol): Likewise.
    	* elfxx-mips.c (_bfd_mips_elf_adjust_dynamic_symbol): Likewise.
    	* elfxx-sparc.c (_bfd_sparc_elf_adjust_dynamic_symbol): Likewise.
    	* elfxx-tilegx.c (tilegx_elf_adjust_dynamic_symbol): Likewise.

-----------------------------------------------------------------------

Summary of changes:
 bfd/ChangeLog       |   36 ++++++++++++++++++++++++++++++++++++
 bfd/elf-bfd.h       |    4 +++-
 bfd/elf-m10300.c    |    2 +-
 bfd/elf32-arm.c     |    2 +-
 bfd/elf32-cr16.c    |    2 +-
 bfd/elf32-cris.c    |    2 +-
 bfd/elf32-hppa.c    |    2 +-
 bfd/elf32-i370.c    |    2 +-
 bfd/elf32-i386.c    |    2 +-
 bfd/elf32-lm32.c    |    2 +-
 bfd/elf32-m32r.c    |    2 +-
 bfd/elf32-m68k.c    |    2 +-
 bfd/elf32-metag.c   |    2 +-
 bfd/elf32-or1k.c    |    2 +-
 bfd/elf32-ppc.c     |    2 +-
 bfd/elf32-s390.c    |    2 +-
 bfd/elf32-sh.c      |    2 +-
 bfd/elf32-tic6x.c   |    2 +-
 bfd/elf32-tilepro.c |    2 +-
 bfd/elf32-vax.c     |    2 +-
 bfd/elf64-ppc.c     |    2 +-
 bfd/elf64-s390.c    |    2 +-
 bfd/elf64-sh64.c    |    2 +-
 bfd/elf64-x86-64.c  |    2 +-
 bfd/elflink.c       |   13 ++++++++++++-
 bfd/elfnn-aarch64.c |    2 +-
 bfd/elfxx-mips.c    |    2 +-
 bfd/elfxx-sparc.c   |    2 +-
 bfd/elfxx-tilegx.c  |    2 +-
 29 files changed, 77 insertions(+), 28 deletions(-)
Comment 2 cvs-commit@gcc.gnu.org 2014-12-12 13:16:58 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  de287215cef5f4271367f75c557c1af788892e69 (commit)
      from  e5a9158d093d53f2bb1057359ac381dcdf6d4305 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=de287215cef5f4271367f75c557c1af788892e69

commit de287215cef5f4271367f75c557c1af788892e69
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Dec 12 23:39:14 2014 +1030

    Set bfd_error in _bfd_elf_adjust_dynamic_copy
    
    	PR 15228
    	* elflink.c (_bfd_elf_adjust_dynamic_copy): Call bfd_set_error.

-----------------------------------------------------------------------

Summary of changes:
 bfd/ChangeLog |    5 +++++
 bfd/elflink.c |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)
Comment 3 Alan Modra 2014-12-13 03:04:56 UTC
Fixed
Comment 4 cvs-commit@gcc.gnu.org 2015-02-11 12:49:16 UTC
The binutils-2_25-branch branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9175ae119052a791bc56daa2dd653d06ae2167c2

commit 9175ae119052a791bc56daa2dd653d06ae2167c2
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Dec 12 22:53:46 2014 +1030

    Copy relocations against protected symbols
    
    Copy relocs are used in a scheme to avoid dynamic text relocations in
    non-PIC executables that refer to variables defined in shared
    libraries.  The idea is to have the linker define any such variable in
    the executable, with a copy reloc copying the initial value, then have
    both the executable and shared library refer to the executable copy.
    If the shared library defines the variable as protected then we have
    two copies of the variable being used.
    
    	PR 15228
    	* elflink.c (_bfd_elf_adjust_dynamic_copy): Add "info" param.
    	Error on copy relocs against protected symbols.
    	(elf_merge_st_other): Set h->protected_def.
    	* elf-bfd.h (struct elf_link_hash_entry): Add "protected_def".
    	(_bfd_elf_adjust_dynamic_copy): Update prototype.
    	* elf-m10300.c (_bfd_mn10300_elf_adjust_dynamic_symbol): Update
    	_bfd_elf_adjust_dynamic_copy call.
    	* elf32-arm.c (elf32_arm_adjust_dynamic_symbol): Likewise.
    	* elf32-cr16.c (_bfd_cr16_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-cris.c (elf_cris_adjust_dynamic_symbol): Likewise.
    	* elf32-hppa.c (elf32_hppa_adjust_dynamic_symbol): Likewise.
    	* elf32-i370.c (i370_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-i386.c (elf_i386_adjust_dynamic_symbol): Likewise.
    	* elf32-lm32.c (lm32_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-m32r.c (m32r_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-m68k.c (elf_m68k_adjust_dynamic_symbol): Likewise.
    	* elf32-metag.c (elf_metag_adjust_dynamic_symbol): Likewise.
    	* elf32-or1k.c (or1k_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-ppc.c (ppc_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-s390.c (elf_s390_adjust_dynamic_symbol): Likewise.
    	* elf32-sh.c (sh_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-tic6x.c (elf32_tic6x_adjust_dynamic_symbol): Likewise.
    	* elf32-tilepro.c (tilepro_elf_adjust_dynamic_symbol): Likewise.
    	* elf32-vax.c (elf_vax_adjust_dynamic_symbol): Likewise.
    	* elf64-ppc.c (ppc64_elf_adjust_dynamic_symbol): Likewise.
    	* elf64-s390.c (elf_s390_adjust_dynamic_symbol): Likewise.
    	* elf64-sh64.c (sh64_elf64_adjust_dynamic_symbol): Likewise.
    	* elf64-x86-64.c (elf_x86_64_adjust_dynamic_symbol): Likewise.
    	* elfnn-aarch64.c (elfNN_aarch64_adjust_dynamic_symbol): Likewise.
    	* elfxx-mips.c (_bfd_mips_elf_adjust_dynamic_symbol): Likewise.
    	* elfxx-sparc.c (_bfd_sparc_elf_adjust_dynamic_symbol): Likewise.
    	* elfxx-tilegx.c (tilegx_elf_adjust_dynamic_symbol): Likewise.
Comment 5 cvs-commit@gcc.gnu.org 2015-02-11 12:49:36 UTC
The binutils-2_25-branch branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=bfd74fb20fc4cade1c36f6c40888b22cac75fa76

commit bfd74fb20fc4cade1c36f6c40888b22cac75fa76
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Dec 12 23:39:14 2014 +1030

    Set bfd_error in _bfd_elf_adjust_dynamic_copy
    
    	PR 15228
    	* elflink.c (_bfd_elf_adjust_dynamic_copy): Call bfd_set_error.
Comment 6 cvs-commit@gcc.gnu.org 2015-03-27 05:53:07 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b84171287ffe60dd1e7c02262a0493862fa21a97

commit b84171287ffe60dd1e7c02262a0493862fa21a97
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Mar 27 15:41:05 2015 +1030

    Relax PR 15228 protected visibility restriction
    
    Allows .dynbss copy of shared library protected visibility variables
    if they are read-only.
    
    To recap: Copying a variable from a shared library into an executable's
    .dynbss is an old hack invented for non-PIC executables, to avoid the
    text relocations you'd otherwise need to access a shared library
    variable.  This works with ELF shared libraries because global
    symbols can be overridden.  The trouble is that protected visibility
    symbols can't be overridden.  A shared library will continue to access
    it's own protected visibility variable while the executable accesses a
    copy.  If either the shared library or the executable updates the
    value then the copy diverges from the original.  This is wrong since
    there is only one definition of the variable in the application.
    
    So I made the linker report an error on attempting to copy protected
    visibility variables into .dynbss.  However, you'll notice the above
    paragraph contains an "If".  An application that does not modify the
    variable value remains correct even though two copies of the variable
    exist.  The linker can detect this situation if the variable was
    defined in a read-only section.
    
    	PR ld/15228
    	PR ld/18167
    	* elflink.c (elf_merge_st_other): Add "sec" parameter.  Don't set
    	protected_def when symbol section is read-only.  Adjust all calls.
    	* elf-bfd.h (struct elf_link_hash_entry): Update protected_def comment.
Comment 7 cvs-commit@gcc.gnu.org 2015-04-03 23:31:20 UTC
The binutils-2_25-branch branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=12aca65e0f9148bf136a9f4cfc2187aa485a143a

commit 12aca65e0f9148bf136a9f4cfc2187aa485a143a
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Mar 27 15:41:05 2015 +1030

    Relax PR 15228 protected visibility restriction
    
    Allows .dynbss copy of shared library protected visibility variables
    if they are read-only.
    
    To recap: Copying a variable from a shared library into an executable's
    .dynbss is an old hack invented for non-PIC executables, to avoid the
    text relocations you'd otherwise need to access a shared library
    variable.  This works with ELF shared libraries because global
    symbols can be overridden.  The trouble is that protected visibility
    symbols can't be overridden.  A shared library will continue to access
    it's own protected visibility variable while the executable accesses a
    copy.  If either the shared library or the executable updates the
    value then the copy diverges from the original.  This is wrong since
    there is only one definition of the variable in the application.
    
    So I made the linker report an error on attempting to copy protected
    visibility variables into .dynbss.  However, you'll notice the above
    paragraph contains an "If".  An application that does not modify the
    variable value remains correct even though two copies of the variable
    exist.  The linker can detect this situation if the variable was
    defined in a read-only section.
    
    	PR ld/15228
    	PR ld/18167
    	* elflink.c (elf_merge_st_other): Add "sec" parameter.  Don't set
    	protected_def when symbol section is read-only.  Adjust all calls.
    	* elf-bfd.h (struct elf_link_hash_entry): Update protected_def comment.