RFC: PATCH: PR x86_64/584: Detect call on protected symbol

RFC: PATCH: PR x86_64/584: Detect call on protected symbol

[H. J. Lu]

X86_64 uses R_X86_64_PC32 for both branch and store/load. Linker
can't tell if a protected symbol reference is local or global just
by relocation. This patch disassembles the code to check for call.


H.J.
----
bfd/

2005-01-19  H.J. Lu  <hongjiu.lu@intel.com>

	PR 584
	* elf64-x86-64.c: Include "dis-asm.h" and "libiberty.h".
	(elf64_x86_64_disasm): New.
	(elf64_x86_64_relocate_section): Disassembly the instruction
	for R_X86_64_PC32 relocation on a protected function symbol
	when building shared library for call instruction.

ld/

2005-01-19  H.J. Lu  <hongjiu.lu@intel.com>

	PR 584
	* Makefile.am (BFDLIB): Change it to @BFDLIB@.
	(eelf_x86_64.c): Also depend on $(srcdir)/emultempl/needdisasm.em.
	(eelf_x86_64_fbsd.c): Likewise.

	* configure.in (BFDLIB): New. Substitute
	(TESTBFDLIB): Updated.

	* configure.tgt (need_opcodes): Set to "yes" for ELF/x86_64.

	* emulparams/elf_x86_64.sh (EXTRA_EM_FILE): Set to needdisasm.

	* emultempl/needdisasm.em: New.

	* Makefile.in: Regenerated.
	* configure: Likewise.

--- binutils/bfd/elf64-x86-64.c.prot	2005-01-11 09:10:28.000000000 -0800
+++ binutils/bfd/elf64-x86-64.c	2005-01-19 13:02:51.582973722 -0800
@@ -25,6 +25,16 @@
 #include "libbfd.h"
 #include "elf-bfd.h"
 
+/* Used to check which instruction R_X86_64_PC32 is used for.  */
+#include "dis-asm.h"
+#include "libiberty.h"
+
+extern void init_disassemble_info
+  (struct disassemble_info *, void *, fprintf_ftype) __attribute__((weak));
+extern void disassemble_init_for_target
+  (struct disassemble_info *) __attribute__((weak));
+extern disassembler_ftype disassembler (bfd *) __attribute__((weak));
+
 #include "elf/x86-64.h"
 
 /* In case we're on a 32-bit machine, construct a 64-bit "-1" value.  */
@@ -1745,6 +1755,12 @@ tpoff (struct bfd_link_info *info, bfd_v
   return address - htab->tls_size - htab->tls_sec->vma;
 }
 
+static int
+elf64_x86_64_disasm (void)
+{
+  return 0;
+}
+
 /* Relocate an x86_64 ELF section.  */
 
 static bfd_boolean
@@ -1760,6 +1776,7 @@ elf64_x86_64_relocate_section (bfd *outp
   bfd_vma *local_got_offsets;
   Elf_Internal_Rela *rel;
   Elf_Internal_Rela *relend;
+  bfd_vma last_offset;
 
   if (info->relocatable)
     return TRUE;
@@ -1769,6 +1786,7 @@ elf64_x86_64_relocate_section (bfd *outp
   sym_hashes = elf_sym_hashes (input_bfd);
   local_got_offsets = elf_local_got_offsets (input_bfd);
 
+  last_offset = 0;
   rel = relocs;
   relend = relocs + input_section->reloc_count;
   for (; rel < relend; rel++)
@@ -1952,13 +1970,72 @@ elf64_x86_64_relocate_section (bfd *outp
 	      && (input_section->flags & SEC_ALLOC) != 0
 	      && (input_section->flags & SEC_READONLY) != 0)
 	    {
-	      (*_bfd_error_handler)
-		(_("%B: relocation %s against `%s' can not be used when making a shared object; recompile with -fPIC"),
-		 input_bfd,
-		 x86_64_elf_howto_table[r_type].name,
-		 (h) ? h->root.root.string : "a local symbol");
-	      bfd_set_error (bfd_error_bad_value);
-	      return FALSE;
+	      bfd_boolean call;
+	      int octets = -1;
+
+	      if (!init_disassemble_info)
+		call = FALSE;
+	      else if (h->def_regular
+		       && r_type == R_X86_64_PC32
+		       && h->type == STT_FUNC
+		       && ELF_ST_VISIBILITY (h->other) == STV_PROTECTED)
+		{
+		  static struct disassemble_info *dis;
+		  static disassembler_ftype dis_fn;
+		  bfd_vma offset;
+
+		  if (dis == NULL && init_disassemble_info)
+		    {
+		      dis = xmalloc (sizeof (*dis));
+		      init_disassemble_info (dis, NULL,
+					    (fprintf_ftype) elf64_x86_64_disasm);
+		      dis->flavour = bfd_get_flavour (input_bfd);
+		      dis->arch = bfd_get_arch (input_bfd);
+		      dis->mach = bfd_get_mach (input_bfd);
+		      dis->octets_per_byte = bfd_octets_per_byte (input_bfd);
+		      if (bfd_big_endian (input_bfd))
+			dis->display_endian = dis->endian = BFD_ENDIAN_BIG;
+		      else
+			dis->display_endian = dis->endian = BFD_ENDIAN_LITTLE;
+		      disassemble_init_for_target (dis);
+		      dis_fn = disassembler (input_bfd);
+		    }
+
+		  dis->buffer = contents;
+		  dis->buffer_vma = input_section->vma;
+		  dis->buffer_length = input_section->size;
+		  dis->section = input_section;
+
+		  for (offset = last_offset; offset < rel->r_offset;)
+		    {
+		      octets = (*dis_fn) (input_section->vma + offset,
+					  dis);
+		      if (octets < 0)
+			break;
+		      else
+			offset += octets / dis->octets_per_byte;
+		    }
+
+		  /* Check if the current instruction is call.  */
+		  if (octets > 0
+		      && *(contents + offset - octets) == 0xe8)
+		    call = TRUE;
+		  else
+		    call = FALSE;
+		}
+	      else
+		call = FALSE;
+
+	      if (! call)
+		{
+		  (*_bfd_error_handler)
+		    (_("%B: relocation %s against `%s' can not be used when making a shared object; recompile with -fPIC"),
+		     input_bfd,
+		     x86_64_elf_howto_table[r_type].name,
+		     (h) ? h->root.root.string : "a local symbol");
+		  bfd_set_error (bfd_error_bad_value);
+		  return FALSE;
+		}
 	    }
 	  /* Fall through.  */
 
@@ -2454,6 +2531,11 @@ elf64_x86_64_relocate_section (bfd *outp
 	      return FALSE;
 	    }
 	}
+
+      if (last_offset == 0)
+	last_offset = rel->r_offset;
+      else if (rel->r_offset < last_offset)
+	last_offset = rel->r_offset;
     }
 
   return TRUE;
--- binutils/ld/Makefile.am.prot	2005-01-19 09:08:58.000000000 -0800
+++ binutils/ld/Makefile.am	2005-01-19 13:47:14.696641705 -0800
@@ -104,7 +104,7 @@ man_MANS = ld.1
 
 INCLUDES = -D_GNU_SOURCE -I. -I$(srcdir) -I../bfd -I$(BFDDIR) -I$(INCDIR) -I$(top_srcdir)/../intl -I../intl $(HDEFINES) $(CFLAGS) -DLOCALEDIR="\"$(datadir)/locale\""
 
-BFDLIB = ../bfd/libbfd.la
+BFDLIB = @BFDLIB@
 LIBIBERTY = ../libiberty/libiberty.a
 
 ALL_EMULATIONS = \
@@ -843,11 +843,13 @@ eelf_i386.c: $(srcdir)/emulparams/elf_i3
   $(srcdir)/emultempl/elf32.em $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
 	${GENSCRIPTS} elf_i386 "$(tdir_elf_i386)"
 eelf_x86_64.c: $(srcdir)/emulparams/elf_x86_64.sh \
-  $(srcdir)/emultempl/elf32.em $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
+  $(srcdir)/emultempl/elf32.em $(srcdir)/emultempl/needdisasm.em \
+  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
 	${GENSCRIPTS} elf_x86_64 "$(tdir_elf_x86_64)"
 eelf_x86_64_fbsd.c: $(srcdir)/emulparams/elf_x86_64_fbsd.sh \
   $(srcdir)/emulparams/elf_x86_64.sh \
-  $(srcdir)/emultempl/elf32.em $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
+  $(srcdir)/emultempl/elf32.em $(srcdir)/emultempl/needdisasm.em \
+  $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
 	${GENSCRIPTS} elf_x86_64_fbsd "$(tdir_elf_x86_64_fbsd)"
 eelf_i386_be.c: $(srcdir)/emulparams/elf_i386_be.sh \
   $(srcdir)/emultempl/elf32.em $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
--- binutils/ld/configure.in.prot	2005-01-17 09:32:46.000000000 -0800
+++ binutils/ld/configure.in	2005-01-19 14:30:50.926558757 -0800
@@ -250,10 +250,26 @@ AC_SUBST(LIB_PATH)
 EMULATION_LIBPATH=$all_libpath
 AC_SUBST(EMULATION_LIBPATH)
 
+# do we need the opcodes library?
+case "${need_opcodes}" in
+yes)
+  BFDLIB="../bfd/libbfd.la ../opcodes/libopcodes.la"
+  if test x${enable_static} = xno; then
+    TESTBFDLIB="--rpath ../opcodes/.libs ../opcodes/.libs/libopcodes.so"
+  else
+    TESTBFDLIB="../opcodes/.libs/libopcodes.a"
+  fi
+  ;;
+*)
+  BFDLIB="../bfd/libbfd.la"
+  ;;
+esac
+AC_SUBST(BFDLIB)
+
 if test x${enable_static} = xno; then
-  TESTBFDLIB="--rpath ../bfd/.libs ../bfd/.libs/libbfd.so"
+  TESTBFDLIB="--rpath ../bfd/.libs ../bfd/.libs/libbfd.so $TESTBFDLIB"
 else
-  TESTBFDLIB="../bfd/.libs/libbfd.a"
+  TESTBFDLIB="../bfd/.libs/libbfd.a $TESTBFDLIB"
 fi
 AC_SUBST(TESTBFDLIB)
 
--- binutils/ld/configure.tgt.prot	2005-01-19 09:08:58.000000000 -0800
+++ binutils/ld/configure.tgt	2005-01-19 13:43:01.696239040 -0800
@@ -11,6 +11,9 @@
 #  targ_extra_ofiles	additional objects needed by the emulation
 #  NATIVE_LIB_DIRS	library directories to search on this host
 #			(if we are a native or sysrooted linker)
+#  need_opcodes		if libopcodes is needed.
+
+need_opcodes=no
 
 targ_extra_emuls=
 targ_extra_ofiles=
@@ -165,6 +168,7 @@ i[3-7]86-*-linux-gnu*)	targ_emul=elf_i38
 			targ_extra_emuls=i386linux
 			if test x${want64} = xtrue; then
 			  targ_extra_emuls="$targ_extra_emuls elf_x86_64"
+			  need_opcodes=yes
 			fi
 			tdir_i386linux=${targ_alias}aout
 			;;
@@ -173,10 +177,12 @@ x86_64-*-linux-gnu*)	targ_emul=elf_x86_6
 			targ_extra_libpath=elf_i386
 			tdir_i386linux=`echo ${targ_alias}aout | sed -e 's/x86_64/i386/'`
 			tdir_elf_i386=`echo ${targ_alias} | sed -e 's/x86_64/i386/'`
+			need_opcodes=yes
 			;;
 i[3-7]86-*-sysv[45]*)	targ_emul=elf_i386 ;;
 i[3-7]86-*-solaris2*)	targ_emul=elf_i386_ldso
                         targ_extra_emuls="elf_i386 elf_x86_64"
+			need_opcodes=yes
                         ;;
 i[3-7]86-*-unixware)	targ_emul=elf_i386 ;;
 i[3-7]86-*-solaris*)	targ_emul=elf_i386_ldso
@@ -205,6 +211,7 @@ x86_64-*-netbsd*)	targ_emul=elf_x86_64
 				    sed -e 's/netbsd/netbsdelf/'`
 				;;
 			esac
+			need_opcodes=yes
 			;;
 i[3-7]86-*-netware)	targ_emul=i386nw ;;
 i[3-7]86-*-elf*)	targ_emul=elf_i386 ;;
@@ -219,6 +226,7 @@ x86_64-*-freebsd* | x86_64-*-kfreebsd*-g
 			targ_extra_emuls="elf_i386_fbsd elf_x86_64 elf_i386"
 			tdir_elf_i386=`echo ${targ_alias} \
 			    | sed -e 's/x86_64/i386/'`
+			need_opcodes=yes
 			;;
 i[3-7]86-*-sysv*)	targ_emul=i386coff ;;
 i[3-7]86-*-ptx*)	targ_emul=i386coff ;;
--- binutils/ld/emulparams/elf_x86_64.sh.prot	2004-05-11 13:34:35.000000000 -0700
+++ binutils/ld/emulparams/elf_x86_64.sh	2005-01-19 13:30:54.792473055 -0800
@@ -13,6 +13,7 @@ GENERATE_SHLIB_SCRIPT=yes
 GENERATE_PIE_SCRIPT=yes
 NO_SMALL_DATA=yes
 SEPARATE_GOTPLT=24
+EXTRA_EM_FILE=needdisasm
 
 if [ "x${host}" = "x${target}" ]; then
   case " $EMULATION_LIBPATH " in
--- binutils/ld/emultempl/needdisasm.em.prot	2005-01-19 13:30:02.508198053 -0800
+++ binutils/ld/emultempl/needdisasm.em	2005-01-19 13:36:40.558345995 -0800
@@ -0,0 +1,34 @@
+# This shell script emits a C file. -*- C -*-
+#   Copyright 2004 Free Software Foundation, Inc.
+#
+# This file is part of GLD, the Gnu Linker.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+#
+
+# This file is sourced from elf32.em.  It is used by targets which
+# need disassembler.
+
+cat >>e${EMULATION_NAME}.c <<EOF
+
+#include "dis-asm.h"
+
+void (*disassembler_dummy []) (void) =
+{
+  (void (*) (void)) init_disassemble_info,
+  (void (*) (void)) disassemble_init_for_target,
+  (void (*) (void)) disassembler,
+};
+EOF

Re: RFC: PATCH: PR x86_64/584: Detect call on protected symbol

[Jakub Jelinek]

On Wed, Jan 19, 2005 at 02:48:34PM -0800, H. J. Lu wrote:
> X86_64 uses R_X86_64_PC32 for both branch and store/load. Linker
> can't tell if a protected symbol reference is local or global just
> by relocation. This patch disassembles the code to check for call.

Eh, why you need the disassembler there?
Isn't it enough just to check if R_X86_64_PC32's r_offset > 0 and
contents[r_offset - 1] == 0xe8?

	Jakub

Re: RFC: PATCH: PR x86_64/584: Detect call on protected symbol

[H. J. Lu]

On Thu, Jan 20, 2005 at 12:05:44AM +0100, Jakub Jelinek wrote:
> On Wed, Jan 19, 2005 at 02:48:34PM -0800, H. J. Lu wrote:
> > X86_64 uses R_X86_64_PC32 for both branch and store/load. Linker
> > can't tell if a protected symbol reference is local or global just
> > by relocation. This patch disassembles the code to check for call.
> 
> Eh, why you need the disassembler there?
> Isn't it enough just to check if R_X86_64_PC32's r_offset > 0 and
> contents[r_offset - 1] == 0xe8?

Can we be sure that contents[r_offset - 1] == 0xe8 will be call?


H.J.

Re: RFC: PATCH: PR x86_64/584: Detect call on protected symbol

[H. J. Lu]

On Wed, Jan 19, 2005 at 04:40:32PM -0800, H. J. Lu wrote:
> On Thu, Jan 20, 2005 at 12:05:44AM +0100, Jakub Jelinek wrote:
> > On Wed, Jan 19, 2005 at 02:48:34PM -0800, H. J. Lu wrote:
> > > X86_64 uses R_X86_64_PC32 for both branch and store/load. Linker
> > > can't tell if a protected symbol reference is local or global just
> > > by relocation. This patch disassembles the code to check for call.
> > 
> > Eh, why you need the disassembler there?
> > Isn't it enough just to check if R_X86_64_PC32's r_offset > 0 and
> > contents[r_offset - 1] == 0xe8?
> 
> Can we be sure that contents[r_offset - 1] == 0xe8 will be call?
> 
> 

If we are, here is a patch.


H.J.
----
2005-01-19  H.J. Lu  <hongjiu.lu@intel.com>

	PR 584
	* elf64-x86-64.c (elf64_x86_64_relocate_section): Alllow
	R_X86_64_PC32 on a protected function symbol when building
	shared library for call instruction.

--- bfd/elf64-x86-64.c.prot	2005-01-11 09:10:28.000000000 -0800
+++ bfd/elf64-x86-64.c	2005-01-19 16:47:36.561055001 -0800
@@ -1950,7 +1950,12 @@ elf64_x86_64_relocate_section (bfd *outp
 	  if (info->shared
 	      && !SYMBOL_REFERENCES_LOCAL (info, h)
 	      && (input_section->flags & SEC_ALLOC) != 0
-	      && (input_section->flags & SEC_READONLY) != 0)
+	      && (input_section->flags & SEC_READONLY) != 0
+	      && (!h->def_regular
+		  || r_type != R_X86_64_PC32
+		  || h->type != STT_FUNC
+		  || ELF_ST_VISIBILITY (h->other) != STV_PROTECTED
+		  || contents [rel->r_offset - 1] != 0xe8))
 	    {
 	      (*_bfd_error_handler)
 		(_("%B: relocation %s against `%s' can not be used when making a shared object; recompile with -fPIC"),

Re: RFC: PATCH: PR x86_64/584: Detect call on protected symbol

[Alan Modra]

On Wed, Jan 19, 2005 at 04:40:32PM -0800, H. J. Lu wrote:
> On Thu, Jan 20, 2005 at 12:05:44AM +0100, Jakub Jelinek wrote:
> > On Wed, Jan 19, 2005 at 02:48:34PM -0800, H. J. Lu wrote:
> > > X86_64 uses R_X86_64_PC32 for both branch and store/load. Linker
> > > can't tell if a protected symbol reference is local or global just
> > > by relocation. This patch disassembles the code to check for call.
> > 
> > Eh, why you need the disassembler there?
> > Isn't it enough just to check if R_X86_64_PC32's r_offset > 0 and
> > contents[r_offset - 1] == 0xe8?
> 
> Can we be sure that contents[r_offset - 1] == 0xe8 will be call?

Not if the PC32 reloc happens to be in data.  Also, I suspect some of
the possible load/store insns might have a sib byte of 0xE8.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: RFC: PATCH: PR x86_64/584: Detect call on protected symbol

[H. J. Lu]

On Thu, Jan 20, 2005 at 12:25:19PM +1030, Alan Modra wrote:
> On Wed, Jan 19, 2005 at 04:40:32PM -0800, H. J. Lu wrote:
> > On Thu, Jan 20, 2005 at 12:05:44AM +0100, Jakub Jelinek wrote:
> > > On Wed, Jan 19, 2005 at 02:48:34PM -0800, H. J. Lu wrote:
> > > > X86_64 uses R_X86_64_PC32 for both branch and store/load. Linker
> > > > can't tell if a protected symbol reference is local or global just
> > > > by relocation. This patch disassembles the code to check for call.
> > > 
> > > Eh, why you need the disassembler there?
> > > Isn't it enough just to check if R_X86_64_PC32's r_offset > 0 and
> > > contents[r_offset - 1] == 0xe8?
> > 
> > Can we be sure that contents[r_offset - 1] == 0xe8 will be call?
> 
> Not if the PC32 reloc happens to be in data.  Also, I suspect some of

We can check if the input section is executable. The worst case is that
you may get a runtime error instead of a linktime one.

> the possible load/store insns might have a sib byte of 0xE8.

I think 0xe8 SIB is [RAX + RIP * 8], which can't have R_X86_64_PC32.


H.J.

Re: RFC: PATCH: PR x86_64/584: Detect call on protected symbol

[Alan Modra]

On Wed, Jan 19, 2005 at 07:17:09PM -0800, H. J. Lu wrote:
> On Thu, Jan 20, 2005 at 12:25:19PM +1030, Alan Modra wrote:
> > On Wed, Jan 19, 2005 at 04:40:32PM -0800, H. J. Lu wrote:
> > > Can we be sure that contents[r_offset - 1] == 0xe8 will be call?
> > 
> > Not if the PC32 reloc happens to be in data.  Also, I suspect some of
> 
> We can check if the input section is executable. The worst case is that
> you may get a runtime error instead of a linktime one.
> 
> > the possible load/store insns might have a sib byte of 0xE8.
> 
> I think 0xe8 SIB is [RAX + RIP * 8], which can't have R_X86_64_PC32.

Ah, no disp field.  A modrm of 0xE8 won't have the PC32 reloc either.
So the only problem is when people put things like jump tables in .text.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: RFC: PATCH: PR x86_64/584: Detect call on protected symbol

[H. J. Lu]

On Thu, Jan 20, 2005 at 02:52:34PM +1030, Alan Modra wrote:
> On Wed, Jan 19, 2005 at 07:17:09PM -0800, H. J. Lu wrote:
> > On Thu, Jan 20, 2005 at 12:25:19PM +1030, Alan Modra wrote:
> > > On Wed, Jan 19, 2005 at 04:40:32PM -0800, H. J. Lu wrote:
> > > > Can we be sure that contents[r_offset - 1] == 0xe8 will be call?
> > > 
> > > Not if the PC32 reloc happens to be in data.  Also, I suspect some of
> > 
> > We can check if the input section is executable. The worst case is that
> > you may get a runtime error instead of a linktime one.
> > 
> > > the possible load/store insns might have a sib byte of 0xE8.
> > 
> > I think 0xe8 SIB is [RAX + RIP * 8], which can't have R_X86_64_PC32.
> 
> Ah, no disp field.  A modrm of 0xE8 won't have the PC32 reloc either.
> So the only problem is when people put things like jump tables in .text.

I don't think jump table uses R_X86_64_PC32. Does R_X86_64_PC32
only come from "branch label" or "insn label(%rip),xxx"? I don't
think we should worry about anything, like wrong insn, r_offset == 0,
jump table or data section.

H.J.

Re: RFC: PATCH: PR x86_64/584: Detect call on protected symbol

[Andreas Schwab]

"H. J. Lu" <hjl@lucon.org> writes:

> I don't think we should worry about anything, like wrong insn, r_offset
> == 0, jump table or data section.

At least we shouldn't crash.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: RFC: PATCH: PR x86_64/584: Detect call on protected symbol

[H. J. Lu]

On Thu, Jan 20, 2005 at 11:17:55AM +0100, Andreas Schwab wrote:
> "H. J. Lu" <hjl@lucon.org> writes:
> 
> > I don't think we should worry about anything, like wrong insn, r_offset
> > == 0, jump table or data section.
> 
> At least we shouldn't crash.

How about this patch?


H.J.
----
2005-01-20  H.J. Lu  <hongjiu.lu@intel.com>

	PR 584
	* elf64-x86-64.c (is_32bit_relative_branch): New.
	(elf64_x86_64_relocate_section): Alllow R_X86_64_PC32 on a
	protected function symbol when building shared library for
	32bit relative branch instruction.

--- bfd/elf64-x86-64.c.prot	2005-01-11 09:10:28.000000000 -0800
+++ bfd/elf64-x86-64.c	2005-01-20 09:31:25.100049886 -0800
@@ -1745,6 +1745,24 @@ tpoff (struct bfd_link_info *info, bfd_v
   return address - htab->tls_size - htab->tls_sec->vma;
 }
 
+/* Is the instruction before OFFSET in CONTENTS a 32bit relative
+   branch?  */
+
+static bfd_boolean
+is_32bit_relative_branch (bfd_byte *contents, bfd_vma offset)
+{
+  /* Opcode		Instruction
+     0xe8		call
+     0xe9		jump
+     0x0f 0x8x		conditional jump */
+  return ((offset > 0
+	   && (contents [offset - 1] == 0xe8
+	       || contents [offset - 1] == 0xe9))
+	  || (offset > 1
+	      && contents [offset - 2] == 0x0f
+	      && (contents [offset - 1] & 0xf0) == 0x80));
+}
+
 /* Relocate an x86_64 ELF section.  */
 
 static bfd_boolean
@@ -1950,7 +1968,12 @@ elf64_x86_64_relocate_section (bfd *outp
 	  if (info->shared
 	      && !SYMBOL_REFERENCES_LOCAL (info, h)
 	      && (input_section->flags & SEC_ALLOC) != 0
-	      && (input_section->flags & SEC_READONLY) != 0)
+	      && (input_section->flags & SEC_READONLY) != 0
+	      && (!h->def_regular
+		  || r_type != R_X86_64_PC32
+		  || h->type != STT_FUNC
+		  || ELF_ST_VISIBILITY (h->other) != STV_PROTECTED
+		  || !is_32bit_relative_branch (contents, rel->r_offset)))
 	    {
 	      (*_bfd_error_handler)
 		(_("%B: relocation %s against `%s' can not be used when making a shared object; recompile with -fPIC"),

PATCH: Properly handle protected function for ia32 and x86_64

[H. J. Lu]

On Thu, Jan 20, 2005 at 09:33:54AM -0800, H. J. Lu wrote:
> On Thu, Jan 20, 2005 at 11:17:55AM +0100, Andreas Schwab wrote:
> > "H. J. Lu" <hjl@lucon.org> writes:
> > 
> > > I don't think we should worry about anything, like wrong insn, r_offset
> > > == 0, jump table or data section.
> > 
> > At least we shouldn't crash.
> 
> How about this patch?
> 
> 

I updated the patch to make ia32 and x86_64 the same in dealing with
protected function symbols. On ia32, I disallow R_386_GOTOFF on
protected function and on x86_64, I allow 32bit relative branch on
protected function. Since we only warn global symbols, there is no
need to check if h is NULL. Also I updated error message.


H.J.
----
2005-01-24  H.J. Lu  <hongjiu.lu@intel.com>

	* elf32-i386.c (elf_i386_relocate_section): Disallow R_386_GOTOFF
	against protected function when building shared library.

	PR 584
	* elf64-x86-64.c (is_32bit_relative_branch): New.
	(elf64_x86_64_relocate_section): Alllow R_X86_64_PC32 on a
	protected function symbol when building shared library for
	32bit relative branch instruction.

--- bfd/elf32-i386.c.prot	2005-01-11 09:09:27.000000000 -0800
+++ bfd/elf32-i386.c	2005-01-24 12:52:26.233408808 -0800
@@ -2274,6 +2274,22 @@ elf_i386_relocate_section (bfd *output_b
 	  /* Relocation is relative to the start of the global offset
 	     table.  */
 
+	  /* Check to make sure it isn't a protected function symbol
+	     for shared library since it may not be local when used
+	     as function address.  */
+	  if (info->shared
+	      && h
+	      && h->def_regular
+	      && h->type == STT_FUNC
+	      && ELF_ST_VISIBILITY (h->other) == STV_PROTECTED)
+	    {
+	      (*_bfd_error_handler)
+		(_("%B: relocation R_386_GOTOFF against protected function `%s' can not be used when making a shared object"),
+		 input_bfd, h->root.root.string);
+	      bfd_set_error (bfd_error_bad_value);
+	      return FALSE;
+	    }
+
 	  /* Note that sgot is not involved in this
 	     calculation.  We always want the start of .got.plt.  If we
 	     defined _GLOBAL_OFFSET_TABLE_ in a different way, as is
--- bfd/elf64-x86-64.c.prot	2005-01-11 09:10:28.000000000 -0800
+++ bfd/elf64-x86-64.c	2005-01-24 13:08:28.622633929 -0800
@@ -1745,6 +1745,24 @@ tpoff (struct bfd_link_info *info, bfd_v
   return address - htab->tls_size - htab->tls_sec->vma;
 }
 
+/* Is the instruction before OFFSET in CONTENTS a 32bit relative
+   branch?  */
+
+static bfd_boolean
+is_32bit_relative_branch (bfd_byte *contents, bfd_vma offset)
+{
+  /* Opcode		Instruction
+     0xe8		call
+     0xe9		jump
+     0x0f 0x8x		conditional jump */
+  return ((offset > 0
+	   && (contents [offset - 1] == 0xe8
+	       || contents [offset - 1] == 0xe9))
+	  || (offset > 1
+	      && contents [offset - 2] == 0x0f
+	      && (contents [offset - 1] & 0xf0) == 0x80));
+}
+
 /* Relocate an x86_64 ELF section.  */
 
 static bfd_boolean
@@ -1950,13 +1968,26 @@ elf64_x86_64_relocate_section (bfd *outp
 	  if (info->shared
 	      && !SYMBOL_REFERENCES_LOCAL (info, h)
 	      && (input_section->flags & SEC_ALLOC) != 0
-	      && (input_section->flags & SEC_READONLY) != 0)
+	      && (input_section->flags & SEC_READONLY) != 0
+	      && (!h->def_regular
+		  || r_type != R_X86_64_PC32
+		  || h->type != STT_FUNC
+		  || ELF_ST_VISIBILITY (h->other) != STV_PROTECTED
+		  || !is_32bit_relative_branch (contents,
+						rel->r_offset)))
 	    {
-	      (*_bfd_error_handler)
-		(_("%B: relocation %s against `%s' can not be used when making a shared object; recompile with -fPIC"),
-		 input_bfd,
-		 x86_64_elf_howto_table[r_type].name,
-		 (h) ? h->root.root.string : "a local symbol");
+	      if (h->def_regular
+		  && r_type == R_X86_64_PC32
+		  && h->type == STT_FUNC
+		  && ELF_ST_VISIBILITY (h->other) == STV_PROTECTED)
+		(*_bfd_error_handler)
+		   (_("%B: relocation R_X86_64_PC32 against protected function `%s' can not be used when making a shared object"),
+		    input_bfd, h->root.root.string);
+	      else
+		(*_bfd_error_handler)
+		  (_("%B: relocation %s against `%s' can not be used when making a shared object; recompile with -fPIC"),
+		   input_bfd, x86_64_elf_howto_table[r_type].name,
+		   h->root.root.string);
 	      bfd_set_error (bfd_error_bad_value);
 	      return FALSE;
 	    }

Re: PATCH: Properly handle protected function for ia32 and x86_64

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]

"H. J. Lu" <hjl@lucon.org> writes:

> On Thu, Jan 20, 2005 at 09:33:54AM -0800, H. J. Lu wrote:
>> On Thu, Jan 20, 2005 at 11:17:55AM +0100, Andreas Schwab wrote:
>> > "H. J. Lu" <hjl@lucon.org> writes:
>> > 
>> > > I don't think we should worry about anything, like wrong insn, r_offset
>> > > == 0, jump table or data section.
>> > 
>> > At least we shouldn't crash.
>> 
>> How about this patch?
>> 
>> 
>
> I updated the patch to make ia32 and x86_64 the same in dealing with
> protected function symbols. On ia32, I disallow R_386_GOTOFF on
> protected function and on x86_64, I allow 32bit relative branch on
> protected function. Since we only warn global symbols, there is no
> need to check if h is NULL. Also I updated error message.
>
>
> H.J.
> ----
> 2005-01-24  H.J. Lu  <hongjiu.lu@intel.com>
>
> 	* elf32-i386.c (elf_i386_relocate_section): Disallow R_386_GOTOFF
> 	against protected function when building shared library.
>
> 	PR 584
> 	* elf64-x86-64.c (is_32bit_relative_branch): New.
> 	(elf64_x86_64_relocate_section): Alllow R_X86_64_PC32 on a
> 	protected function symbol when building shared library for
> 	32bit relative branch instruction.

I guess this bug is the reason for this one:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19664

I approve the x86-64 part of the patch but would suggest that you wait
another day for comments before checking in,

thanks,
Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: PATCH: Properly handle protected function for ia32 and x86_64

[Alan Modra]

On Sun, Jan 30, 2005 at 11:22:29AM +0100, Andreas Jaeger wrote:
> "H. J. Lu" <hjl@lucon.org> writes:
> > 	* elf32-i386.c (elf_i386_relocate_section): Disallow R_386_GOTOFF
> > 	against protected function when building shared library.
> >
> > 	PR 584
> > 	* elf64-x86-64.c (is_32bit_relative_branch): New.
> > 	(elf64_x86_64_relocate_section): Alllow R_X86_64_PC32 on a
> > 	protected function symbol when building shared library for
> > 	32bit relative branch instruction.
> 
> I approve the x86-64 part of the patch but would suggest that you wait
> another day for comments before checking in,

I'm not happy with the i386 one, because conceptually there isn't any
reason why the GOT of a shared library can't contain an entry for a
protected symbol.  I believe such a shared lib will work properly, so it
isn't appropriate to issue an error.  The problem occurs when an
executable tries to reference such a symbol, and copy relocs are
involved.

I think you should *warn* that a shared library built with protected
data symbols may result in non-shared text pages for apps linked against
the lib, and force elimination of the copy reloc when building apps
against such libs.  Another issue is whether seeing a GOTOFF reloc
against a protected symbol is a sufficient indicator of a problematic
shared lib.  ie. Perhaps there are other relocs used when different code
sequences in a shared lib access the lib's protected data.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: Properly handle protected function for ia32 and x86_64

[H. J. Lu]

On Tue, Feb 01, 2005 at 03:21:10PM +1030, Alan Modra wrote:
> On Sun, Jan 30, 2005 at 11:22:29AM +0100, Andreas Jaeger wrote:
> > "H. J. Lu" <hjl@lucon.org> writes:
> > > 	* elf32-i386.c (elf_i386_relocate_section): Disallow R_386_GOTOFF
> > > 	against protected function when building shared library.
> > >
> > > 	PR 584
> > > 	* elf64-x86-64.c (is_32bit_relative_branch): New.
> > > 	(elf64_x86_64_relocate_section): Alllow R_X86_64_PC32 on a
> > > 	protected function symbol when building shared library for
> > > 	32bit relative branch instruction.
> > 
> > I approve the x86-64 part of the patch but would suggest that you wait
> > another day for comments before checking in,
> 
> I'm not happy with the i386 one, because conceptually there isn't any
> reason why the GOT of a shared library can't contain an entry for a
> protected symbol.  I believe such a shared lib will work properly, so it
> isn't appropriate to issue an error.  The problem occurs when an
> executable tries to reference such a symbol, and copy relocs are
> involved.

Please check it again. It is R_386_GOTOFF against protected FUNCTION
symbol. It has nothing to do with copy relocation. It is the function
pointer problem with protected function.

> 
> I think you should *warn* that a shared library built with protected
> data symbols may result in non-shared text pages for apps linked against
> the lib, and force elimination of the copy reloc when building apps
> against such libs.  Another issue is whether seeing a GOTOFF reloc
> against a protected symbol is a sufficient indicator of a problematic
> shared lib.  ie. Perhaps there are other relocs used when different code
> sequences in a shared lib access the lib's protected data.
> 

What you described is my proposal:

http://sources.redhat.com/ml/binutils/2005-01/msg00349.html

I will implement it when I find time.


H.J.

Re: PATCH: Properly handle protected function for ia32 and x86_64

[Alan Modra]

On Mon, Jan 31, 2005 at 09:50:18PM -0800, H. J. Lu wrote:
> On Tue, Feb 01, 2005 at 03:21:10PM +1030, Alan Modra wrote:
> > I'm not happy with the i386 one, because conceptually there isn't any
> > reason why the GOT of a shared library can't contain an entry for a
> > protected symbol.  I believe such a shared lib will work properly, so it
> > isn't appropriate to issue an error.  The problem occurs when an
> > executable tries to reference such a symbol, and copy relocs are
> > involved.
> 
> Please check it again. It is R_386_GOTOFF against protected FUNCTION
> symbol. It has nothing to do with copy relocation. It is the function
> pointer problem with protected function.

OK, I misunderstood the problem.  Do you have a testcase?

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: Properly handle protected function for ia32 and x86_64

[H. J. Lu]

On Tue, Feb 01, 2005 at 06:14:21PM +1030, Alan Modra wrote:
> On Mon, Jan 31, 2005 at 09:50:18PM -0800, H. J. Lu wrote:
> > On Tue, Feb 01, 2005 at 03:21:10PM +1030, Alan Modra wrote:
> > > I'm not happy with the i386 one, because conceptually there isn't any
> > > reason why the GOT of a shared library can't contain an entry for a
> > > protected symbol.  I believe such a shared lib will work properly, so it
> > > isn't appropriate to issue an error.  The problem occurs when an
> > > executable tries to reference such a symbol, and copy relocs are
> > > involved.
> > 
> > Please check it again. It is R_386_GOTOFF against protected FUNCTION
> > symbol. It has nothing to do with copy relocation. It is the function
> > pointer problem with protected function.
> 
> OK, I misunderstood the problem.  Do you have a testcase?
> 

There is a testcase in

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19520



H.J.