[PATCH] Commit f64e188b5 broke core file support in gdb

Keith Seitz
2014-12-10T21:22:28Z

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