[PATCH kdumpid 0/3] Prevent segfault on missing disassembler

[PATCH kdumpid 0/3] Prevent segfault on missing disassembler

[Stephen Brennan]

Hi Petr,

I didn't know quite where to send patches for kdumpid, so I elected to
send direct to you, and CC the linux-debuggers list. Hope that's ok!

I encountered a segmentation fault of kdumpid (master branch):

#0  0x0000000000000000 in ?? ()
#1  0x0000000000406aa9 in disas_at (dd=0x7fffffffde70, info=0x7fffffffdc80, pc=0) at ppc64.c:112
#2  0x0000000000406d42 in looks_like_kcode_ppc64 (dd=0x7fffffffde70, addr=0) at ppc64.c:174
#3  0x0000000000405616 in explore_kernel (dd=0x7fffffffde70, fn=0x405880 <explore_utsname>) at util.c:269
#4  0x0000000000405d3f in explore_raw_data (dd=0x7fffffffde70) at util.c:465
#5  0x0000000000404c98 in main (argc=2, argv=0x7fffffffe168) at main.c:248

The print_insn function pointer was NULL and calling it resulted in the
segfault. So I've included a patch to check the return value of
disassembler() and avoid calling print_insn in those cases.

I also added some fixes to build issues I encountered - hopefully to
help you avoid autotools for even longer :P

Thanks,
Stephen

Stephen Brennan (3):
  cdefs.sh: require bash
  Use -lz unconditionally
  Gracefully handle missing dissasembler function

 Makefile | 2 +-
 cdefs.sh | 2 +-
 libs.sh  | 3 ---
 ppc.c    | 2 ++
 ppc64.c  | 2 ++
 s390.c   | 2 ++
 x86.c    | 2 ++
 7 files changed, 10 insertions(+), 5 deletions(-)

-- 
2.39.2

[PATCH kdumpid 1/3] cdefs.sh: require bash

[Stephen Brennan]

The "&>" stderr redirection feature is a feature of bash, not sh. Use
the correct shebang to avoid the error:

make: *** No rule to make target 'dis_asm.o'.  Stop.

Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
---
 cdefs.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cdefs.sh b/cdefs.sh
index c2d3de6..a056556 100755
--- a/cdefs.sh
+++ b/cdefs.sh
@@ -1,4 +1,4 @@
-#! /bin/sh
+#!/bin/bash
 tmpdir=$(mktemp -d)
 trap 'rm -rf "$tmpdir"' EXIT
 cd "$tmpdir"
-- 
2.39.2

[PATCH kdumpid 2/3] Use -lz unconditionally

[Stephen Brennan]

The file util.c uses inflate* functions, so there's no point in testing
for libz: we need it regardless. If the test in libs.sh fails, then -lz
will not be appended to the link, and linking will fail. So remove the
test and add -lz to the Makefile regardless.

Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
---
 Makefile | 2 +-
 libs.sh  | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index b8e88c0..eebcafb 100644
--- a/Makefile
+++ b/Makefile
@@ -11,7 +11,7 @@ MANDIR=$(PREFIX)/man
 endif
 
 CUSTOM_CFLAGS= -ggdb -Wall -I/home/petr/.local/include
-LIBS += -L/home/petr/.local/lib64 -lkdumpfile -laddrxlat $(shell ./libs.sh)
+LIBS += -L/home/petr/.local/lib64 -lkdumpfile -laddrxlat -lz $(shell ./libs.sh)
 
 LD=ld
 
diff --git a/libs.sh b/libs.sh
index cd491d6..6cc40bd 100755
--- a/libs.sh
+++ b/libs.sh
@@ -28,9 +28,6 @@ if nm -u disas.o | grep -q '^ *U \(htab_create\|splay_tree_new\)$' ; then
     LIBS="$LIBS -liberty"
     rebuild
 fi
-if nm -u disas.o | grep -q '^ *U inflate$' ; then
-    LIBS="$LIBS -lz"
-fi
 if nm -u disas.o | grep -q '^ *U ZSTD_' ; then
     LIBS="$LIBS -lzstd"
 fi
-- 
2.39.2

[PATCH kdumpid 3/3] Gracefully handle missing dissasembler function

[Stephen Brennan]

If disassembler() returns NULL then kdumpid is guaranteed to segfault.
Gracefully return a 0 so we can continue on.

Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
---
 ppc.c   | 2 ++
 ppc64.c | 2 ++
 s390.c  | 2 ++
 x86.c   | 2 ++
 4 files changed, 8 insertions(+)

diff --git a/ppc.c b/ppc.c
index 8d460ae..3683381 100644
--- a/ppc.c
+++ b/ppc.c
@@ -155,5 +155,7 @@ looks_like_kcode_ppc(struct dump_desc *dd, uint64_t addr)
 	print_insn = disassembler(bfd_arch_powerpc,
 				  dd->endian != KDUMP_LITTLE_ENDIAN,
 				  bfd_mach_ppc, NULL);
+	if (!print_insn)
+		return 0;
 	return disas_at(dd, &info, 0);
 }
diff --git a/ppc64.c b/ppc64.c
index 67a912f..fbfb728 100644
--- a/ppc64.c
+++ b/ppc64.c
@@ -171,5 +171,7 @@ looks_like_kcode_ppc64(struct dump_desc *dd, uint64_t addr)
 	print_insn = disassembler(bfd_arch_powerpc,
 				  dd->endian != KDUMP_LITTLE_ENDIAN,
 				  bfd_mach_ppc64, NULL);
+	if (!print_insn)
+		return 0;
 	return disas_at(dd, &info, 0);
 }
diff --git a/s390.c b/s390.c
index fdd23f7..68c8e5d 100644
--- a/s390.c
+++ b/s390.c
@@ -159,6 +159,8 @@ looks_like_kcode_s390(struct dump_desc *dd, uint64_t addr)
 	disassemble_init_for_target(&info);
 	print_insn = disassembler(bfd_arch_s390, TRUE,
 				  bfd_mach_s390_64, NULL);
+	if (!print_insn)
+		return 0;
 	ret |= disas_at(dd, &info, 0);
 
 	if (ret > 0 && priv.state.flags & SAM64_SEEN)
diff --git a/x86.c b/x86.c
index 5c72bbb..5e35778 100644
--- a/x86.c
+++ b/x86.c
@@ -289,6 +289,7 @@ looks_like_kcode_x86(struct dump_desc *dd, uint64_t addr)
 	print_insn = disassembler(bfd_arch_i386, FALSE,
 				  bfd_mach_i386_i386, NULL);
 	if ((!dd->arch || strcmp(dd->arch, "x86_64")) &&
+	    print_insn &&
 	    disas_at(dd, &info, 0) > 0) {
 		free(priv);
 		return 1;
@@ -301,6 +302,7 @@ looks_like_kcode_x86(struct dump_desc *dd, uint64_t addr)
 	print_insn = disassembler(bfd_arch_i386, FALSE,
 				  bfd_mach_x86_64, NULL);
 	if ((!dd->arch || strcmp(dd->arch, "i386")) &&
+	    print_insn &&
 	    disas_at(dd, &info, 0) > 0) {
 		free(priv);
 		return 1;
-- 
2.39.2

Re: [PATCH kdumpid 0/3] Prevent segfault on missing disassembler

[Stephen Brennan]

Sorry, I forgot to add one thing!

Stephen Brennan <stephen.s.brennan@oracle.com> writes:
> Hi Petr,
>
> I didn't know quite where to send patches for kdumpid, so I elected to
> send direct to you, and CC the linux-debuggers list. Hope that's ok!
>
> I encountered a segmentation fault of kdumpid (master branch):
>
> #0  0x0000000000000000 in ?? ()
> #1  0x0000000000406aa9 in disas_at (dd=0x7fffffffde70, info=0x7fffffffdc80, pc=0) at ppc64.c:112
> #2  0x0000000000406d42 in looks_like_kcode_ppc64 (dd=0x7fffffffde70, addr=0) at ppc64.c:174
> #3  0x0000000000405616 in explore_kernel (dd=0x7fffffffde70, fn=0x405880 <explore_utsname>) at util.c:269
> #4  0x0000000000405d3f in explore_raw_data (dd=0x7fffffffde70) at util.c:465

As you can see by the fact that the code fell into the need_explore() /
explore_kernel() path... this vmcore was missing much of the important
metadata, since it was generated by qemu without a vmcoreinfo device.

I think I shared before, but I have a small piece of code [1] which can
locate the vmcoreinfo note reasonably quickly by iterating over pages
and simply testing the first bytes against "OSRELEASE=", since the
vmcoreinfo can only occur on a page boundary. If you'd like, I could
send a patch with some sort of "explore_vmcoreinfo()" function to do
this? I know it's not necessarily the most efficient, but I believe that
it would be valuable when all other heuristics fail.

Thanks,
Stephen

[1] https://github.com/brenns10/kernel_stuff/blob/master/vmcoreinfo/dumpphys.c

Re: [PATCH kdumpid 0/3] Prevent segfault on missing disassembler

[Petr Tesařík]

On Wed, 28 Jun 2023 14:03:41 -0700
Stephen Brennan <stephen.s.brennan@oracle.com> wrote:

> Hi Petr,
> 
> I didn't know quite where to send patches for kdumpid, so I elected to
> send direct to you, and CC the linux-debuggers list. Hope that's ok!


Yes, absolutely! Thank you for the fixes, they are much appreciated.

I should move this project to a better place. In my defense I should
say that the code predates GitHub, and there weren't many options back
then. ;-)

Petr T

> I encountered a segmentation fault of kdumpid (master branch):
> 
> #0  0x0000000000000000 in ?? ()
> #1  0x0000000000406aa9 in disas_at (dd=0x7fffffffde70, info=0x7fffffffdc80, pc=0) at ppc64.c:112
> #2  0x0000000000406d42 in looks_like_kcode_ppc64 (dd=0x7fffffffde70, addr=0) at ppc64.c:174
> #3  0x0000000000405616 in explore_kernel (dd=0x7fffffffde70, fn=0x405880 <explore_utsname>) at util.c:269
> #4  0x0000000000405d3f in explore_raw_data (dd=0x7fffffffde70) at util.c:465
> #5  0x0000000000404c98 in main (argc=2, argv=0x7fffffffe168) at main.c:248
> 
> The print_insn function pointer was NULL and calling it resulted in the
> segfault. So I've included a patch to check the return value of
> disassembler() and avoid calling print_insn in those cases.
> 
> I also added some fixes to build issues I encountered - hopefully to
> help you avoid autotools for even longer :P
> 
> Thanks,
> Stephen
> 
> Stephen Brennan (3):
>   cdefs.sh: require bash
>   Use -lz unconditionally
>   Gracefully handle missing dissasembler function
> 
>  Makefile | 2 +-
>  cdefs.sh | 2 +-
>  libs.sh  | 3 ---
>  ppc.c    | 2 ++
>  ppc64.c  | 2 ++
>  s390.c   | 2 ++
>  x86.c    | 2 ++
>  7 files changed, 10 insertions(+), 5 deletions(-)
>