[Crash-utility] [PATCH] netdump, diskdump: address the issue caused by no NT_PRSTATUS notes

HATAYAMA Daisuke d.hatayama at jp.fujitsu.com
Fri Oct 26 08:28:38 UTC 2012


Hello Dave,

I have three patches today.

The 1st and 2nd patches address the issue that vmcore has no NT_PRSTATUS
notes.

- The 1st patch makes crash consider the case where the number of
NT_PRSTATUS notes
is 0. Without this patch, crash leads to NULL pointer dereference when
using bt
command for active tasks.

- The 2nd patch makes crash pass boot-time sanity check to nr_cpus
memebr of diskdump
header on x86 and x86_64 if it is 0. On both architectures, we can get cpu
information from memory part. In fact, the value of nr_cpus member is
not used
except for the sanity check.

I saw the situation where vmcore with no NT_PRSTATUS notes is generated,
on Xen HVM,
where the version of kexec not supporting Xen HVM. It determins if
system is Xen Dom0
wrongly and uses interface for Dom0. As a result, vmcore doesn't contain any
NT_PRSTATUS note.

When converting this vmcore using makedumpfile, nr_cpus member of
diskdump header of
the resulted vmcore is 0. So the 2nd patch is needed.

The 3rd patch is differnet from the above two, fixing potencial bug in
panic task
detection code, which I found during investigation for the above two
patches.
But sorry, I didn't test this yet.

I made every patch on top of crash version 6.1.0.

Thanks.
HATAYAMA, Daisuke

-------------- next part --------------
From 357af1f7b26da928d916e7b3c571e51dec58d1c9 Mon Sep 17 00:00:00 2001
From: HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com>
Date: Thu, 25 Oct 2012 11:19:58 +0900
Subject: [PATCH] netdump: fix NULL pointer dereference when no NT_PRSTATUS
 exists

We faced a vmcore with no NT_PRSTATUS. netdump code now assumes that
there is at least one NT_PRSTATUS note. (The assumption is correct
since NT_PRSTATUS is per-cpu note information and normal machine has
always at least one logical processor.) So, crash resulted in NULL
pointer dereference when reading this vmcore.

The vmcore was generated by kdump on Xen HVM. Unfortunately, kexec on
the system had yet to support Xen HVM, it wrongly entered logic for
Xen DOM0. On DOM0, positions of NT_PRSTATUS is available from Crash
note entries in /proc/iomem. But since it was actually Xen HVM, there
is no Crash note entry. So, the resulted vmcore had no NT_PRSTATUS
note.

The cause of this was using kexec that didn't support Xen HVM. But
there might be cases in the future that vmcore with no NT_PRSTATUS is
generated from any reason. Then, it's useful to be analyze the vmcore
with even no NT_PRSTATUS.

Signed-off-by: HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com>
---
 netdump.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/netdump.c b/netdump.c
index c16eaab..20bf2f7 100644
--- a/netdump.c
+++ b/netdump.c
@@ -2348,6 +2348,9 @@ get_netdump_regs_x86_64(struct bt_info *bt, ulong *ripp, ulong *rspp)
 		else
                 	note = (Elf64_Nhdr *)nd->nt_prstatus;
 
+		if (!note)
+			goto no_nt_prstatus_exists;
+
                 len = sizeof(Elf64_Nhdr);
                 len = roundup(len + note->n_namesz, 4);
                 len = roundup(len + note->n_descsz, 4);
@@ -2384,6 +2387,10 @@ get_netdump_regs_x86_64(struct bt_info *bt, ulong *ripp, ulong *rspp)
 	    (bt->flags & BT_DUMPFILE_SEARCH) && DISKDUMP_DUMPFILE() && 
 	    (note = (Elf64_Nhdr *)
 	     diskdump_get_prstatus_percpu(bt->tc->processor))) {
+
+		if (!note)
+			goto no_nt_prstatus_exists;
+
 		user_regs = get_regs_from_note((char *)note, &rip, &rsp);
 
 		if (CRASHDEBUG(1))
@@ -2399,6 +2406,7 @@ get_netdump_regs_x86_64(struct bt_info *bt, ulong *ripp, ulong *rspp)
 		bt->machdep = (void *)user_regs;
 	}
 
+no_nt_prstatus_exists:
         machdep->get_stack_frame(bt, ripp, rspp);
 }
 
@@ -2668,12 +2676,16 @@ get_netdump_regs_ppc(struct bt_info *bt, ulong *eip, ulong *esp)
 		} else
 			note = (Elf32_Nhdr *)nd->nt_prstatus;
 
+		if (!note)
+			goto no_nt_prstatus_exists;
+
 		len = sizeof(Elf32_Nhdr);
 		len = roundup(len + note->n_namesz, 4);
 		bt->machdep = (void *)((char *)note + len + 
 			MEMBER_OFFSET("elf_prstatus", "pr_reg"));
 	}
 
+no_nt_prstatus_exists:
 	machdep->get_stack_frame(bt, eip, esp);
 }
 
@@ -2702,12 +2714,16 @@ get_netdump_regs_ppc64(struct bt_info *bt, ulong *eip, ulong *esp)
 		} else
 			note = (Elf64_Nhdr *)nd->nt_prstatus;
 
+		if (!note)
+			goto no_nt_prstatus_exists;
+
 		len = sizeof(Elf64_Nhdr);
 		len = roundup(len + note->n_namesz, 4);
 		bt->machdep = (void *)((char *)note + len + 
 			MEMBER_OFFSET("elf_prstatus", "pr_reg"));
 	}
 
+no_nt_prstatus_exists:
 	machdep->get_stack_frame(bt, eip, esp);
 }
 
@@ -3085,6 +3101,10 @@ get_x86_regs_from_elf_notes(struct task_context *tc)
 		note = (void *)nd->nt_prstatus_percpu[tc->processor];
 	else
 		note = (void *)nd->nt_prstatus;
+
+	if (!note)
+		goto no_nt_prstatus_exists;
+
 	if (nd->elf32) {
 		note_32 = (Elf32_Nhdr *)note;
 		len = sizeof(Elf32_Nhdr);
@@ -3100,6 +3120,7 @@ get_x86_regs_from_elf_notes(struct task_context *tc)
 	/* NEED TO BE FIXED: Hack to get the proper alignment */
 	pt_regs +=4;
 
+no_nt_prstatus_exists:
 	return pt_regs;
 
 }
@@ -3118,11 +3139,15 @@ get_x86_64_regs_from_elf_notes(struct task_context *tc)
 	else
 		note = (Elf64_Nhdr *)nd->nt_prstatus;
 
+	if (!note)
+		goto no_nt_prstatus_exists;
+
 	len = sizeof(Elf64_Nhdr);
 	len = roundup(len + note->n_namesz, 4);
 	pt_regs = (void *)((char *)note + len +
 			   MEMBER_OFFSET("elf_prstatus", "pr_reg"));
 
+no_nt_prstatus_exists:
 	return pt_regs;
 }
 
@@ -3146,11 +3171,15 @@ get_ppc_regs_from_elf_notes(struct task_context *tc)
 	} else
 		note = (Elf32_Nhdr *)nd->nt_prstatus;
 
+	if (!note)
+		goto no_nt_prstatus_exists;
+
 	len = sizeof(Elf32_Nhdr);
 	len = roundup(len + note->n_namesz, 4);
 	pt_regs = (void *)((char *)note + len +
 			   MEMBER_OFFSET("elf_prstatus", "pr_reg"));
 
+no_nt_prstatus_exists:
 	return pt_regs;
 }
 
@@ -3174,11 +3203,15 @@ get_ppc64_regs_from_elf_notes(struct task_context *tc)
 	} else
 		note = (Elf64_Nhdr *)nd->nt_prstatus;
 
+	if (!note)
+		goto no_nt_prstatus_exists;
+
 	len = sizeof(Elf64_Nhdr);
 	len = roundup(len + note->n_namesz, 4);
 	pt_regs = (void *)((char *)note + len +
 			   MEMBER_OFFSET("elf_prstatus", "pr_reg"));
 
+no_nt_prstatus_exists:
 	return pt_regs;
 }
 
@@ -3225,6 +3258,10 @@ get_arm_regs_from_elf_notes(struct task_context *tc)
 		note = (void *)nd->nt_prstatus_percpu[tc->processor];
 	else
 		note = (void *)nd->nt_prstatus;
+
+	if (!note)
+		goto no_nt_prstatus_exists;
+
 	if (nd->elf32) {
 		note_32 = (Elf32_Nhdr *)note;
 		len = sizeof(Elf32_Nhdr);
@@ -3238,6 +3275,7 @@ get_arm_regs_from_elf_notes(struct task_context *tc)
 	pt_regs = (void *)((char *)note + len +
 			   MEMBER_OFFSET("elf_prstatus", "pr_reg"));
 
+no_nt_prstatus_exists:
 	return pt_regs;
 }
 
-- 
1.7.7.6

-------------- next part --------------
From 75e02b4366b94cd2494f0bcad3f51226352ecf1e Mon Sep 17 00:00:00 2001
From: HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com>
Date: Thu, 25 Oct 2012 19:22:03 +0900
Subject: [PATCH] diskdump: Pass sanity check if nr_cpus member in diskdump
 header has 0

Currently, if diskdump header has 0 in nr_cpus member, crash regards
it as an unsupported file format. But on X86 and X86_64, the number of
cpus can be calculated from memory. Both archs should be passed here.

Signed-off-by: HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com>
---
 diskdump.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/diskdump.c b/diskdump.c
index 39f4652..ac86ee0 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -397,7 +397,8 @@ restart:
                 error(WARNING, "%s: invalid nr_cpus value: %d\n",
                         DISKDUMP_VALID() ? "diskdump" : "compressed kdump",
                         header->nr_cpus);
-		if (!machine_type("S390") && !machine_type("S390X")) {
+		if (!machine_type("S390") && !machine_type("S390X") &&
+		    !machine_type("X86") && !machine_type("X86_64")) {
 			/* s390 can get register information also from memory */
 			if (DISKDUMP_VALID())
 				goto err;
-- 
1.7.7.6

-------------- next part --------------
From 45a787761f011fb59833db0edd428a7110a40eeb Mon Sep 17 00:00:00 2001
From: HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com>
Date: Thu, 25 Oct 2012 12:00:03 +0900
Subject: [PATCH] netdump: fix potencial panic_task detection bug

Move a check for crashing_cpu in an inner position so that
nd->nt_prstatus is not wrongly used when nd->num_prstatus_notes > 1
but crashing_cpu == 1.

Signed-off-by: HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com>
---
 netdump.c |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/netdump.c b/netdump.c
index c16eaab..92d4681 100644
--- a/netdump.c
+++ b/netdump.c
@@ -768,12 +768,13 @@ get_netdump_panic_task(void)
 	}
 
         if (nd->elf32 && (nd->elf32->e_machine == EM_386)) {
-		Elf32_Nhdr *note32;
+		Elf32_Nhdr *note32 = NULL;
 
-                if ((nd->num_prstatus_notes > 1) && (crashing_cpu != -1))
-                        note32 = (Elf32_Nhdr *)
-                                nd->nt_prstatus_percpu[crashing_cpu];
-                else
+                if (nd->num_prstatus_notes > 1) {
+			if (crashing_cpu != -1)
+				note32 = (Elf32_Nhdr *)
+					nd->nt_prstatus_percpu[crashing_cpu];
+                } else
                         note32 = (Elf32_Nhdr *)nd->nt_prstatus;
 
 		if (!note32)
@@ -815,12 +816,13 @@ check_ebp_esp:
                         }
                 }
 	} else if (nd->elf64) {
-		Elf64_Nhdr *note64;
+		Elf64_Nhdr *note64 = NULL;
 
-                if ((nd->num_prstatus_notes > 1) && (crashing_cpu != -1))
-                        note64 = (Elf64_Nhdr *)
-                                nd->nt_prstatus_percpu[crashing_cpu];
-                else
+                if (nd->num_prstatus_notes > 1) {
+			if (crashing_cpu != -1)
+				note64 = (Elf64_Nhdr *)
+					nd->nt_prstatus_percpu[crashing_cpu];
+                } else
                         note64 = (Elf64_Nhdr *)nd->nt_prstatus;
 
 		if (!note64)
-- 
1.7.7.6



More information about the Crash-utility mailing list