diff mbox

[v5,0/8] Validate binary before use

Message ID 20140319223004.14668.20989.stgit@host1.jankratochvil.net
State Not Applicable
Headers show

Commit Message

Jan Kratochvil March 19, 2014, 10:30 p.m. UTC
Hi,

git://sourceware.org/git/archer.git
jankratochvil/gdbserverbuildid

an update.  Patch below is an overall v4->v5 diff of the whole series.


Jan


v5
 * svr4_validate() considers missing local build-id as not-a-match
 * target_so_ops->validate() now returns not-a-match reason as a string
 * rename common/common-target.[ch] -> common/target-utils.[ch]
 * testcase runs (but broken) even on different-filesystem remote target
 * testcase simplified by using with_test_prefix()

v4
 * NEWS, doc/gdb.texinfo additions.
 * Used host-defs.h.
 * New set/show solib-build-id-force.
 * testsuite: Do not run on non-localhost remote targets.

v3
	[patchv3 0/8] Validate binary before use
	https://sourceware.org/ml/gdb-patches/2014-02/msg00842.html
	Message-ID: <20140227213229.GA21121@host2.jankratochvil.net>
 * Implemented the review comments I made.
 * Removed fetching build-id in solib-svr4.c for NAT run.

v2
	[PATCH 0/8] v2 - validate binary before use
	https://sourceware.org/ml/gdb-patches/2013-04/msg00472.html
	Message-ID: <1366127096-5744-1-git-send-email-ARistovski@qnx.com>

---

 gdb/Makefile.in                                  |   15 +
 gdb/NEWS                                         |   10 +
 gdb/cli/cli-utils.c                              |   24 -
 gdb/cli/cli-utils.h                              |    9 -
 gdb/common/common-utils.c                        |  125 +++++++
 gdb/common/common-utils.h                        |   11 +
 gdb/common/host-defs.h                           |   21 +
 gdb/common/linux-maps.c                          |  222 +++++++++++++
 gdb/common/linux-maps.h                          |   47 +++
 gdb/common/target-utils.c                        |  115 +++++++
 gdb/common/target-utils.h                        |   34 ++
 gdb/config/i386/linux.mh                         |    2 
 gdb/config/i386/linux64.mh                       |    2 
 gdb/defs.h                                       |   19 -
 gdb/doc/gdb.texinfo                              |   55 +++
 gdb/features/library-list-svr4.dtd               |   13 -
 gdb/gdbserver/Makefile.in                        |    9 -
 gdb/gdbserver/configure.srv                      |    2 
 gdb/gdbserver/gdbreplay.c                        |    6 
 gdb/gdbserver/linux-low.c                        |  389 ++++++++++++++++++++--
 gdb/linux-tdep.c                                 |  183 ++--------
 gdb/monitor.c                                    |   16 -
 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-svr4.c                                 |   85 +++++
 gdb/solib-target.c                               |    2 
 gdb/solib.c                                      |   62 +++-
 gdb/solib.h                                      |    4 
 gdb/solist.h                                     |   21 +
 gdb/target.c                                     |   96 +----
 gdb/testsuite/gdb.server/solib-mismatch-lib.c    |   29 ++
 gdb/testsuite/gdb.server/solib-mismatch-libmod.c |   29 ++
 gdb/testsuite/gdb.server/solib-mismatch.c        |   56 +++
 gdb/testsuite/gdb.server/solib-mismatch.exp      |  156 +++++++++
 gdb/utils.c                                      |   99 ------
 gdb/utils.h                                      |    2 
 43 files changed, 1531 insertions(+), 448 deletions(-)
 create mode 100644 gdb/common/linux-maps.c
 create mode 100644 gdb/common/linux-maps.h
 create mode 100644 gdb/common/target-utils.c
 create mode 100644 gdb/common/target-utils.h
 create mode 100644 gdb/testsuite/gdb.server/solib-mismatch-lib.c
 create mode 100644 gdb/testsuite/gdb.server/solib-mismatch-libmod.c
 create mode 100644 gdb/testsuite/gdb.server/solib-mismatch.c
 create mode 100644 gdb/testsuite/gdb.server/solib-mismatch.exp
diff mbox

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 1e2c5f6..2dc5c47 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -924,7 +924,7 @@  common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h \
 gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h common/linux-btrace.h \
 ctf.h common/i386-cpuid.h common/i386-gcc-cpuid.h target/resume.h \
 target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
-common/print-utils.h common/rsp-low.h common/common-target.h
+common/print-utils.h common/rsp-low.h common/target-utils.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -1023,7 +1023,7 @@  COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	gdb_vecs.o jit.o progspace.o skip.o probe.o \
 	common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o \
 	format.o registry.o btrace.o record-btrace.o waitstatus.o \
-	print-utils.o rsp-low.o common-target.o
+	print-utils.o rsp-low.o target-utils.o
 
 TSOBS = inflow.o
 
@@ -2137,8 +2137,8 @@  common-agent.o: $(srcdir)/common/agent.c
 	$(COMPILE) $(srcdir)/common/agent.c
 	$(POSTCOMPILE)
 
-common-target.o: ${srcdir}/common/common-target.c
-	$(COMPILE) $(srcdir)/common/common-target.c
+target-utils.o: ${srcdir}/common/target-utils.c
+	$(COMPILE) $(srcdir)/common/target-utils.c
 	$(POSTCOMPILE)
 
 vec.o: ${srcdir}/common/vec.c
diff --git a/gdb/common/linux-maps.c b/gdb/common/linux-maps.c
index bb587df..4d0b973 100644
--- a/gdb/common/linux-maps.c
+++ b/gdb/common/linux-maps.c
@@ -29,7 +29,7 @@ 
 #include "gdb_assert.h"
 #include <ctype.h>
 #include <string.h>
-#include "common-target.h"
+#include "target-utils.h"
 
 /* Service function for corefiles and info proc.  */
 
diff --git a/gdb/common/common-target.c b/gdb/common/target-utils.c
similarity index 99%
rename from gdb/common/common-target.c
rename to gdb/common/target-utils.c
index d6e5d60..84d1bca 100644
--- a/gdb/common/common-target.c
+++ b/gdb/common/target-utils.c
@@ -24,7 +24,7 @@ 
 #endif
 
 #include <string.h>
-#include "common-target.h"
+#include "target-utils.h"
 #include "gdb_assert.h"
 
 LONGEST
diff --git a/gdb/common/common-target.h b/gdb/common/target-utils.h
similarity index 93%
rename from gdb/common/common-target.h
rename to gdb/common/target-utils.h
index 9aedc12..f8ca972 100644
--- a/gdb/common/common-target.h
+++ b/gdb/common/target-utils.h
@@ -17,8 +17,8 @@ 
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifndef COMMON_COMMON_TARGET_H
-#define COMMON_COMMON_TARGET_H
+#ifndef COMMON_TARGET_UTILS_H
+#define COMMON_TARGET_UTILS_H
 
 typedef int (read_alloc_pread_ftype) (int handle, gdb_byte *read_buf, int len,
                                       ULONGEST offset, int *target_errno);
@@ -31,4 +31,4 @@  typedef LONGEST (read_stralloc_func_ftype) (const char *filename,
 extern char *read_stralloc (const char *filename,
 			    read_stralloc_func_ftype *func);
 
-#endif /* COMMON_COMMON_TARGET_H */
+#endif /* COMMON_TARGET_UTILS_H */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c683ede..a4af3ec 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17116,33 +17116,37 @@  discarded.
 
 @table @code
 @kindex set solib-build-id-force
+@cindex override @value{GDBN} build-id check
 @item set solib-build-id-force @var{mode}
 Setting to override @value{GDBN} build-id check.
 
-Inferior shared library and symbol file may contain unique build-id.
-If both build-ids are present but they do not match then this setting
-enables (@var{mode} is @code{on}) or disables (@var{mode} is @code{off})
-loading of such symbol file.  On systems where build-id is not present
-in files this setting has no effect.  The default value is @code{off}.
-
-Loading non-matching symbol file may confuse debugging including breakage
-of backtrace output.
-
+Inferior shared libraries and symbol files may contain unique build-id.
 By default @value{GDBN} will ignore symbol files with non-matching build-id
 while printing:
 
 @smallexample
-  Shared object "libfoo.so.1" could not be validated and will be ignored;
-  or use 'set solib-build-id-force'.
+  warning: Shared object "libfoo.so.1" could not be validated (remote
+  build ID 2bc1745e does not match local build ID a08f8767) and will be
+  ignored; or use 'set solib-build-id-force'.
 @end smallexample
 
 Turning on this setting would load such symbol file while still printing:
 
 @smallexample
-  Shared object "libfoo.so.1" could not be validated but it is being loaded
-  due to 'set solib-build-id-force'.
+  warning: Shared object "libfoo.so.1" could not be validated (remote
+  build ID 2bc1745e does not match local build ID a08f8767) but it is
+  being loaded due to 'set solib-build-id-force'.
 @end smallexample
 
+If remote build-id is present but it does not match local build-id (or local
+build-id is not present) then this setting enables (@var{mode} is @code{on}) or
+disables (@var{mode} is @code{off}) loading of such symbol file.  On systems
+where build-id is not present in the remote system this setting has no effect.
+The default value is @code{off}.
+
+Loading non-matching symbol file may confuse debugging including breakage
+of backtrace output.
+
 @kindex show solib-build-id-force
 @item show solib-build-id-force
 Display the current mode of build-id check override.
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 4ae4702..7d267f7 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -177,7 +177,7 @@  OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
       target.o waitstatus.o utils.o debug.o version.o vec.o gdb_vecs.o \
       mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
       common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
-      tdesc.o print-utils.o rsp-low.o common-target.o \
+      tdesc.o print-utils.o rsp-low.o target-utils.o \
       $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
 GDBSERVER_LIBS = @GDBSERVER_LIBS@
@@ -502,7 +502,7 @@  linux-procfs.o: ../common/linux-procfs.c
 linux-ptrace.o: ../common/linux-ptrace.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
-common-target.o: ../common/common-target.c
+target-utils.o: ../common/target-utils.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
 common-utils.o: ../common/common-utils.c
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 7f87380..65d8746 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -965,26 +965,59 @@  svr4_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
 /* Validate SO by comparing build-id from the associated bfd and
    corresponding build-id from target memory.  */
 
-static int
+static char *
 svr4_validate (const struct so_list *const so)
 {
+  bfd_byte *local_id;
+  size_t local_idsz;
+
   gdb_assert (so != NULL);
 
-  /* Target doesn't support reporting the build ID.  */
+  /* Target doesn't support reporting the build ID or the remote shared library
+     does not have build ID.  */
   if (so->build_id == NULL)
-    return 1;
+    return NULL;
 
-  if (so->abfd == NULL)
-    return 1;
+  /* Build ID may be present in the local file, just GDB is unable to retrieve
+     it.  As it has been reported by gdbserver it is not FSF gdbserver.  */
+  if (so->abfd == NULL
+      || !bfd_check_format (so->abfd, bfd_object)
+      || bfd_get_flavour (so->abfd) != bfd_target_elf_flavour)
+    return NULL;
 
-  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;
+  /* GDB has verified the local file really does not contain the build ID.  */
+  if (elf_tdata (so->abfd)->build_id == NULL)
+    {
+      char *remote_hex;
+
+      remote_hex = alloca (so->build_idsz * 2 + 1);
+      bin2hex (so->build_id, remote_hex, so->build_idsz);
+
+      return xstrprintf (_("remote build ID is %s "
+			   "but local file does not have build ID"),
+			 remote_hex);
+    }
+
+  local_id = elf_tdata (so->abfd)->build_id->data;
+  local_idsz = elf_tdata (so->abfd)->build_id->size;
+
+  if (so->build_idsz != local_idsz
+      || memcmp (so->build_id, local_id, so->build_idsz) != 0)
+    {
+      char *remote_hex, *local_hex;
+
+      remote_hex = alloca (so->build_idsz * 2 + 1);
+      bin2hex (so->build_id, remote_hex, so->build_idsz);
+      local_hex = alloca (local_idsz * 2 + 1);
+      bin2hex (local_id, local_hex, local_idsz);
+
+      return xstrprintf (_("remote build ID %s "
+			   "does not match local build ID %s"),
+			 remote_hex, local_hex);
+    }
 
-  return (so->build_idsz == elf_tdata (so->abfd)->build_id->size
-	  && memcmp (so->build_id, elf_tdata (so->abfd)->build_id->data,
-		     so->build_idsz) == 0);
+  /* Both build IDs are present and they match.  */
+  return NULL;
 }
 
 /* Implement the "open_symbol_file_object" target_so_ops method.
diff --git a/gdb/solib.c b/gdb/solib.c
index 5ce0d58..83366ae 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -484,7 +484,7 @@  static int
 solib_map_sections (struct so_list *so)
 {
   const struct target_so_ops *ops = solib_ops (target_gdbarch ());
-  char *filename;
+  char *filename, *validate_error;
   struct target_section *p;
   struct cleanup *old_chain;
   bfd *abfd;
@@ -502,20 +502,23 @@  solib_map_sections (struct so_list *so)
 
   gdb_assert (ops->validate != NULL);
 
-  if (!ops->validate (so))
+  validate_error = ops->validate (so);
+  if (validate_error != NULL)
     {
       if (!solib_build_id_force)
 	{
-	  warning (_("Shared object \"%s\" could not be validated "
-		     "and will be ignored; or use 'set solib-build-id-force'."),
-		   so->so_name);
+	  warning (_("Shared object \"%s\" could not be validated (%s) and "
+	             "will be ignored; or use 'set solib-build-id-force'."),
+		   so->so_name, validate_error);
+	  xfree (validate_error);
 	  gdb_bfd_unref (so->abfd);
 	  so->abfd = NULL;
 	  return 0;
 	}
-      warning (_("Shared object \"%s\" could not be validated "
+      warning (_("Shared object \"%s\" could not be validated (%s) "
 		 "but it is being loaded due to 'set solib-build-id-force'."),
-	       so->so_name);
+	       so->so_name, validate_error);
+      xfree (validate_error);
     }
 
   /* Copy the full path name into so_name, allowing symbol_file_add
@@ -1560,10 +1563,10 @@  remove_user_added_objfile (struct objfile *objfile)
 
 /* Default implementation does not perform any validation.  */
 
-int
+char *
 default_solib_validate (const struct so_list *const so)
 {
-  return 1; /* No validation.  */
+  return NULL; /* No validation.  */
 }
 
 extern initialize_file_ftype _initialize_solib; /* -Wmissing-prototypes */
diff --git a/gdb/solib.h b/gdb/solib.h
index 1f4d2b7..ab0dccd 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -100,6 +100,6 @@  extern void handle_solib_event (void);
 
 /* Default validation always returns 1.  */
 
-extern int default_solib_validate (const struct so_list *so);
+extern char *default_solib_validate (const struct so_list *so);
 
 #endif /* SOLIB_H */
diff --git a/gdb/solist.h b/gdb/solist.h
index b5fa91a..10b67ee 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -185,9 +185,10 @@  struct target_so_ops
        for this target.  */
     void (*handle_event) (void);
 
-    /* Return 0 if SO does not match target SO it is supposed to
-       represent.  Return 1 otherwise.  */
-    int (*validate) (const struct so_list *so);
+    /* Return NULL if SO does match target SO it is supposed to
+       represent.  Otherwise return string describing why it does not match.
+       Caller has to free the string.  */
+    char *(*validate) (const struct so_list *so);
   };
 
 /* Free the memory associated with a (so_list *).  */
diff --git a/gdb/target.c b/gdb/target.c
index fa51952..0e5ea33 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -45,7 +45,7 @@ 
 #include "gdb/fileio.h"
 #include "agent.h"
 #include "auxv.h"
-#include "common-target.h"
+#include "target-utils.h"
 
 static void target_info (char *, int);
 
diff --git a/gdb/testsuite/gdb.base/solib-mismatch-lib.c b/gdb/testsuite/gdb.server/solib-mismatch-lib.c
similarity index 100%
rename from gdb/testsuite/gdb.base/solib-mismatch-lib.c
rename to gdb/testsuite/gdb.server/solib-mismatch-lib.c
diff --git a/gdb/testsuite/gdb.base/solib-mismatch-libmod.c b/gdb/testsuite/gdb.server/solib-mismatch-libmod.c
similarity index 100%
rename from gdb/testsuite/gdb.base/solib-mismatch-libmod.c
rename to gdb/testsuite/gdb.server/solib-mismatch-libmod.c
diff --git a/gdb/testsuite/gdb.base/solib-mismatch.c b/gdb/testsuite/gdb.server/solib-mismatch.c
similarity index 91%
rename from gdb/testsuite/gdb.base/solib-mismatch.c
rename to gdb/testsuite/gdb.server/solib-mismatch.c
index 7bf425d..c8be18a 100644
--- a/gdb/testsuite/gdb.base/solib-mismatch.c
+++ b/gdb/testsuite/gdb.server/solib-mismatch.c
@@ -22,9 +22,9 @@ 
 #include <signal.h>
 #include <string.h>
 
-/* The following defines must correspond to solib-mismatch.exp */
+/* The following defines must correspond to solib-mismatch.exp .  */
 
-/* DIRNAME must be defined at compile time.  */
+/* DIRNAME and LIB must be defined at compile time.  */
 #ifndef DIRNAME
 #error DIRNAME not defined
 #endif
diff --git a/gdb/testsuite/gdb.base/solib-mismatch.exp b/gdb/testsuite/gdb.server/solib-mismatch.exp
similarity index 85%
rename from gdb/testsuite/gdb.base/solib-mismatch.exp
rename to gdb/testsuite/gdb.server/solib-mismatch.exp
index 4b723e0..9dbce7f 100644
--- a/gdb/testsuite/gdb.base/solib-mismatch.exp
+++ b/gdb/testsuite/gdb.server/solib-mismatch.exp
@@ -20,9 +20,8 @@  if ![is_remote target] {
   untested "only gdbserver supports build-id reporting"
   return -1
 }
-if { [board_info target sockethost] != "localhost:" } {
-  # The testcase below could be fixed for remote targets.
-  untested "only gdbserver on localhost is supported (found [board_info target sockethost])"
+if [is_remote host] {
+  untested "only local host is currently supported"
   return -1
 }
 
@@ -83,7 +82,7 @@  if { [gdb_compile_shlib "${srcdir}/${subdir}/${srclibfilerun}" "${binlibfilerun}
   return -1
 }
 
-proc solib_matching_test { solibfile symsloaded msg } {
+proc solib_matching_test { solibfile symsloaded } {
   global gdb_prompt
   global testfile
   global executable
@@ -117,7 +116,7 @@  proc solib_matching_test { solibfile symsloaded msg } {
 
   gdb_test "info sharedlibrary ${solibfile}" \
     "${expected_header}\r\n.*${expected_line}.*" \
-    "${msg} - Symbols for ${solibfile} loaded: expected '${symsloaded}'"
+    "Symbols for ${solibfile} loaded: expected '${symsloaded}'"
 
   return 0
 }
@@ -128,9 +127,8 @@  file copy -force "${binlibfiledirgdb}/${executable}" \
 		 "${binlibfiledirrun}/${executable}"
 
 # Test unstripped, .dynamic matching
-if { [solib_matching_test "${binlibfilebase}" "No" \
-      "test unstripped, .dynamic matching"] != 0 } {
-  untested "test unstripped, .dynamic matching"
+with_test_prefix "test unstripped, .dynamic matching" {
+  solib_matching_test "${binlibfilebase}" "No"
 }
 
 # Keep original so for debugging purposes
@@ -142,9 +140,8 @@  if {$result != 0} {
 }
 
 # Test --only-keep-debug, .dynamic matching so
-if { [solib_matching_test "${binlibfilebase}" "No" \
-      "test --only-keep-debug"] != 0 } {
-  untested "test --only-keep-debug"
+with_test_prefix "test --only-keep-debug" {
+  solib_matching_test "${binlibfilebase}" "No"
 }
 
 # Keep previous so for debugging puroses
@@ -154,7 +151,6 @@  file copy -force "${binlibfilegdb}" "${binlibfilegdb}-orig1"
 file copy -force "${binlibfilerun}" "${binlibfilegdb}"
 
 # Now test it does not mis-invalidate matching libraries
-if { [solib_matching_test "${binlibfilebase}" "Yes" \
-      "test matching libraries"] } {
-  untested "test matching libraries"
+with_test_prefix "test matching libraries" {
+  solib_matching_test "${binlibfilebase}" "Yes"
 }