linux-kernel - Re: Avoiding unnecessary jump relocations in gas?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <554B6E0102000078000778D0@mail.emea.novell.com>
Date:	Thu, 07 May 2015 12:52:01 +0100
From:	"Jan Beulich" <JBeulich@...e.com>
To:	"Andy Lutomirski" <luto@...capital.net>
Cc:	<binutils@...rceware.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: Avoiding unnecessary jump relocations in gas?

>>> On 07.05.15 at 08:02, <luto@...capital.net> wrote:
> AFAICT gas will produce relocations for jumps to global labels in the
> same file.  This doesn't seem directly harmful to me, except that, on
> x86, it forces five-byte jumps instead of two-byte jumps.
> 
> This seems especially unfortunate, since even hidden and protected
> symbols have this problem.
> 
> Given that many users don't want interposition support (especially the
> kernel and anyone using .hidden or .protected), it would be nice to
> have a command-line option to turn this off and probably also to turn
> it off by default for hidden and protected symbols.  Can gas do this?

I've been running with the below changes (taken off of a bigger set
of changes, so the line numbers may look a little odd) for the last
couple of years. I never tried to submit this change because so far
I couldn't find the time to check whether this would have any
unwanted side effects on cases I don't normally use.

--- binutils-2.25/gas/config/tc-i386.c
+++ 2.25/gas/config/tc-i386.c
@@ -8872,6 +8899,22 @@ i386_frag_max_var (fragS *frag)
 int
 md_estimate_size_before_relax (fragS *fragP, segT segment)
 {
+#if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
+  asymbol *bfdsym = NULL;
+  elf_symbol_type *elfsym = NULL;
+
+  if (IS_ELF)
+    {
+      bfdsym = symbol_get_bfdsym (fragP->fr_symbol);
+
+      if (S_IS_EXTERNAL (fragP->fr_symbol))
+	{
+	  elfsym = elf_symbol_from (bfd_asymbol_bfd (bfdsym), bfdsym);
+	  gas_assert (elfsym);
+	}
+    }
+#endif
+
   /* We've already got fragP->fr_subtype right;  all we have to do is
      check for un-relaxable symbols.  On an ELF system, we can't relax
      an externally visible symbol, because it may be overridden by a
@@ -8879,10 +8922,12 @@ md_estimate_size_before_relax (fragS *fr
   if (S_GET_SEGMENT (fragP->fr_symbol) != segment
 #if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
       || (IS_ELF
-	  && (S_IS_EXTERNAL (fragP->fr_symbol)
+	  && ((S_IS_EXTERNAL (fragP->fr_symbol)
+	       && elfsym->internal_elf_sym.st_other != STV_PROTECTED
+	       && elfsym->internal_elf_sym.st_other != STV_HIDDEN
+	       && elfsym->internal_elf_sym.st_other != STV_INTERNAL)
 	      || S_IS_WEAK (fragP->fr_symbol)
-	      || ((symbol_get_bfdsym (fragP->fr_symbol)->flags
-		   & BSF_GNU_INDIRECT_FUNCTION))))
+	      || (bfdsym->flags & BSF_GNU_INDIRECT_FUNCTION)))
 #endif
 #if defined (OBJ_COFF) && defined (TE_PE)
       || (OUTPUT_FLAVOR == bfd_target_coff_flavour

But then I also have this (intentionally compiled out)

--- binutils-2.25/gas/symbols.c
+++ 2.25/gas/symbols.c
@@ -2075,13 +2075,43 @@ S_IS_DEFINED (symbolS *s)
 int
 S_FORCE_RELOC (symbolS *s, int strict)
 {
+#if 0/*defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)*/
+  While generally correct, this and the similarly framed code fragments
+  below cause, if enabled, relocations to be dropped that should not be (i.e.
+  BFD_RELOC_X86_64_GOTPCREL) because the side effects of using a certain
+  relocation type are not accounted for in generic_force_reloc (in write.c)
+  except for the vtable relocations. GOT references, at least, would have
+  to fall into this category too, but there does not seem to be a flag in
+  struct fix to indicate such side effects (maybe fx_plt really is, but then
+  it is grossly misnamed), nor does it seem reasonable to enumerate all such
+  relocations in generic_force_reloc.
+  elf_symbol_type *elfsym = NULL;
+#endif
+
   if (LOCAL_SYMBOL_CHECK (s))
     return ((struct local_symbol *) s)->lsy_section == undefined_section;
 
+#if 0/*defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)*/
+  if (IS_ELF && (s->bsym->flags & BSF_GLOBAL))
+    {
+      asymbol *bfdsym = symbol_get_bfdsym (s);
+
+      elfsym = elf_symbol_from (bfd_asymbol_bfd (bfdsym), bfdsym);
+      gas_assert (elfsym);
+    }
+#endif
+
   return ((strict
 	   && ((s->bsym->flags & BSF_WEAK) != 0
 	       || (EXTERN_FORCE_RELOC
-		   && (s->bsym->flags & BSF_GLOBAL) != 0)))
+		   && (s->bsym->flags & BSF_GLOBAL) != 0
+#if 0/*defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)*/
+		   && (elfsym == NULL
+		       || (elfsym->internal_elf_sym.st_other != STV_PROTECTED
+		           && elfsym->internal_elf_sym.st_other != STV_HIDDEN
+			   && elfsym->internal_elf_sym.st_other != STV_INTERNAL))
+#endif
+		   )))
 	  || (s->bsym->flags & BSF_GNU_INDIRECT_FUNCTION) != 0
 	  || s->bsym->section == undefined_section
 	  || bfd_is_com_section (s->bsym->section));

Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/