RFC: Can static executables contain relocations against symbols ?

  Hi Guys,

  Can static executables contain relocations against symbols ?

  This question has come up in Fedora as part of the investigation into
  a problem linking some binaries compiled with the Rust compiler:

https://bugzilla.redhat.com/show_bug.cgi?id=2166149

  The issue appears to be that static-pie executables are being created
  with a .rela.got section that has an sh_link field set to point to the
  .symtab section.  This in turns means that running strip on the
  executables will not remove the .symtab section, which is then being
  flagged as an error by the build system.

  The problem is not happening for x86_64 binaries because they contain
  a .dynsym section, and the code in bfd/elf.c:assign_section_numbers
  will preferentially use that section if it exists.  (It is not clear
  to me *why* x86_64 static pie binaries contain a .dynsym section, but
  they do).

  It is possible for static executables to contain relocations - for
  ifuncs for example - but I think that they should always be absolute.
  So my question is: is it safe to set the sh_link field for relocation
  sections in static executables to 0 ?

  The attached patch is a suggestion for the change I am considering...

  Thoughts ?
  
Cheers
  Nick
  

Hi Nick,

On Wed, 2023-03-29 at 15:39 +0100, Nick Clifton wrote:
>   Can static executables contain relocations against symbols ?

I see nothing in the standard that says they cannot, but also nothing
that says that if there are no relocations against symbols (which is
the case in the issue below) that a relocation section is required to
point to a symbol table.

>   This question has come up in Fedora as part of the investigation into
>   a problem linking some binaries compiled with the Rust compiler:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=2166149
> 
>   The issue appears to be that static-pie executables are being created
>   with a .rela.got section that has an sh_link field set to point to the
>   .symtab section.  This in turns means that running strip on the
>   executables will not remove the .symtab section, which is then being
>   flagged as an error by the build system.
> 
>   The problem is not happening for x86_64 binaries because they contain
>   a .dynsym section, and the code in bfd/elf.c:assign_section_numbers
>   will preferentially use that section if it exists.  (It is not clear
>   to me *why* x86_64 static pie binaries contain a .dynsym section, but
>   they do).

Note that the x86_64 .dynsym table actually is empty (contains a single
null symbol). I think it makes sense that if you refer from an
allocated relocation section to reference an allocated .dynsym section
(even if it is empty) instead of referencing an unallocated .symtab
section (because that won't be available at runtime).

>   It is possible for static executables to contain relocations - for
>   ifuncs for example - but I think that they should always be absolute.
>   So my question is: is it safe to set the sh_link field for relocation
>   sections in static executables to 0 ?

I think it is safe if you make sure there are no symbol references.
Otherwise it seems it must point to an allocated .dynsym section and
not an unallocated .symtab section.

Cheers,

Mark
  

Hello,

On Wed, 29 Mar 2023, Nick Clifton via Binutils wrote:

>   Can static executables contain relocations against symbols ?

Generally I agree with you that they should not.  Only symbol-less 
relocations make sense (not all of them will be absolute, though.  In PIEs 
they will be base-relative, still symbolless of course).  But for the sake 
of discussion let's assume we somehow want to allow symbol-based relocs 
nevertheless.  Then:

In final-linked ELF files symbol headers always have to be optional.  The 
place of relocations, if necessary, (and symbols) needs to be communicated 
in a different way anyway (on dynamic ELF files via the program headers 
and dynamic entries, on static files via some other side channel, like 
hardcoding the offset/addresses of the interesting pieces).

But: there is another observation that makes the Rust output invalid.  An 
allocated (SHF_ALLOC) section (here .got.rela) _must_ only refer to 
allocated sections via its sh_link field.  The .symtab section in the 
example at hand was not SHF_ALLOC.  So that's another bug.

Leaving a sh_link field of an SHT_RELA section to be zero seems a bit 
dubious.  Normally client code would be able to rely on sh_link containing 
an existing section index.  OTOH SHN_UNDEF (i.e. zero) _is_ somewhat of a 
section index, so leaving it as zero might just be fine.

So, even if the above problems would be fixed I think your patch still 
makes sense.  Perhaps add another check to only leave it zero if 
elf_onesymtab is not allocated itself.


Ciao,
Michael.
  

On Wed, Mar 29, 2023 at 03:39:46PM +0100, Nick Clifton via Binutils wrote:
>   The issue appears to be that static-pie executables are being created
>   with a .rela.got section that has an sh_link field set to point to the
>   .symtab section.

Nick, the patch you posted is for code with this comment:
	case SHT_REL:
	case SHT_RELA:
	  /* A reloc section which we are treating as a normal BFD
	     section.  sh_link is the section index of the symbol
	     table.  sh_info is the section index of the section to
	     which the relocation entries apply.  We assume that an
	     allocated reloc section uses the dynamic symbol table
	     if there is one.  Otherwise we guess the normal symbol
	     table.  FIXME: How can we be sure?  */

ie. we are way off into the weeds already.  However, I think the
comment is wrongly placed, because it looks to me like we handle the
usual SHT_REL/SHT_RELA sections here too.

So is the .rela.got (or .rela.plt) section one of the reloc sections
created in bfd_section_from_shdr elf.c:2382?  If we need to handle
this sort of reloc section better (and I doubt we do need to) then we
should do something about tracking the original sh_link and sh_info
fields when creating them,  Hmm, maybe they are still available, and
can be mapped to output sections?

Anyway, does the following fix the particular problem you are seeing?

diff --git a/bfd/elf.c b/bfd/elf.c
index 45e53640e8f..027d0143735 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -3870,21 +3870,23 @@ assign_section_numbers (bfd *abfd, struct bfd_link_info *link_info)
 	{
 	case SHT_REL:
 	case SHT_RELA:
-	  /* A reloc section which we are treating as a normal BFD
-	     section.  sh_link is the section index of the symbol
-	     table.  sh_info is the section index of the section to
-	     which the relocation entries apply.  We assume that an
-	     allocated reloc section uses the dynamic symbol table
-	     if there is one.  Otherwise we guess the normal symbol
-	     table.  FIXME: How can we be sure?  */
-	  if (d->this_hdr.sh_link == 0 && (sec->flags & SEC_ALLOC) != 0)
-	    {
-	      s = bfd_get_section_by_name (abfd, ".dynsym");
-	      if (s != NULL)
-		d->this_hdr.sh_link = elf_section_data (s)->this_idx;
-	    }
+	  /* sh_link is the section index of the symbol table.
+	     sh_info is the section index of the section to which the
+	     relocation entries apply.  */
 	  if (d->this_hdr.sh_link == 0)
-	    d->this_hdr.sh_link = elf_onesymtab (abfd);
+	    {
+	      /* FIXME maybe: If this is a reloc section which we are
+		 treating as a normal section then we likely should
+		 not be assuming its sh_link is .dynsym or .symtab.  */
+	      if ((sec->flags & SEC_ALLOC) != 0)
+		{
+		  s = bfd_get_section_by_name (abfd, ".dynsym");
+		  if (s != NULL)
+		    d->this_hdr.sh_link = elf_section_data (s)->this_idx;
+		}
+	      else
+		d->this_hdr.sh_link = elf_onesymtab (abfd);
+	    }
 
 	  s = elf_get_reloc_section (sec);
 	  if (s != NULL)
  

On Thu, Mar 30, 2023 at 11:52:01AM +1030, Alan Modra wrote:
> Anyway, does the following fix the particular problem you are seeing?

I'm going to commit the patch even if it doesn't fix your problem
since that 1995 comment from commit ede4eed48386 needs fixing, and
it's wrong to use a non-alloc symtab with an alloc reloc section as
others have commented.
  

Hi Alan,

>> Anyway, does the following fix the particular problem you are seeing?

I am not sure - I will have to investigate.  Reproducing the original
problem is difficult because it involves using the Rust compiler on
a non-x86_64 platform, so it may take me a while.

> I'm going to commit the patch even if it doesn't fix your problem
> since that 1995 comment from commit ede4eed48386 needs fixing, and
> it's wrong to use a non-alloc symtab with an alloc reloc section as
> others have commented.

That is a good point.  Thanks for creating a better patch.

Cheers
   Nick
  

Hi Nick,
On Thu, Mar 30, 2023 at 09:57:42AM +0100, Nick Clifton wrote:
> That is a good point.  Thanks for creating a better patch.

Eh, well, it was likeley bad code of mine from commit b6d1f70cc7e9
that caused the problem in the first place.