[PATCH 7/8] Validate symbol file using build-id.

Aleksandar Ristovski
2013-04-10T18:27:20Z

Full patch.
I also created new branch in the archer: aristovski/validate-binary-before-use

Thanks,

Aleksandar

* solib-darwin.c (_initialize_darwin_solib): Assign validate value.
* solib-dsbt.c (_initialize_dsbt_solib): Ditto.
* solib-frv.c (_initialize_frv_solib): Ditto.
* solib-ia64-hpux.c (ia64_hpux_target_so_ops): Ditto.
* solib-irix.c (_initialize_irix_solib): Ditto.
* solib-osf.c (_initialize_osf_solib): Ditto.
* solib-pa64.c (_initialize_pa64_solib): Ditto.
* solib-som.c (_initialize_som_solib): Ditto.
* solib-spu.c (set_spu_solib_ops): Ditto.
* solib-sunos.c (_initialize_sunos_solib): Ditto.
* solib-svr4.c (NOTE_GNU_BUILD_ID_NAME): New define.
(svr4_validate): New function.
(library_list_start_library): Parse 'build-id' attribute.
(svr4_library_attributes): Add 'build-id' attribute.
(_initialize_svr4_solib): Assign validate value.
* solib-target.c (solib.h): Include.
(_initialize_solib_target): Assign validate value.
* solib.c (solib_map_sections): Use ops->validate.
(free_so): Free build_id.
(solib_validate): New function.
* solib.h (solib_validate): New declaration.
* solist.h (so_list): New fields 'build_idsz' and 'build_id'.
(target_so_ops): New field 'validate'.
---
gdb/solib-darwin.c | 1 +
gdb/solib-dsbt.c | 1 +
gdb/solib-frv.c | 1 +
gdb/solib-ia64-hpux.c | 1 +
gdb/solib-irix.c | 1 +
gdb/solib-osf.c | 1 +
gdb/solib-pa64.c | 1 +
gdb/solib-som.c | 1 +
gdb/solib-spu.c | 1 +
gdb/solib-sunos.c | 1 +
gdb/solib-svr4.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++
gdb/solib-target.c | 2 +
gdb/solib.c | 20 ++++++++
gdb/solib.h | 4 ++
gdb/solist.h | 14 +++++
15 files changed, 185 insertions(+)

diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index b9a4be1..ff57016 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -647,4 +647,5 @@ _initialize_darwin_solib (void)
darwin_so_ops.in_dynsym_resolve_code = darwin_in_dynsym_resolve_code;
darwin_so_ops.lookup_lib_global_symbol = darwin_lookup_lib_symbol;
darwin_so_ops.bfd_open = darwin_bfd_open;
+ darwin_so_ops.validate = default_solib_validate;
}
diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index e2822c1..6cadf82 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -1182,6 +1182,7 @@ _initialize_dsbt_solib (void)
dsbt_so_ops.open_symbol_file_object = open_symbol_file_object;
dsbt_so_ops.in_dynsym_resolve_code = dsbt_in_dynsym_resolve_code;
dsbt_so_ops.bfd_open = solib_bfd_open;
+ dsbt_so_ops.validate = default_solib_validate;

/* Debug this file's internals. */
add_setshow_zuinteger_cmd ("solib-dsbt", class_maintenance,
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 52588bc..741ede5 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -1182,6 +1182,7 @@ _initialize_frv_solib (void)
frv_so_ops.open_symbol_file_object = open_symbol_file_object;
frv_so_ops.in_dynsym_resolve_code = frv_in_dynsym_resolve_code;
frv_so_ops.bfd_open = solib_bfd_open;
+ frv_so_ops.validate = default_solib_validate;

/* Debug this file's internals. */
add_setshow_zuinteger_cmd ("solib-frv", class_maintenance,
diff --git a/gdb/solib-ia64-hpux.c b/gdb/solib-ia64-hpux.c
index 67085d7..6fb146c 100644
--- a/gdb/solib-ia64-hpux.c
+++ b/gdb/solib-ia64-hpux.c
@@ -686,6 +686,7 @@ ia64_hpux_target_so_ops (void)
ops->open_symbol_file_object = ia64_hpux_open_symbol_file_object;
ops->in_dynsym_resolve_code = ia64_hpux_in_dynsym_resolve_code;
ops->bfd_open = solib_bfd_open;
+ ops->validate = default_solib_validate;

return ops;
}
diff --git a/gdb/solib-irix.c b/gdb/solib-irix.c
index af3e7d6..871fb19 100644
--- a/gdb/solib-irix.c
+++ b/gdb/solib-irix.c
@@ -652,4 +652,5 @@ _initialize_irix_solib (void)
irix_so_ops.open_symbol_file_object = irix_open_symbol_file_object;
irix_so_ops.in_dynsym_resolve_code = irix_in_dynsym_resolve_code;
irix_so_ops.bfd_open = solib_bfd_open;
+ irix_so_ops.validate = default_solib_validate;
}
diff --git a/gdb/solib-osf.c b/gdb/solib-osf.c
index d05c5c1..5b1cd0b 100644
--- a/gdb/solib-osf.c
+++ b/gdb/solib-osf.c
@@ -633,6 +633,7 @@ _initialize_osf_solib (void)
osf_so_ops.open_symbol_file_object = osf_open_symbol_file_object;
osf_so_ops.in_dynsym_resolve_code = osf_in_dynsym_resolve_code;
osf_so_ops.bfd_open = solib_bfd_open;
+ osf_so_ops.validate = default_solib_validate;

/* FIXME: Don't do this here. *_gdbarch_init() should set so_ops. */
current_target_so_ops = &osf_so_ops;
diff --git a/gdb/solib-pa64.c b/gdb/solib-pa64.c
index f646cfb..795bcda 100644
--- a/gdb/solib-pa64.c
+++ b/gdb/solib-pa64.c
@@ -621,6 +621,7 @@ _initialize_pa64_solib (void)
pa64_so_ops.open_symbol_file_object = pa64_open_symbol_file_object;
pa64_so_ops.in_dynsym_resolve_code = pa64_in_dynsym_resolve_code;
pa64_so_ops.bfd_open = solib_bfd_open;
+ pa64_so_ops.validate = default_solib_validate;

memset (&dld_cache, 0, sizeof (dld_cache));
}
diff --git a/gdb/solib-som.c b/gdb/solib-som.c
index 650e3df..2fbf0a2 100644
--- a/gdb/solib-som.c
+++ b/gdb/solib-som.c
@@ -811,6 +811,7 @@ _initialize_som_solib (void)
som_so_ops.open_symbol_file_object = som_open_symbol_file_object;
som_so_ops.in_dynsym_resolve_code = som_in_dynsym_resolve_code;
som_so_ops.bfd_open = solib_bfd_open;
+ som_so_ops.validate = default_solib_validate;
}

void
diff --git a/gdb/solib-spu.c b/gdb/solib-spu.c
index 7be5232..4cc343b 100644
--- a/gdb/solib-spu.c
+++ b/gdb/solib-spu.c
@@ -519,6 +519,7 @@ set_spu_solib_ops (struct gdbarch *gdbarch)
spu_so_ops.current_sos = spu_current_sos;
spu_so_ops.bfd_open = spu_bfd_open;
spu_so_ops.lookup_lib_global_symbol = spu_lookup_lib_symbol;
+ spu_so_ops.validate = default_solib_validate;
}

set_solib_ops (gdbarch, &spu_so_ops);
diff --git a/gdb/solib-sunos.c b/gdb/solib-sunos.c
index 5863fc2..71c8ee3 100644
--- a/gdb/solib-sunos.c
+++ b/gdb/solib-sunos.c
@@ -738,6 +738,7 @@ _initialize_sunos_solib (void)
sunos_so_ops.open_symbol_file_object = open_symbol_file_object;
sunos_so_ops.in_dynsym_resolve_code = sunos_in_dynsym_resolve_code;
sunos_so_ops.bfd_open = solib_bfd_open;
+ sunos_so_ops.validate = default_solib_validate;

/* FIXME: Don't do this here. *_gdbarch_init() should set so_ops. */
current_target_so_ops = &sunos_so_ops;
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index bb2a4e9..54a7e6f 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -47,6 +47,8 @@
#include "exceptions.h"
#include "gdb_bfd.h"

+#define NOTE_GNU_BUILD_ID_NAME ".note.gnu.build-id"
+
static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
static int svr4_have_link_map_offsets (void);
static void svr4_relocate_main_executable (void);
@@ -871,6 +873,117 @@ svr4_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
return (name_lm >= vaddr && name_lm < vaddr + size);
}

+/* Validate SO by comparing build-id from the associated bfd and
+ corresponding build-id from target memory. */
+
+static int
+svr4_validate (const struct so_list *const so)
+{
+ gdb_byte *build_id;
+ size_t build_idsz;
+
+ gdb_assert (so != NULL);
+
+ if (so->abfd == NULL)
+ return 1;
+
+ if (!bfd_check_format (so->abfd, bfd_object)
+ || bfd_get_flavour (so->abfd) != bfd_target_elf_flavour
+ || elf_tdata (so->abfd)->build_id == NULL)
+ return 1;
+
+ build_id = so->build_id;
+ build_idsz = so->build_idsz;
+
+ if (build_id == NULL)
+ {
+ /* Get build_id from NOTE_GNU_BUILD_ID_NAME section.
+ This is a fallback mechanism for targets that do not
+ implement TARGET_OBJECT_SOLIB_SVR4. */
+
+ const asection *const asec
+ = bfd_get_section_by_name (so->abfd, NOTE_GNU_BUILD_ID_NAME);
+ ULONGEST bfd_sect_size;
+
+ if (asec == NULL)
+ return 1;
+
+ bfd_sect_size = bfd_get_section_size (asec);
+
+ if ((asec->flags & SEC_LOAD) == SEC_LOAD
+ && bfd_sect_size != 0
+ && strcmp (bfd_section_name (asec->bfd, asec),
+ NOTE_GNU_BUILD_ID_NAME) == 0)
+ {
+ const enum bfd_endian byte_order
+ = gdbarch_byte_order (target_gdbarch ());
+ Elf_External_Note *const note = xmalloc (bfd_sect_size);
+ gdb_byte *const note_raw = (void *) note;
+ struct cleanup *cleanups = make_cleanup (xfree, note);
+
+ if (target_read_memory (bfd_get_section_vma (so->abfd, asec)
+ + lm_addr_check (so, so->abfd),
+ note_raw, bfd_sect_size) == 0)
+ {
+ build_idsz
+ = extract_unsigned_integer ((gdb_byte *) note->descsz,
+ sizeof (note->descsz),
+ byte_order);
+
+ if (build_idsz == elf_tdata (so->abfd)->build_id->size)
+ {
+ const char gnu[4] = "GNU\0";
+
+ if (memcmp (note->name, gnu, 4) == 0)
+ {
+ ULONGEST namesz
+ = extract_unsigned_integer ((gdb_byte *) note->namesz,
+ sizeof (note->namesz),
+ byte_order);
+ CORE_ADDR build_id_offs;
+
+ /* Rounded to next sizeof (ElfXX_Word). */
+ namesz = ((namesz + (sizeof (note->namesz) - 1))
+ & ~((ULONGEST) (sizeof (note->namesz) - 1)));
+ build_id_offs = (sizeof (note->namesz)
+ + sizeof (note->descsz)
+ + sizeof (note->type) + namesz);
+ build_id = xmalloc (build_idsz);
+ memcpy (build_id, note_raw + build_id_offs, build_idsz);
+ }
+ }
+
+ if (build_id == NULL)
+ {
+ /* If we are here, it means target memory read succeeded
+ but note was not where it was expected according to the
+ abfd. Allow the logic below to perform the check
+ with an impossible build-id and fail validation. */
+ build_idsz = 0;
+ build_id = xstrdup ("");
+ }
+
+ }
+ do_cleanups (cleanups);
+ }
+ }
+
+ if (build_id != NULL)
+ {
+ const int match
+ = elf_tdata (so->abfd)->build_id->size == build_idsz
+ && memcmp (build_id, elf_tdata (so->abfd)->build_id->data,
+ elf_tdata (so->abfd)->build_id->size) == 0;
+
+ if (build_id != so->build_id)
+ xfree (build_id);
+
+ return match;
+ }
+
+ return 1;
+}
+
/* Implement the "open_symbol_file_object" target_so_ops method.

If no open symbol file, attempt to locate and open the main symbol
@@ -999,6 +1112,9 @@ library_list_start_library (struct gdb_xml_parser *parser,
ULONGEST *lmp = xml_find_attribute (attributes, "lm")->value;
ULONGEST *l_addrp = xml_find_attribute (attributes, "l_addr")->value;
ULONGEST *l_ldp = xml_find_attribute (attributes, "l_ld")->value;
+ const struct gdb_xml_value *const att_build_id
+ = xml_find_attribute (attributes, "build-id");
+ const char *const hex_build_id = att_build_id ? att_build_id->value : NULL;
struct so_list *new_elem;

new_elem = XZALLOC (struct so_list);
@@ -1010,6 +1126,23 @@ library_list_start_library (struct gdb_xml_parser *parser,
strncpy (new_elem->so_name, name, sizeof (new_elem->so_name) - 1);
new_elem->so_name[sizeof (new_elem->so_name) - 1] = 0;
strcpy (new_elem->so_original_name, new_elem->so_name);
+ if (hex_build_id != NULL)
+ {
+ const size_t hex_build_id_len = strlen (hex_build_id);
+
+ if (hex_build_id_len > 0)
+ {
+ new_elem->build_id = xmalloc (hex_build_id_len / 2);
+ new_elem->build_idsz = hex2bin (hex_build_id, new_elem->build_id,
+ hex_build_id_len);
+ if (new_elem->build_idsz != (hex_build_id_len / 2))
+ {
+ xfree (new_elem->build_id);
+ new_elem->build_id = NULL;
+ new_elem->build_idsz = 0;
+ }
+ }
+ }

*list->tailp = new_elem;
list->tailp = &new_elem->next;
@@ -1044,6 +1177,7 @@ static const struct gdb_xml_attribute svr4_library_attributes[] =
{ "lm", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
{ "l_addr", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
{ "l_ld", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
+ { "build-id", GDB_XML_AF_OPTIONAL, NULL, NULL },
{ NULL, GDB_XML_AF_NONE, NULL, NULL }
};

@@ -2458,4 +2592,5 @@ _initialize_svr4_solib (void)
svr4_so_ops.lookup_lib_global_symbol = elf_lookup_lib_symbol;
svr4_so_ops.same = svr4_same;
svr4_so_ops.keep_data_in_core = svr4_keep_data_in_core;
+ svr4_so_ops.validate = svr4_validate;
}
diff --git a/gdb/solib-target.c b/gdb/solib-target.c
index 0ad29ba..b67509f 100644
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -25,6 +25,7 @@
#include "target.h"
#include "vec.h"
#include "solib-target.h"
+#include "solib.h"

#include "gdb_string.h"

@@ -501,6 +502,7 @@ _initialize_solib_target (void)
solib_target_so_ops.in_dynsym_resolve_code
= solib_target_in_dynsym_resolve_code;
solib_target_so_ops.bfd_open = solib_bfd_open;
+ solib_target_so_ops.validate = default_solib_validate;

/* Set current_target_so_ops to solib_target_so_ops if not already
set. */
diff --git a/gdb/solib.c b/gdb/solib.c
index 8129c0f..20c709e 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -471,6 +471,17 @@ solib_map_sections (struct so_list *so)
error (_("Shared library file name is too long."));
strcpy (so->so_name, bfd_get_filename (abfd));

+ gdb_assert (ops->validate != NULL);
+
+ if (!ops->validate (so))
+ {
+ warning (_("Shared object \"%s\" could not be validated "
+ "and will be ignored."), so->so_name);
+ gdb_bfd_unref (so->abfd);
+ so->abfd = NULL;
+ return 0;
+ }
+
if (build_section_table (abfd, &so->sections, &so->sections_end))
{
error (_("Can't find the file sections in `%s': %s"),
@@ -551,6 +562,7 @@ free_so (struct so_list *so)
{
struct target_so_ops *ops = solib_ops (target_gdbarch ());

+ xfree (so->build_id);
free_so_symbols (so);
ops->free_so (so);

@@ -1448,6 +1460,14 @@ gdb_bfd_lookup_symbol (bfd *abfd,
return symaddr;
}

+/* Default implementation does not perform any validation. */
+
+int
+default_solib_validate (const struct so_list *const so)
+{
+ return 1; /* No validation. */
+}
+
extern initialize_file_ftype _initialize_solib; /* -Wmissing-prototypes */

void
diff --git a/gdb/solib.h b/gdb/solib.h
index b811866..670949a 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -90,4 +90,8 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
void *),
void *data);

+/* Default validation always returns 1. */
+
+extern int default_solib_validate (const struct so_list *so);
+
#endif /* SOLIB_H */
diff --git a/gdb/solist.h b/gdb/solist.h
index f784fc3..72e003d 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -75,6 +75,16 @@ struct so_list
There may not be just one (e.g. if two segments are relocated
differently); but this is only used for "info sharedlibrary". */
CORE_ADDR addr_low, addr_high;
+
+ /* Build id in raw format, contains verbatim contents of
+ .note.gnu.build-id including note header. This is actual
+ BUILD_ID which comes either from the remote target via qXfer
+ packet or via reading target memory. Therefore, it may differ
+ from the build-id of the associated bfd. In a normal
+ scenario, this so would soon lose its abfd due to failed
+ validation. */
+ size_t build_idsz;
+ gdb_byte *build_id;
};

struct target_so_ops
@@ -148,6 +158,10 @@ struct target_so_ops
core file (in particular, for readonly sections). */
int (*keep_data_in_core) (CORE_ADDR vaddr,
unsigned long size);
+
+ /* Return 0 if SO does not match target SO it is supposed to
+ represent. Return 1 otherwise. */
+ int (*validate) (const struct so_list *so);
};

/* Free the memory associated with a (so_list *). */
--
1.7.10.4