[PATCH] Commit f64e188b5 broke core file support in gdb

[Keith Seitz <keiths at redhat dot com>]

This commit causes hundreds of core file regressions in gdb:

commit f64e188b58f4aab4cbd03aa6e9fc1aa602546e26
Author: Nick Clifton <nickc@redhat.com>
Date:   Tue Dec 9 12:42:18 2014 +0000

    More fixes for memory access violations triggered by fuzzed binaries.
    
        PR binutils/17512
        * objdump.c (display_any_bfd): Avoid infinite loop closing and
        opening the same archive again and again.
    
        * archive64.c (bfd_elf64_archive_slurp_armap): Add range checks.
        * libbfd.c (safe_read_leb128): New function.
        * libbfd-in.h (safe_read_leb128): Add prototype.
        * libbfd.h: Regenerate.
        * elf-attrs.c (_bfd_elf_parse_attributes): Use safe_read_leb128.
        Check for an over-long subsection length.
        * elf.c (elf_parse_notes): Check that the namedata is long enough
        for the string comparison that is about to be performed.
        (elf_read_notes): Zero-terminate the note buffer.


This hunk is the culprit:

diff --git a/bfd/elf.c b/bfd/elf.c
index 405ec33..f6923b4 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9817,32 +9817,33 @@ elf_parse_notes (bfd *abfd, char *buf, size_t size, file_ptr offset)
 	  return TRUE;
 
 	case bfd_core:
-	  if (CONST_STRNEQ (in.namedata, "NetBSD-CORE"))
-	    {
-	      if (! elfcore_grok_netbsd_note (abfd, &in))
-		return FALSE;
-	    }
-	  else if (CONST_STRNEQ (in.namedata, "OpenBSD"))
-	    {
-	      if (! elfcore_grok_openbsd_note (abfd, &in))
-		return FALSE;
-	    }
-	  else if (CONST_STRNEQ (in.namedata, "QNX"))
+	  {
+	    struct
 	    {
-	      if (! elfcore_grok_nto_note (abfd, &in))
-		return FALSE;
+	      const char * string;
+	      bfd_boolean (* func)(bfd *, Elf_Internal_Note *);
 	    }
-	  else if (CONST_STRNEQ (in.namedata, "SPU/"))
+	    grokers[] =
 	    {
-	      if (! elfcore_grok_spu_note (abfd, &in))
-		return FALSE;
-	    }
-	  else
-	    {
-	      if (! elfcore_grok_note (abfd, &in))
-		return FALSE;
-	    }
-	  break;
+	      { "", elfcore_grok_note },
+	      { "NetBSD-CORE", elfcore_grok_netbsd_note },
+	      { "OpenBSD", elfcore_grok_openbsd_note },
+	      { "QNX", elfcore_grok_nto_note },
+	      { "SPU/", elfcore_grok_spu_note }
+	    };
+	    int i;
+
+	    for (i = ARRAY_SIZE (grokers); i--;)
+	      if (in.namesz >= sizeof grokers[i].string - 1
+		  && strncmp (in.namedata, grokers[i].string,
+			      sizeof (grokers[i].string) - 1) == 0)
+		{
+		  if (! grokers[i].func (abfd, & in))
+		    return FALSE;
+		  break;
+		}
+	    break;
+	  }
 
 	case bfd_object:
 	  if (in.namesz == sizeof "GNU" && strcmp (in.namedata, "GNU") == 0)

Note how this applies sizeof to grokers[i].string...

Keith

bfd/ChangeLog

	* elf.c (elf_parse_notes): Define convenience macro
	GROKER_ELEMENT to add string lengths to 'grokers'.
	Use grokers.len instead of sizeof in string comparisons.
---
 bfd/elf.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index f7c1b9e..c8238ba 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9706,30 +9706,35 @@ elf_parse_notes (bfd *abfd, char *buf, size_t size, file_ptr offset)
 
 	case bfd_core:
 	  {
+#define GROKER_ELEMENT(S,F) {S, sizeof (S) - 1, F}
 	    struct
 	    {
 	      const char * string;
+	      size_t len;
 	      bfd_boolean (* func)(bfd *, Elf_Internal_Note *);
 	    }
 	    grokers[] =
 	    {
-	      { "", elfcore_grok_note },
-	      { "NetBSD-CORE", elfcore_grok_netbsd_note },
-	      { "OpenBSD", elfcore_grok_openbsd_note },
-	      { "QNX", elfcore_grok_nto_note },
-	      { "SPU/", elfcore_grok_spu_note }
+	      GROKER_ELEMENT ("", elfcore_grok_note),
+	      GROKER_ELEMENT ("NetBSD-CORE", elfcore_grok_netbsd_note),
+	      GROKER_ELEMENT ( "OpenBSD", elfcore_grok_openbsd_note),
+	      GROKER_ELEMENT ("QNX", elfcore_grok_nto_note),
+	      GROKER_ELEMENT ("SPU/", elfcore_grok_spu_note)
 	    };
+#undef GROKER_ELEMENT
 	    int i;
 
 	    for (i = ARRAY_SIZE (grokers); i--;)
-	      if (in.namesz >= sizeof grokers[i].string - 1
-		  && strncmp (in.namedata, grokers[i].string,
-			      sizeof (grokers[i].string) - 1) == 0)
-		{
-		  if (! grokers[i].func (abfd, & in))
-		    return FALSE;
-		  break;
-		}
+	      {
+		if (in.namesz >= grokers[i].len
+		    && strncmp (in.namedata, grokers[i].string,
+				grokers[i].len) == 0)
+		  {
+		    if (! grokers[i].func (abfd, & in))
+		      return FALSE;
+		    break;
+		  }
+	      }
 	    break;
 	  }
 
-- 
1.9.3