[committed] arc: Fix potential invalid pointer access when fixing got symbols.

Message ID 20210914091251.55431-1-claziss@synopsys.com
State New
Headers show
Series
  • [committed] arc: Fix potential invalid pointer access when fixing got symbols.
Related show

Commit Message

Nick Clifton via Binutils Sept. 14, 2021, 9:12 a.m.
When statically linking, it can arrive to an undefined weak symbol of
which its value cannot be determined. However, we are having pieces of
code which doesn't take this situation into account, leading to access
a structure which may not be initialized. Fix this situation and add a
test.

bfd/
xxxx-xx-xx  Cupertino Miranda  <cmiranda@synopsys.com>
            Claudiu Zissulescu  <claziss@synopsys.com>

	* arc-got.h (arc_static_sym_data): New structure.
	(get_static_sym_data): New function.
	(relocate_fix_got_relocs_for_got_info): Move the computation fo
	symbol value and section to above introduced function, and use
	this new function.

ld/testsuite/
xxxx-xx-xx  Claudiu Zissulescu  <claziss@synopsys.com>

	* ld-arc/got-weak.d: New file.
	* ld-arc/got-weak.s: Likewise.

Signed-off-by: Claudiu Zissulescu <claziss@synopsys.com>


fix
---
 bfd/ChangeLog                  |  9 ++++
 bfd/arc-got.h                  | 94 +++++++++++++++++++++-------------
 ld/ChangeLog                   |  5 ++
 ld/testsuite/ld-arc/got-weak.d | 12 +++++
 ld/testsuite/ld-arc/got-weak.s |  7 +++
 5 files changed, 90 insertions(+), 37 deletions(-)
 create mode 100644 ld/testsuite/ld-arc/got-weak.d
 create mode 100644 ld/testsuite/ld-arc/got-weak.s

-- 
2.31.1

Comments

Nick Clifton via Binutils Sept. 15, 2021, 1:59 a.m. | #1
> 	* ld-arc/got-weak.d: New file.

> 	* ld-arc/got-weak.s: Likewise.


Fails on arc-linux-uclibc

regexp_diff match failure
regexp "^00000100 <.*>:$"
line   "00010074 <__start>:"
regexp_diff match failure
regexp "^ 100:	2730 7f80 0000 2014 	ld	r0,\[pcl,0x2014\].*$"
line   "   10074:	2730 7f80 0000 2008 	ld	r0,[pcl,0x2008]	;1207c <.got>"
FAIL: ld-arc/got-weak

-- 
Alan Modra
Australia Development Lab, IBM
Nick Clifton via Binutils Sept. 15, 2021, 10:51 a.m. | #2
Sorry.  Fixed in a new patch.

Best,
Claudiu
________________________________
From: Alan Modra <amodra@gmail.com>

Sent: Wednesday, September 15, 2021 4:59 AM
To: Claudiu Zissulescu <claziss@gmail.com>
Cc: binutils@sourceware.org <binutils@sourceware.org>; Francois Bedard <fbedard@synopsys.com>; Alexey Brodkin <abrodkin@synopsys.com>; Claudiu Zissulescu <claziss@synopsys.com>
Subject: Re: [committed] arc: Fix potential invalid pointer access when fixing got symbols.

>        * ld-arc/got-weak.d: New file.

>        * ld-arc/got-weak.s: Likewise.


Fails on arc-linux-uclibc

regexp_diff match failure
regexp "^00000100 <.*>:$"
line   "00010074 <__start>:"
regexp_diff match failure
regexp "^ 100:  2730 7f80 0000 2014      ld      r0,\[pcl,0x2014\].*$"
line   "   10074:       2730 7f80 0000 2008      ld      r0,[pcl,0x2008] ;1207c <.got>"
FAIL: ld-arc/got-weak

--
Alan Modra
Australia Development Lab, IBM

Patch

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index d878d051217..a69b50c6199 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,12 @@ 
+2021-09-014  Cupertino Miranda  <cmiranda@synopsys.com>
+           Claudiu Zissulescu  <claziss@synopsys.com>
+
+	* arc-got.h (arc_static_sym_data): New structure.
+	(get_static_sym_data): New function.
+	(relocate_fix_got_relocs_for_got_info): Move the computation fo
+	symbol value and section to above introduced function, and use
+	this new function.
+
 2021-09-07  Luis Machado  <luis.machado@linaro.org>
 
 	Revert: [AArch64] MTE corefile support
diff --git a/bfd/arc-got.h b/bfd/arc-got.h
index 76fc761a8cf..c45d981b504 100644
--- a/bfd/arc-got.h
+++ b/bfd/arc-got.h
@@ -262,6 +262,48 @@  arc_fill_got_info_for_reloc (enum tls_type_e type,
   return true;
 }
 
+struct arc_static_sym_data {
+  bfd_vma sym_value;
+  const char *symbol_name;
+};
+
+static struct arc_static_sym_data
+get_static_sym_data (unsigned long  r_symndx,
+		     Elf_Internal_Sym  *local_syms,
+		     asection **local_sections,
+		     struct elf_link_hash_entry *h,
+		     struct arc_relocation_data *reloc_data)
+{
+  static const char local_name[] = "(local)";
+  struct arc_static_sym_data ret = { 0, NULL };
+
+  if (h != NULL)
+    {
+      BFD_ASSERT (h->root.type != bfd_link_hash_undefweak
+		  && h->root.type != bfd_link_hash_undefined);
+      /* TODO: This should not be here.  */
+      reloc_data->sym_value = h->root.u.def.value;
+      reloc_data->sym_section = h->root.u.def.section;
+
+      ret.sym_value = h->root.u.def.value
+	+ h->root.u.def.section->output_section->vma
+	+ h->root.u.def.section->output_offset;
+
+      ret.symbol_name = h->root.root.string;
+    }
+  else
+  {
+    Elf_Internal_Sym *sym = local_syms + r_symndx;
+    asection *sec = local_sections[r_symndx];
+
+    ret.sym_value = sym->st_value
+      + sec->output_section->vma
+      + sec->output_offset;
+
+    ret.symbol_name = local_name;
+  }
+  return ret;
+}
 
 static bfd_vma
 relocate_fix_got_relocs_for_got_info (struct got_entry **	   list_p,
@@ -290,38 +332,7 @@  relocate_fix_got_relocs_for_got_info (struct got_entry **	   list_p,
 	      && SYMBOL_REFERENCES_LOCAL (info, h))))
     {
       const char ATTRIBUTE_UNUSED *symbol_name;
-      static const char local_name[] = "(local)";
-      asection *tls_sec = NULL;
-      bfd_vma sym_value = 0;
-
-      if (h != NULL)
-	{
-	  /* TODO: This should not be here.  */
-	  reloc_data->sym_value = h->root.u.def.value;
-	  reloc_data->sym_section = h->root.u.def.section;
-
-	  sym_value = h->root.u.def.value
-	    + h->root.u.def.section->output_section->vma
-	    + h->root.u.def.section->output_offset;
-
-	  tls_sec = elf_hash_table (info)->tls_sec;
-
-	  symbol_name = h->root.root.string;
-	}
-      else
-	{
-	  Elf_Internal_Sym *sym = local_syms + r_symndx;
-	  asection *sec = local_sections[r_symndx];
-
-	  sym_value = sym->st_value
-	    + sec->output_section->vma
-	    + sec->output_offset;
-
-	  tls_sec = elf_hash_table (info)->tls_sec;
-
-	  symbol_name = local_name;
-	}
-
+      asection *tls_sec = elf_hash_table (info)->tls_sec;
 
       if (entry && !entry->processed)
 	{
@@ -335,8 +346,12 @@  relocate_fix_got_relocs_for_got_info (struct got_entry **	   list_p,
 		if (h == NULL || h->forced_local
 		   || !elf_hash_table (info)->dynamic_sections_created)
 		  {
+		    struct arc_static_sym_data tmp =
+		      get_static_sym_data (r_symndx, local_syms, local_sections,
+					   h, reloc_data);
+
 		    bfd_put_32 (output_bfd,
-			    sym_value - sec_vma
+			    tmp.sym_value - sec_vma
 			    + (elf_hash_table (info)->dynamic_sections_created
 			       ? 0
 			       : (align_power (0,
@@ -355,7 +370,7 @@  relocate_fix_got_relocs_for_got_info (struct got_entry **	   list_p,
 			     + entry->offset
 			     + (entry->existing_entries == TLS_GOT_MOD_AND_OFF
 				? 4 : 0)),
-			  symbol_name);
+			  tmp.symbol_name);
 		  }
 	      }
 	      break;
@@ -366,8 +381,12 @@  relocate_fix_got_relocs_for_got_info (struct got_entry **	   list_p,
 		bfd_vma ATTRIBUTE_UNUSED sec_vma
 		  = tls_sec->output_section->vma;
 
+		struct arc_static_sym_data tmp =
+		  get_static_sym_data (r_symndx, local_syms, local_sections,
+				       h, reloc_data);
+
 		bfd_put_32 (output_bfd,
-			    sym_value - sec_vma
+			    tmp.sym_value - sec_vma
 			    + (elf_hash_table (info)->dynamic_sections_created
 			       ? 0
 			       : (align_power (TCB_SIZE,
@@ -386,7 +405,7 @@  relocate_fix_got_relocs_for_got_info (struct got_entry **	   list_p,
 			      + entry->offset
 			      + (entry->existing_entries == TLS_GOT_MOD_AND_OFF
 				 ? 4 : 0)),
-			   symbol_name);
+			   tmp.symbol_name);
 	      }
 	      break;
 
@@ -415,7 +434,8 @@  relocate_fix_got_relocs_for_got_info (struct got_entry **	   list_p,
 			       "@ %#08lx for sym %s in got offset %#lx\n",
 			       (long) (reloc_data->sym_value + sec_vma),
 			       (long) (htab->sgot->output_section->vma
-				       + htab->sgot->output_offset + entry->offset),
+				       + htab->sgot->output_offset
+				       + entry->offset),
 			       symbol_name,
 			       (long) entry->offset);
 		  }
diff --git a/ld/ChangeLog b/ld/ChangeLog
index 28deefc93fb..ebcd4cd9e93 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,8 @@ 
+2021-09-14  Claudiu Zissulescu  <claziss@synopsys.com>
+
+	* ld-arc/got-weak.d: New file.
+	* ld-arc/got-weak.s: Likewise.
+
 2021-07-26  Roland McGrath  <mcgrathr@google.com>
 
 	* testsuite/ld-x86-64/x86-64.exp (Build textrel-1): Use --warn-textrel.
diff --git a/ld/testsuite/ld-arc/got-weak.d b/ld/testsuite/ld-arc/got-weak.d
new file mode 100644
index 00000000000..cf15069b518
--- /dev/null
+++ b/ld/testsuite/ld-arc/got-weak.d
@@ -0,0 +1,12 @@ 
+#source: got-weak.s
+#as:
+#ld: -Bstatic
+#objdump: -d
+
+[^:]*:     file format elf32-.*arc
+
+
+Disassembly of section \.text:
+
+00000100 <.*>:
+ 100:	2730 7f80 0000 2014 	ld	r0,\[pcl,0x2014\].*
diff --git a/ld/testsuite/ld-arc/got-weak.s b/ld/testsuite/ld-arc/got-weak.s
new file mode 100644
index 00000000000..8ea18be3780
--- /dev/null
+++ b/ld/testsuite/ld-arc/got-weak.s
@@ -0,0 +1,7 @@ 
+	.cpu archs
+
+	.weak symb
+	.global __start
+	.text
+__start:
+	ld	r0,[pcl,@symb@gotpc]