[Crash-utility] [PATCH] Fix some regression issues for crash gdb-10.1 v4 patch

Tao Liu ltao at redhat.com
Sat Jun 26 03:26:20 UTC 2021


On x86-64, the v4 patch of updating crash gdb-10.1 has some regression
issues.

Issue 1: Some vmcores will segfault the updated crash:

gdb_interface.c:gdb_set_crash_scope:request is not initialized, so
request.fp may be an invalid address, then it will be passed to
gdb-10.1/gdb/symtab.c:gdb_command_funnel_1:set_stream(req->fp), and
segfaults crash.

So I initialize struct request to be {0}, and check if req->fp==0 before
set_stream.

Issue 2: Some vmcores and vmlinux make crash output "not match" message and exit
with 1.

In kernel.c:verify_version, symbol "linux_banner" is used for version
check. In some kernel, linux_banner is in .data (type 'D'), some is in .rdata
(type 'R'). gdb-10.1 patch [1] removed .rdata from "const struct
section_to_type stt", thus type 'R' is lost and will be granded as type 'D' later,
so it cannot enter kernel.c:verify_version:"else if ((sp->type == 'R') ||
(sp->type == 'r')". In v4 patch, it appended (sp->type == 'D')
to this line, it looks type 'R' case is handled so far.

But when we have the real type 'D' linux_banner, it will enter the "else if"
path as well. It is handled by function symbol_value, it is incorrect,
in fact we want it handled in else path by function get_symbol_data.

So I take the original "const struct section_to_type stt" array back and removed
"(sp->type == 'D')".

When using this patch, please apply the gdb-10.1 v4 patch first, then
apply this patch to crash.

[1] https://github.com/bminor/binutils-gdb/commit/49d9fd42acefc1c0ee282b5808874a1074bf1ecd.patch

---
 gdb-10.1.patch  | 36 +++++++++++++++++++++++++++++++++++-
 gdb_interface.c |  2 +-
 kernel.c        |  2 +-
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/gdb-10.1.patch b/gdb-10.1.patch
index 86ad3b4..7f37e02 100644
--- a/gdb-10.1.patch
+++ b/gdb-10.1.patch
@@ -717,7 +717,7 @@ diff -aurp -X diff_exclude gdb-10.1.orig/gdb/symtab.c gdb-10.1/gdb/symtab.c
 +{
 +        struct symbol *sym;
 +
-+	if (req->command != GNU_VERSION && req->command != GNU_USER_PRINT_OPTION) {
++	if (req->command != GNU_VERSION && req->command != GNU_USER_PRINT_OPTION && req->fp) {
 +		(dynamic_cast<stdio_file *>gdb_stdout)->set_stream(req->fp);
 +		(dynamic_cast<stdio_file *>gdb_stderr)->set_stream(req->fp);
 +	}
@@ -1627,3 +1627,37 @@ diff -aurp -X diff_exclude gdb-10.1.orig/readline/readline/util.c gdb-10.1/readl
    _rl_tracefp = 0;
    return r;
  }
+diff -aurp -X diff_exclude gdb-10.1.orig/bfd/syms.c gdb-10.1/bfd/syms.c
+index cb25af1..79d6fdd 100644
+--- gdb-10.1.orig/bfd/syms.c
++++ gdb-10.1/bfd/syms.c
+@@ -570,10 +570,25 @@ struct section_to_type
+    adding entries.  Since it is so short, a linear search is used.  */
+ static const struct section_to_type stt[] =
+ {
+-  {".drectve", 'i'},		/* MSVC's .drective section */
+-  {".edata", 'e'},		/* MSVC's .edata (export) section */
+-  {".idata", 'i'},		/* MSVC's .idata (import) section */
+-  {".pdata", 'p'},		/* MSVC's .pdata (stack unwind) section */
++  {".bss", 'b'},
++  {"code", 't'},		/* MRI .text */
++  {".data", 'd'},
++  {"*DEBUG*", 'N'},
++  {".debug", 'N'},              /* MSVC's .debug (non-standard debug syms) */
++  {".drectve", 'i'},            /* MSVC's .drective section */
++  {".edata", 'e'},              /* MSVC's .edata (export) section */
++  {".fini", 't'},		/* ELF fini section */
++  {".idata", 'i'},              /* MSVC's .idata (import) section */
++  {".init", 't'},		/* ELF init section */
++  {".pdata", 'p'},              /* MSVC's .pdata (stack unwind) section */
++  {".rdata", 'r'},		/* Read only data.  */
++  {".rodata", 'r'},		/* Read only data.  */
++  {".sbss", 's'},		/* Small BSS (uninitialized data).  */
++  {".scommon", 'c'},		/* Small common.  */
++  {".sdata", 'g'},		/* Small initialized data.  */
++  {".text", 't'},
++  {"vars", 'd'},		/* MRI .data */
++  {"zerovars", 'b'},		/* MRI .bss */
+   {0, 0}
+ };
+ 
diff --git a/gdb_interface.c b/gdb_interface.c
index 5e0544b..3c7eb39 100644
--- a/gdb_interface.c
+++ b/gdb_interface.c
@@ -993,7 +993,7 @@ gdb_user_print_option_address(char *name)
 int
 gdb_set_crash_scope(ulong vaddr, char *arg)
 {
-        struct gnu_request request, *req = &request;
+        struct gnu_request request = {0}, *req = &request;
 	char name[BUFSIZE];
 	struct load_module *lm;
 
diff --git a/kernel.c b/kernel.c
index 360dd8c..e123f76 100644
--- a/kernel.c
+++ b/kernel.c
@@ -1056,7 +1056,7 @@ verify_version(void)
 
 	if (!(sp = symbol_search("linux_banner")))
 		error(FATAL, "linux_banner symbol does not exist?\n");
-	else if ((sp->type == 'R') || (sp->type == 'r') || (sp->type == 'D') ||
+	else if ((sp->type == 'R') || (sp->type == 'r') ||
 		 (machine_type("ARM") && sp->type == 'T') ||
 		 (machine_type("ARM64")))
 		linux_banner = symbol_value("linux_banner");
-- 
2.27.0




More information about the Crash-utility mailing list