[PATCH v4 0/8] Validate binary before use

Jan Kratochvil
2014-03-02T19:53:44Z

Hi,

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

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

Jan

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-target.c | 115 +++++++
gdb/common/common-target.h | 34 ++
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/config/i386/linux.mh | 2
gdb/config/i386/linux64.mh | 2
gdb/defs.h | 19 -
gdb/doc/gdb.texinfo | 51 +++
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 | 52 +++
gdb/solib-target.c | 2
gdb/solib.c | 57 ++++
gdb/solib.h | 4
gdb/solist.h | 20 +
gdb/target.c | 96 ++----
gdb/testsuite/gdb.base/solib-mismatch-lib.c | 29 ++
gdb/testsuite/gdb.base/solib-mismatch-libmod.c | 29 ++
gdb/testsuite/gdb.base/solib-mismatch.c | 56 +++
gdb/testsuite/gdb.base/solib-mismatch.exp | 160 ++++++++++
gdb/utils.c | 99 ------
gdb/utils.h | 2
43 files changed, 1493 insertions(+), 447 deletions(-)
create mode 100644 gdb/common/common-target.c
create mode 100644 gdb/common/common-target.h
create mode 100644 gdb/common/linux-maps.c
create mode 100644 gdb/common/linux-maps.h
create mode 100644 gdb/testsuite/gdb.base/solib-mismatch-lib.c
create mode 100644 gdb/testsuite/gdb.base/solib-mismatch-libmod.c
create mode 100644 gdb/testsuite/gdb.base/solib-mismatch.c
create mode 100644 gdb/testsuite/gdb.base/solib-mismatch.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 2a384ba..629cc13 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -43,6 +43,12 @@ maint ada show ignore-descriptive-types
the user manual for more details on descriptive types and the intended
usage of this option.

+set solib-build-id-force (on|off)
+show solib-build-id-force
+ 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 (on) or disables (off) loading of such symbol file.
+
* New features in the GDB remote stub, GDBserver

** New option --debug-format=option1[,option2,...] allows one to add
@@ -51,6 +57,10 @@ maint ada show ignore-descriptive-types
Timestamps can also be turned on with the
"monitor set debug-format timestamps" command from GDB.

+ ** library-list-svr4 contains also optional attribute 'build-id' for
+ each library. GDB does not load library with build-id that
+ does not match such attribute.
+
* The 'record instruction-history' command now starts counting instructions
at one. This also affects the instruction ranges reported by the
'record function-call-history' command when given the /i modifier.
diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c
index aa98768..3e0c15e 100644
--- a/gdb/common/common-utils.c
+++ b/gdb/common/common-utils.c
@@ -23,6 +23,7 @@
#include "defs.h"
#endif

+#include "host-defs.h"
#include "gdb_assert.h"
#include <string.h>

diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index fc994a3..97889d5 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -25,25 +25,6 @@
#include <stddef.h>
#include <stdarg.h>

-/* Static target-system-dependent parameters for GDB. */
-
-/* * Number of bits in a char or unsigned char for the target machine.
- Just like CHAR_BIT in <limits.h> but describes the target machine. */
-#if !defined (TARGET_CHAR_BIT)
-#define TARGET_CHAR_BIT 8
-#endif
-
-/* * If we picked up a copy of CHAR_BIT from a configuration file
- (which may get it by including <limits.h>) then use it to set
- the number of bits in a host char. If not, use the same size
- as the target. */
-
-#if defined (CHAR_BIT)
-#define HOST_CHAR_BIT CHAR_BIT
-#else
-#define HOST_CHAR_BIT TARGET_CHAR_BIT
-#endif
-
/* If possible, define FUNCTION_NAME, a macro containing the name of
the function being defined. Since this macro may not always be
defined, all uses must be protected by appropriate macro definition
diff --git a/gdb/common/host-defs.h b/gdb/common/host-defs.h
index e4acef0..71a7029 100644
--- a/gdb/common/host-defs.h
+++ b/gdb/common/host-defs.h
@@ -19,6 +19,27 @@
#ifndef HOST_DEFS_H
#define HOST_DEFS_H

+#include <limits.h>
+
+/* Static host-system-dependent parameters for GDB. */
+
+/* * Number of bits in a char or unsigned char for the target machine.
+ Just like CHAR_BIT in <limits.h> but describes the target machine. */
+#if !defined (TARGET_CHAR_BIT)
+#define TARGET_CHAR_BIT 8
+#endif
+
+/* * If we picked up a copy of CHAR_BIT from a configuration file
+ (which may get it by including <limits.h>) then use it to set
+ the number of bits in a host char. If not, use the same size
+ as the target. */
+
+#if defined (CHAR_BIT)
+#define HOST_CHAR_BIT CHAR_BIT
+#else
+#define HOST_CHAR_BIT TARGET_CHAR_BIT
+#endif
+
#ifdef __MSDOS__
# define CANT_FORK
# define GLOBAL_CURDIR
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b1b29bd..c683ede 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17114,6 +17114,40 @@ libraries that were loaded by explicit user requests are not
discarded.
@end table

+@table @code
+@kindex set solib-build-id-force
+@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.
+
+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'.
+@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'.
+@end smallexample
+
+@kindex show solib-build-id-force
+@item show solib-build-id-force
+Display the current mode of build-id check override.
+@end table
+
Sometimes you may wish that @value{GDBN} stops and gives you control
when any of shared library events happen. The best way to do this is
to use @code{catch load} and @code{catch unload} (@pxref{Set
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 6f05bd4..972b609 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -5782,7 +5782,7 @@ struct find_memory_region_callback_data
};

/* Read build-id from PT_NOTE.
- Argument LOAD_ADDR pepresents run time virtual address corresponding to
+ Argument LOAD_ADDR represents run time virtual address corresponding to
the beginning of the first loadable segment. L_ADDR is displacement
as supplied by the dynamic linker. */

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 86a262a..14c4c19 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -970,9 +970,7 @@ svr4_validate (const struct so_list *const so)
{
gdb_assert (so != NULL);

- /* There is no way to safely fetch build-id from running inferior without OS
- specific code. The code from get_hex_build_id from gdbserver/linux-low.c
- could be used for GNU/Linux NAT target. */
+ /* Target doesn't support reporting the build ID. */
if (so->build_id == NULL)
return 1;

diff --git a/gdb/solib.c b/gdb/solib.c
index 4f92b63..5ce0d58 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -454,6 +454,20 @@ solib_bfd_open (char *pathname)
return abfd;
}

+/* Boolean for command 'set solib-build-id-force'. */
+static int solib_build_id_force = 0;
+
+/* Implement 'show solib-build-id-force'. */
+
+static void
+show_solib_build_id_force (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c, const char *value)
+{
+ fprintf_filtered (file, _("Loading of shared libraries "
+ "with non-matching build-id is %s.\n"),
+ value);
+}
+
/* Given a pointer to one of the shared objects in our list of mapped
objects, use the recorded name to open a bfd descriptor for the
object, build a section table, relocate all the section addresses
@@ -490,11 +504,18 @@ solib_map_sections (struct so_list *so)

if (!ops->validate (so))
{
+ 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);
+ gdb_bfd_unref (so->abfd);
+ so->abfd = NULL;
+ return 0;
+ }
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;
+ "but it is being loaded due to 'set solib-build-id-force'."),
+ so->so_name);
}

/* Copy the full path name into so_name, allowing symbol_file_add
@@ -1601,4 +1622,18 @@ PATH and LD_LIBRARY_PATH."),
reload_shared_libraries,
show_solib_search_path,
&setlist, &showlist);
+
+ add_setshow_boolean_cmd ("solib-build-id-force", class_support,
+ &solib_build_id_force, _("\
+Set loading of shared libraries with non-matching build-id."), _("\
+Show loading of shared libraries with non-matching build-id."), _("\
+Inferior shared library and symbol file may contain unique build-id.\n\
+If both build-ids are present but they do not match then this setting\n\
+enables (on) or disables (off) loading of such symbol file.\n\
+Loading non-matching symbol file may confuse debugging including breakage\n\
+of backtrace output."),
+ NULL,
+ show_solib_build_id_force,
+ &setlist, &showlist);
+
}
diff --git a/gdb/solist.h b/gdb/solist.h
index 68c88ee..b5fa91a 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -82,7 +82,13 @@ struct so_list
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. */
+ validation.
+ Reading target memory should be done by following execution view
+ of the binary (following program headers in the case of ELF).
+ Computing address from the linking view (following ELF section
+ headers) may give incorrect build-id memory address despite the
+ symbols still match.
+ Such an example is a prelinked vs. unprelinked i386 ELF file. */
size_t build_idsz;
gdb_byte *build_id;
};
diff --git a/gdb/testsuite/gdb.base/solib-mismatch.exp b/gdb/testsuite/gdb.base/solib-mismatch.exp
index 7a625ba..4b723e0 100644
--- a/gdb/testsuite/gdb.base/solib-mismatch.exp
+++ b/gdb/testsuite/gdb.base/solib-mismatch.exp
@@ -20,6 +20,11 @@ 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])"
+ return -1
+}

# Test overview:
# generate two shared objects. One that will be used by the process
@@ -127,7 +132,7 @@ if { [solib_matching_test "${binlibfilebase}" "No" \
"test unstripped, .dynamic matching"] != 0 } {
untested "test unstripped, .dynamic matching"
}
-
+
# Keep original so for debugging purposes
file copy -force "${binlibfilegdb}" "${binlibfilegdb}-orig"
set objcopy_program [transform objcopy]
@@ -142,10 +147,10 @@ if { [solib_matching_test "${binlibfilebase}" "No" \
untested "test --only-keep-debug"
}

-# Keep previous so for debugging puroses
+# Keep previous so for debugging puroses
file copy -force "${binlibfilegdb}" "${binlibfilegdb}-orig1"

-# Copy loaded so over the one gdb will find
+# Copy loaded so over the one gdb will find
file copy -force "${binlibfilerun}" "${binlibfilegdb}"

# Now test it does not mis-invalidate matching libraries