This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH v5 0/8] Validate binary before use
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: gdb-patches at sourceware dot org
- Cc: Aleksandar Ristovski <ARistovski at qnx dot com>
- Date: Wed, 19 Mar 2014 23:30:04 +0100
- Subject: [PATCH v5 0/8] Validate binary before use
- Authentication-results: sourceware.org; auth=none
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 --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"
}