loongarch readelf support

Message ID ZHkxOZ+7zNpFcAUv@squeak.grove.modra.org
State Unresolved
Headers
Series loongarch readelf support |

Checks

Context Check Description
snail/binutils-gdb-check warning Git am fail log

Commit Message

Alan Modra June 2, 2023, midnight UTC
  This fixes two buffer overflows found by fuzzers.

	* readelf.c (target_specific_reloc_handling): Sanity check
	loongarch reloc symbol index.  Don't apply reloc after errors.
	Reduce translation work of "invalid symbol index" error message.
  

Comments

Alan Modra June 3, 2023, 8:05 a.m. UTC | #1
Another segfault on fuzzed input.

	* readelf.c (target_specific_reloc_handling): Sanity check
	loongarch reloc r_offset.

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 042d2301517..23d3e21bea6 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -14021,8 +14021,9 @@ target_specific_reloc_handling (Filedata *filedata,
 		unsigned int reloc_size = 0;
 		int leb_ret = 0;
 
-		value = read_leb128 (start + reloc->r_offset, end, false,
-			      &reloc_size, &leb_ret);
+		if (reloc->r_offset < (size_t) (end - start))
+		  value = read_leb128 (start + reloc->r_offset, end, false,
+				       &reloc_size, &leb_ret);
 		if (leb_ret != 0 || reloc_size == 0 || reloc_size > 8)
 		  error (_("LoongArch ULEB128 field at 0x%lx contains invalid "
 			   "ULEB128 value\n"),
  
Vaseeharan Vinayagamoorthy June 6, 2023, 8:58 a.m. UTC | #2
I have observed the following build error:

binutils-gdb/binutils/readelf.c:14044:15: error: ‘value’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
         value -= reloc->r_addend + symtab[sym_index].st_value;
               ^
cc1: all warnings being treated as errors
make[3]: *** [Makefile:1932: readelf.o] Error 1


Build machine: x86_64-none-linux-gnu
For Host: x86_64-none-linux-gnu
Target: aarch64-none-elf / aarch64_be-none-elf / arm-none-eabi

Please let me know if you require any other information to help resolve this.
  
Alan Modra June 6, 2023, 10:19 a.m. UTC | #3
On Tue, Jun 06, 2023 at 08:58:16AM +0000, Vaseeharan Vinayagamoorthy wrote:
> I have observed the following build error:
> 
> binutils-gdb/binutils/readelf.c:14044:15: error: ‘value’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>          value -= reloc->r_addend + symtab[sym_index].st_value;
>                ^
> cc1: all warnings being treated as errors
> make[3]: *** [Makefile:1932: readelf.o] Error 1

It is a false positive, but I'll commit the obvious patch to fix
this.
  

Patch

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 7292dd0941a..042d2301517 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -14028,24 +14028,32 @@  target_specific_reloc_handling (Filedata *filedata,
 			   "ULEB128 value\n"),
 			 (long) reloc->r_offset);
 
-		if (107 == reloc_type)
-		  value += (reloc->r_addend + symtab[sym_index].st_value);
-		else if (108 == reloc_type)
-		  value -= (reloc->r_addend + symtab[sym_index].st_value);
-
-		/* Write uleb128 value to p.  */
-		bfd_byte c;
-		bfd_byte *p = start + reloc->r_offset;
-		do
+		else if (sym_index >= num_syms)
+		  error (_("%s reloc contains invalid symbol index "
+			   "%" PRIu64 "\n"),
+			 (reloc_type == 107
+			  ? "R_LARCH_ADD_ULEB128"
+			  : "R_LARCH_SUB_ULEB128"),
+			 sym_index);
+		else
 		  {
-		    c = value & 0x7f;
-		    if (reloc_size > 1)
-		      c |= 0x80;
-		    *(p++) = c;
-		    value >>= 7;
-		    reloc_size--;
+		    if (reloc_type == 107)
+		      value += reloc->r_addend + symtab[sym_index].st_value;
+		    else
+		      value -= reloc->r_addend + symtab[sym_index].st_value;
+
+		    /* Write uleb128 value to p.  */
+		    bfd_byte *p = start + reloc->r_offset;
+		    do
+		      {
+			bfd_byte c = value & 0x7f;
+			value >>= 7;
+			if (--reloc_size != 0)
+			  c |= 0x80;
+			*p++ = c;
+		      }
+		    while (reloc_size);
 		  }
-		while (reloc_size);
 
 		return true;
 	      }
@@ -14075,8 +14083,8 @@  target_specific_reloc_handling (Filedata *filedata,
 	  case 23: /* R_MSP430X_GNU_SUB_ULEB128 */
 	    /* PR 21139.  */
 	    if (sym_index >= num_syms)
-	      error (_("MSP430 SYM_DIFF reloc contains invalid symbol index"
-		       " %" PRIu64 "\n"), sym_index);
+	      error (_("%s reloc contains invalid symbol index "
+		       "%" PRIu64 "\n"), "MSP430 SYM_DIFF", sym_index);
 	    else
 	      saved_sym = symtab + sym_index;
 	    return true;
@@ -14126,9 +14134,8 @@  target_specific_reloc_handling (Filedata *filedata,
 			   " contains invalid ULEB128 value\n"),
 			 reloc->r_offset);
 		else if (sym_index >= num_syms)
-		  error (_("MSP430 reloc contains invalid symbol index "
-			   "%" PRIu64 "\n"),
-			 sym_index);
+		  error (_("%s reloc contains invalid symbol index "
+			   "%" PRIu64 "\n"), "MSP430", sym_index);
 		else
 		  {
 		    value = reloc->r_addend + (symtab[sym_index].st_value
@@ -14173,9 +14180,8 @@  target_specific_reloc_handling (Filedata *filedata,
 	    return true;
 	  case 33: /* R_MN10300_SYM_DIFF */
 	    if (sym_index >= num_syms)
-	      error (_("MN10300_SYM_DIFF reloc contains invalid symbol index "
-		       "%" PRIu64 "\n"),
-		     sym_index);
+	      error (_("%s reloc contains invalid symbol index "
+		       "%" PRIu64 "\n"), "MN10300_SYM_DIFF", sym_index);
 	    else
 	      saved_sym = symtab + sym_index;
 	    return true;
@@ -14188,9 +14194,8 @@  target_specific_reloc_handling (Filedata *filedata,
 		uint64_t value;
 
 		if (sym_index >= num_syms)
-		  error (_("MN10300 reloc contains invalid symbol index "
-			   "%" PRIu64 "\n"),
-			 sym_index);
+		  error (_("%s reloc contains invalid symbol index "
+			   "%" PRIu64 "\n"), "MN10300", sym_index);
 		else
 		  {
 		    value = reloc->r_addend + (symtab[sym_index].st_value
@@ -14233,8 +14238,8 @@  target_specific_reloc_handling (Filedata *filedata,
 	  case 0x80: /* R_RL78_SYM.  */
 	    saved_sym1 = saved_sym2;
 	    if (sym_index >= num_syms)
-	      error (_("RL78_SYM reloc contains invalid symbol index "
-		       "%" PRIu64 "\n"), sym_index);
+	      error (_("%s reloc contains invalid symbol index "
+		       "%" PRIu64 "\n"), "RL78_SYM", sym_index);
 	    else
 	      {
 		saved_sym2 = symtab[sym_index].st_value;