public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Fix two bugs breaking IFUNC in static PIE
@ 2022-09-20  6:09 Xi Ruoyao
  2022-09-20  6:09 ` [PATCH v2 1/2] LoongArch: Don't write into GOT for local ifunc Xi Ruoyao
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Xi Ruoyao @ 2022-09-20  6:09 UTC (permalink / raw)
  To: binutils; +Cc: liuzhensong, Lulu Cheng, Wang Xuerui, Chenghua Xu, Xi Ruoyao

To generate a static PIE, the compiler should pass "-static -pie
--no-dynamic-linker -z text" options to the linker [1].  But, LoongArch
BFD linker currently produces wrong result (missing R_LARCH_IRELATIVE
relocs) with IFUNC and those options, causing test failures in Glibc if
static PIE is enabled.  Fix them now.

For the detailed analysis of the bugs see the commit messages.

Changes v1 -> v2:

- Stop writing into GOT for local ifunc.
- Add a test case for R_LARCH_IRELATIVE insertion.

[1]: https://gcc.gnu.org/r13-2728

Xi Ruoyao (2):
  LoongArch: Don't write into GOT for local ifunc
  LoongArch: Fix R_LARCH_IRELATIVE insertion after elf_link_sort_relocs

 bfd/elfnn-loongarch.c                         | 46 +++++++++++--------
 .../ld-loongarch-elf/ld-loongarch-elf.exp     |  1 +
 .../ld-loongarch-elf/local-ifunc-reloc.d      | 12 +++++
 .../ld-loongarch-elf/local-ifunc-reloc.s      | 28 +++++++++++
 4 files changed, 68 insertions(+), 19 deletions(-)
 create mode 100644 ld/testsuite/ld-loongarch-elf/local-ifunc-reloc.d
 create mode 100644 ld/testsuite/ld-loongarch-elf/local-ifunc-reloc.s

-- 
2.37.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/2] LoongArch: Don't write into GOT for local ifunc
  2022-09-20  6:09 [PATCH v2 0/2] Fix two bugs breaking IFUNC in static PIE Xi Ruoyao
@ 2022-09-20  6:09 ` Xi Ruoyao
  2022-09-20  6:09 ` [PATCH v2 2/2] LoongArch: Fix R_LARCH_IRELATIVE insertion after elf_link_sort_relocs Xi Ruoyao
  2022-09-20  9:29 ` [PATCH v2 0/2] Fix two bugs breaking IFUNC in static PIE liuzhensong
  2 siblings, 0 replies; 4+ messages in thread
From: Xi Ruoyao @ 2022-09-20  6:09 UTC (permalink / raw)
  To: binutils; +Cc: liuzhensong, Lulu Cheng, Wang Xuerui, Chenghua Xu, Xi Ruoyao

Local ifuncs are always resolved at runtime via R_LARCH_IRELATIVE, so
there is no need to write anything into GOT.  And when we write the GOT
we actually trigger a heap-buffer-overflow: If a and b are different
sections, we cannot access something in b with "a->contents + (offset
from a)" because "a->contents" and "b->contents" are heap buffers
allocated separately, not slices of a large buffer.

So stop writing into GOT for local ifunc now.
---
 bfd/elfnn-loongarch.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index ed42b8b6770..af18a8a0168 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -3179,6 +3179,8 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
 						     htab->elf.srelgot, &rela);
 			}
 		      h->got.offset |= 1;
+		      bfd_put_NN (output_bfd, relocation,
+				  got->contents + got_off);
 		    }
 		}
 	      else
@@ -3200,10 +3202,9 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
 			}
 		      local_got_offsets[r_symndx] |= 1;
 		    }
+		  bfd_put_NN (output_bfd, relocation, got->contents + got_off);
 		}
 
-	      bfd_put_NN (output_bfd, relocation, got->contents + got_off);
-
 	      relocation = got_off + sec_addr (got);
 	    }
 
-- 
2.37.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] LoongArch: Fix R_LARCH_IRELATIVE insertion after elf_link_sort_relocs
  2022-09-20  6:09 [PATCH v2 0/2] Fix two bugs breaking IFUNC in static PIE Xi Ruoyao
  2022-09-20  6:09 ` [PATCH v2 1/2] LoongArch: Don't write into GOT for local ifunc Xi Ruoyao
@ 2022-09-20  6:09 ` Xi Ruoyao
  2022-09-20  9:29 ` [PATCH v2 0/2] Fix two bugs breaking IFUNC in static PIE liuzhensong
  2 siblings, 0 replies; 4+ messages in thread
From: Xi Ruoyao @ 2022-09-20  6:09 UTC (permalink / raw)
  To: binutils; +Cc: liuzhensong, Lulu Cheng, Wang Xuerui, Chenghua Xu, Xi Ruoyao

loongarch_elf_finish_dynamic_symbol is called after elf_link_sort_relocs
if -z combreloc.  elf_link_sort_relocs redistributes the contents of
.rela.* sections those would be merged into .rela.dyn, so the slot for
R_LARCH_IRELATIVE may be out of relplt->contents now.

To make things worse, the boundary check

    dyn < dyn + relplt->size / sizeof (*dyn)

is obviously wrong ("x + 10 < x"? :), causing the issue undetected
during the linking process and the resulted executable suddenly crashes
at runtime.

The issue was found during an attempt to add static-pie support to the
toolchain.

Fix it by iterating through the inputs of .rela.dyn to find the slot.
---
 bfd/elfnn-loongarch.c                         | 41 +++++++++++--------
 .../ld-loongarch-elf/ld-loongarch-elf.exp     |  1 +
 .../ld-loongarch-elf/local-ifunc-reloc.d      | 12 ++++++
 .../ld-loongarch-elf/local-ifunc-reloc.s      | 28 +++++++++++++
 4 files changed, 65 insertions(+), 17 deletions(-)
 create mode 100644 ld/testsuite/ld-loongarch-elf/local-ifunc-reloc.d
 create mode 100644 ld/testsuite/ld-loongarch-elf/local-ifunc-reloc.s

diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index af18a8a0168..33c85e5207c 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -3514,6 +3514,12 @@ loongarch_elf_finish_dynamic_symbol (bfd *output_bfd,
 {
   struct loongarch_elf_link_hash_table *htab = loongarch_elf_hash_table (info);
   const struct elf_backend_data *bed = get_elf_backend_data (output_bfd);
+  asection *rela_dyn = bfd_get_section_by_name (output_bfd, ".rela.dyn");
+  struct bfd_link_order *lo = NULL;
+  Elf_Internal_Rela *slot = NULL, *last_slot = NULL;
+
+  if (rela_dyn)
+    lo = rela_dyn->map_head.link_order;
 
   if (h->plt.offset != MINUS_ONE)
     {
@@ -3523,6 +3529,7 @@ loongarch_elf_finish_dynamic_symbol (bfd *output_bfd,
       uint32_t plt_entry[PLT_ENTRY_INSNS];
       bfd_byte *loc;
       Elf_Internal_Rela rela;
+      asection *rela_sec = NULL;
 
       if (htab->elf.splt)
 	{
@@ -3575,31 +3582,31 @@ loongarch_elf_finish_dynamic_symbol (bfd *output_bfd,
 	  && (relplt == htab->elf.srelgot
 	      || relplt == htab->elf.irelplt))
 	{
-	    {
-	      rela.r_info = ELFNN_R_INFO (0, R_LARCH_IRELATIVE);
-	      rela.r_addend = (h->root.u.def.value
+	  rela.r_info = ELFNN_R_INFO (0, R_LARCH_IRELATIVE);
+	  rela.r_addend = (h->root.u.def.value
 			       + h->root.u.def.section->output_section->vma
 			       + h->root.u.def.section->output_offset);
-	    }
 
-	    /* Find the space after dyn sort.  */
+	  /* Find the space after dyn sort.  */
+	  while (slot == last_slot || slot->r_offset != 0)
 	    {
-	      Elf_Internal_Rela *dyn = (Elf_Internal_Rela *)relplt->contents;
-	      bool fill = false;
-	      for (;dyn < dyn + relplt->size / sizeof (*dyn); dyn++)
+	      if (slot != last_slot)
 		{
-		  if (0 == dyn->r_offset)
-		    {
-		      bed->s->swap_reloca_out (output_bfd, &rela,
-					       (bfd_byte *)dyn);
-		      relplt->reloc_count++;
-		      fill = true;
-		      break;
-		    }
+		  slot++;
+		  continue;
 		}
-	      BFD_ASSERT (fill);
+
+	      BFD_ASSERT (lo != NULL);
+	      rela_sec = lo->u.indirect.section;
+	      lo = lo->next;
+
+	      slot = (Elf_Internal_Rela *)rela_sec->contents;
+	      last_slot = (Elf_Internal_Rela *)(rela_sec->contents +
+						rela_sec->size);
 	    }
 
+	  bed->s->swap_reloca_out (output_bfd, &rela, (bfd_byte *)slot);
+	  rela_sec->reloc_count++;
 	}
       else
 	{
diff --git a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
index dfa8ee18865..726ee823701 100644
--- a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
+++ b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
@@ -31,6 +31,7 @@ if [istarget "loongarch64-*-*"] {
     run_dump_test "macro_op"
     run_dump_test "syscall"
     run_dump_test "disas-jirl"
+    run_dump_test "local-ifunc-reloc"
 }
 
 if [istarget "loongarch32-*-*"] {
diff --git a/ld/testsuite/ld-loongarch-elf/local-ifunc-reloc.d b/ld/testsuite/ld-loongarch-elf/local-ifunc-reloc.d
new file mode 100644
index 00000000000..d7cadb8c527
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/local-ifunc-reloc.d
@@ -0,0 +1,12 @@
+#as:
+#ld: -shared -z combreloc
+#objdump: -R
+
+.*: +file format .*
+
+DYNAMIC RELOCATION RECORDS
+OFFSET +TYPE +VALUE
+[[:xdigit:]]+ R_LARCH_IRELATIVE +\*ABS\*\+0x[[:xdigit:]]+
+[[:xdigit:]]+ R_LARCH_64 +test
+
+
diff --git a/ld/testsuite/ld-loongarch-elf/local-ifunc-reloc.s b/ld/testsuite/ld-loongarch-elf/local-ifunc-reloc.s
new file mode 100644
index 00000000000..77c487463f5
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/local-ifunc-reloc.s
@@ -0,0 +1,28 @@
+.text
+.align 2
+
+.local ifunc
+.type ifunc, @gnu_indirect_function
+.set ifunc, resolver
+
+resolver:
+  la.local $a0, impl
+  jr $ra
+
+impl:
+  li.w $a0, 42
+  jr $ra
+
+.global test
+.type test, @function
+test:
+  move $s0, $ra
+  bl ifunc
+  xori $a0, $a0, 42
+  jr $s0
+
+.data
+.global ptr
+.type ptr, @object
+ptr:
+  .dword test
-- 
2.37.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 0/2] Fix two bugs breaking IFUNC in static PIE
  2022-09-20  6:09 [PATCH v2 0/2] Fix two bugs breaking IFUNC in static PIE Xi Ruoyao
  2022-09-20  6:09 ` [PATCH v2 1/2] LoongArch: Don't write into GOT for local ifunc Xi Ruoyao
  2022-09-20  6:09 ` [PATCH v2 2/2] LoongArch: Fix R_LARCH_IRELATIVE insertion after elf_link_sort_relocs Xi Ruoyao
@ 2022-09-20  9:29 ` liuzhensong
  2 siblings, 0 replies; 4+ messages in thread
From: liuzhensong @ 2022-09-20  9:29 UTC (permalink / raw)
  To: Xi Ruoyao, binutils; +Cc: Lulu Cheng, Wang Xuerui, Chenghua Xu


在 2022/9/20 下午2:09, Xi Ruoyao 写道:
> To generate a static PIE, the compiler should pass "-static -pie
> --no-dynamic-linker -z text" options to the linker [1].  But, LoongArch
> BFD linker currently produces wrong result (missing R_LARCH_IRELATIVE
> relocs) with IFUNC and those options, causing test failures in Glibc if
> static PIE is enabled.  Fix them now.
>
> For the detailed analysis of the bugs see the commit messages.
>
> Changes v1 -> v2:
>
> - Stop writing into GOT for local ifunc.
> - Add a test case for R_LARCH_IRELATIVE insertion.
>
> [1]: https://gcc.gnu.org/r13-2728
>
> Xi Ruoyao (2):
>    LoongArch: Don't write into GOT for local ifunc
>    LoongArch: Fix R_LARCH_IRELATIVE insertion after elf_link_sort_relocs
>
>   bfd/elfnn-loongarch.c                         | 46 +++++++++++--------
>   .../ld-loongarch-elf/ld-loongarch-elf.exp     |  1 +
>   .../ld-loongarch-elf/local-ifunc-reloc.d      | 12 +++++
>   .../ld-loongarch-elf/local-ifunc-reloc.s      | 28 +++++++++++
>   4 files changed, 68 insertions(+), 19 deletions(-)
>   create mode 100644 ld/testsuite/ld-loongarch-elf/local-ifunc-reloc.d
>   create mode 100644 ld/testsuite/ld-loongarch-elf/local-ifunc-reloc.s
>
Done.

https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=6224a6c2ead26a04f0b2b8ccf4ff5b817afbb425

https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=ae2e4d4035f511543d12f74b3b7fdb1ba0daab16


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-09-20  9:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-20  6:09 [PATCH v2 0/2] Fix two bugs breaking IFUNC in static PIE Xi Ruoyao
2022-09-20  6:09 ` [PATCH v2 1/2] LoongArch: Don't write into GOT for local ifunc Xi Ruoyao
2022-09-20  6:09 ` [PATCH v2 2/2] LoongArch: Fix R_LARCH_IRELATIVE insertion after elf_link_sort_relocs Xi Ruoyao
2022-09-20  9:29 ` [PATCH v2 0/2] Fix two bugs breaking IFUNC in static PIE liuzhensong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).