[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Crash-utility] add support for incomplete elf dump file



Hello Wenjian,

First -- please send patches as email attachments.  Even if I cut-and-paste your
patch from the crash-utility archives (which normally works), there are
still no tabs in your patch.

Now, with respect to the patch, it's not clear to me what would happen if you
make no changes to check_dumpfile_size()?

It seems to me that a read error would occur regardless whether you change
the PT_LOAD-related contents or not.  

If no changes are made:

  If a readmem() of a truncated page is attempted, read_netdump() would
  calculate an offset based upon the original PT_LOAD contents, but then
  the subsequent read() would fail, and would return a READ_ERROR.  

If your patch is applied:

  If a readmem() of a truncated page is attempted, read_netdump() would not 
  be able to calculate an offset, and would return a READ_ERROR.  

What's the difference?  Why even bother making the changes?  

I also don't understand what the difference between "truncated" and "incomplete"
is?  Why did you separate the messages into two?

Anyway, I had already created a patch in preparation for the changes to 
makedumpfile for ELF and compressed kdump vmcores.  The patch will apply
to the current github master branch.  Please apply it alone, and tell me
what happens.  

Thanks,
  Dave



----- Original Message -----
> Since the incomplete dump file generated by ENOSPC error can't be analysed
> by crash utility, but sometimes this file may contain important information
> and the panic problem won't be reproduced, then we came up with an idea to
> modify the exist data of the incomplete dump file to make it analysable by
> crash utility.
> However, we found it will be more suitable to check the incomplete data than
> modifying it in make dump file.
> So, we change the p_filesz of PT_LOAD header, zero_fill and phys_end of
> PT_LOAD
> segments, to make crash can analyse incomplete ELF dump file, when incomplete
> flag exists.
> 
> the issue was discussed at
> 	http://lists.infradead.org/pipermail/kexec/2014-October/012669.html
> 
> --- a/netdump.c
> +++ b/netdump.c
> @@ -52,6 +52,7 @@ static char *vmcoreinfo_read_string(const char *);
> 
>   #define MIN_PAGE_SIZE (4096)
> 
> +#define DUMP_ELF_INCOMPLETE 0x1
>   /*
>    * Architectures that have configurable page sizes,
>    * can differ from the host machine's page size.
> @@ -488,6 +489,10 @@ check_dumpfile_size(char *file)
>          if (stat64(file, &stat) < 0)
>                  return;
> 
> +       Elf64_Phdr *load64 = nd->load64;
> +       Elf32_Phdr *load32 = nd->load32;
> +       unsigned int e_flag = (NULL == nd->elf64) ? (nd->elf32)->e_flags :
> (nd->elf64)->e_flags;
> +       int status = e_flag & DUMP_ELF_INCOMPLETE;
>          for (i = 0; i < nd->num_pt_load_segments; i++) {
>                  pls = &nd->pt_load_segments[i];
> 
> @@ -495,7 +500,19 @@ check_dumpfile_size(char *file)
>                          (pls->phys_end - pls->phys_start);
> 
>                  if (segment_end > stat.st_size) {
> -                       error(WARNING, "%s: may be truncated or incomplete\n"
> +                       if (!status){
> +                               error(WARNING, "%s: may be truncated\n"
> +                                       "         PT_LOAD p_offset: %lld\n"
> +                                       "                 p_filesz: %lld\n"
> +                                       "           bytes required: %lld\n"
> +                                       "            dumpfile size:
> %lld\n\n",
> +                                       file, pls->file_offset,
> +                                       pls->phys_end - pls->phys_start,
> +                                       segment_end, stat.st_size);
> +                               return;
> +                       }
> +                       else{
> +                               error(WARNING, "%s: may be incomplete\n"
>                                  "         PT_LOAD p_offset: %lld\n"
>                                  "                 p_filesz: %lld\n"
>                                  "           bytes required: %lld\n"
> @@ -503,8 +520,25 @@ check_dumpfile_size(char *file)
>                                  file, pls->file_offset,
>                                  pls->phys_end - pls->phys_start,
>                                  segment_end, stat.st_size);
> -                       return;
> +                       }
> +                       if (pls->file_offset > stat.st_size){
> +                                pls->file_offset = 0;
> +                                pls->phys_start = 0;
> +                                pls->phys_end = 0;
> +                        }
> +                        else {
> +                               if (NULL == load32)
> +                                       load64->p_filesz = stat.st_size -
> pls->file_offset;
> +                               else
> +                                       load32->p_filesz = stat.st_size -
> pls->file_offset;
> +                               pls->zero_fill = pls->phys_end;
> +                                pls->phys_end = stat.st_size -
> pls->file_offset + pls->phys_start;
> +                       }
>                  }
> +               if (NULL == load32)
> +                       load64++;
> +               else
> +                       load32++;
>          }
>   }
> 
> --
> Crash-utility mailing list
> Crash-utility redhat com
> https://www.redhat.com/mailman/listinfo/crash-utility
> 
--- a/defs.h
+++ b/defs.h
@@ -504,9 +504,11 @@ struct program_context {
 #define VMCOREINFO    (0x400ULL)
 #define ALLOW_FP      (0x800ULL)
 #define REM_PAUSED_F (0x1000ULL)
-#define RAMDUMP	(0x2000ULL)
+#define RAMDUMP	     (0x2000ULL)
 #define REMOTE_PAUSED() (pc->flags2 & REM_PAUSED_F)
-#define OFFLINE_HIDE (0x4000ULL)
+#define OFFLINE_HIDE     (0x4000ULL)
+#define INCOMPLETE_DUMP  (0x8000ULL)
+#define is_incomplete_dump() (pc->flags2 & INCOMPLETE_DUMP)
 	char *cleanup;
 	char *namelist_orig;
 	char *namelist_debug_orig;
--- a/diskdump.c
+++ b/diskdump.c
@@ -712,6 +712,10 @@ restart:
 		dd->sub_header_kdump->size_vmcoreinfo)
 		pc->flags2 |= VMCOREINFO;
 
+	if (KDUMP_CMPRS_VALID() && 
+	    (dd->header->status & DUMP_DH_COMPRESSED_INCOMPLETE))
+		pc->flags2 |= INCOMPLETE_DUMP;
+
 	/* For split dumpfile */
 	if (KDUMP_CMPRS_VALID()) {
 		is_split = ((dd->header->header_version >= 2) &&
@@ -1577,6 +1581,8 @@ __diskdump_memory_dump(FILE *fp)
 			fprintf(fp, "DUMP_DH_COMPRESSED_LZO");
 		if (dh->status & DUMP_DH_COMPRESSED_SNAPPY)
 			fprintf(fp, "DUMP_DH_COMPRESSED_SNAPPY");
+		if (dh->status & DUMP_DH_COMPRESSED_INCOMPLETE)
+			fprintf(fp, "DUMP_DH_COMPRESSED_INCOMPLETE");
 		break;
 	}
 	fprintf(fp, ")\n");
@@ -1859,13 +1865,17 @@ show_split_dumpfiles(void)
 {
 	int i;
 	struct diskdump_data *ddp;
+	struct disk_dump_header *dh;
 
         for (i = 0; i < num_dumpfiles; i++) {
         	ddp = dd_list[i];
-		fprintf(fp, "%s%s %s", 
+		dh = ddp->header;
+		fprintf(fp, "%s%s%s%s", 
 			i ? "              " : "", 
 			ddp->filename, 
-			is_partial_diskdump() ? "[PARTIAL DUMP]" : "");
+			is_partial_diskdump() ? " [PARTIAL DUMP]" : "",
+			dh->status & DUMP_DH_COMPRESSED_INCOMPLETE ? 
+			" [INCOMPLETE]" : "");
 		if ((i+1) < num_dumpfiles)
 			fprintf(fp, "\n");
 	}
--- a/diskdump.h
+++ b/diskdump.h
@@ -84,6 +84,7 @@ struct kdump_sub_header {
 #define DUMP_DH_COMPRESSED_ZLIB    0x1   /* page is compressed with zlib */
 #define DUMP_DH_COMPRESSED_LZO     0x2   /* page is compressed with lzo */
 #define DUMP_DH_COMPRESSED_SNAPPY  0x4   /* page is compressed with snappy */
+#define DUMP_DH_COMPRESSED_INCOMPLETE  0x8   /* dumpfile is incomplete */
 
 /* descriptor of each page for vmcore */
 typedef struct page_desc {
--- a/kernel.c
+++ b/kernel.c
@@ -4790,9 +4790,17 @@ display_sys_stats(void)
 		if (NETDUMP_DUMPFILE() && is_partial_netdump())
 			fprintf(fp, "  [PARTIAL DUMP]");
 
+		if (KDUMP_DUMPFILE() && is_incomplete_dump())
+			fprintf(fp, "  [INCOMPLETE]");
+
 		if (DISKDUMP_DUMPFILE() && !dumpfile_is_split() &&
-		     is_partial_diskdump())
-			fprintf(fp, "  [PARTIAL DUMP]");
+		    (is_partial_diskdump() || is_incomplete_dump())) {
+			fprintf(fp, " %s%s",
+				is_partial_diskdump() ? 
+				" [PARTIAL DUMP]" : "",
+				is_incomplete_dump() ? 
+				" [INCOMPLETE]" : "");
+		}
 
 		fprintf(fp, "\n");
 
--- a/main.c
+++ b/main.c
@@ -694,7 +694,14 @@ main_loop(void)
 		    "Kernel data has been erased from this dumpfile.  This may "
 		    "cause\n         the crash session to fail entirely, may "
                     "cause commands to fail,\n         or may result in "
-		    "unpredictable runtime behavior.\n",
+		    "unpredictable\n         runtime behavior.\n",
+			pc->dumpfile);
+
+	if (pc->flags2 & INCOMPLETE_DUMP)
+		error(WARNING, "\n%s:\n         "
+		    "This dumpfile is incomplete.  This may cause the crash session"
+		    "\n         to fail entirely, may cause commands to fail, or may"
+		    " result in\n         unpredictable runtime behavior.\n",
 			pc->dumpfile);
 
         if (!(pc->flags & GDB_INIT)) {
@@ -1406,6 +1413,8 @@ dump_program_context(void)
 		fprintf(fp, "%sRAMDUMP", others++ ? "|" : "");
 	if (pc->flags2 & OFFLINE_HIDE)
 		fprintf(fp, "%sOFFLINE_HIDE", others++ ? "|" : "");
+	if (pc->flags2 & INCOMPLETE_DUMP)
+		fprintf(fp, "%sINCOMPLETE_DUMP", others++ ? "|" : "");
 	fprintf(fp, ")\n");
 
 	fprintf(fp, "         namelist: %s\n", pc->namelist);
--- a/netdump.c
+++ b/netdump.c
@@ -1288,6 +1288,9 @@ dump_Elf32_Ehdr(Elf32_Ehdr *elf)
         netdump_print("                e_phoff: %lx\n", elf->e_phoff);
         netdump_print("                e_shoff: %lx\n", elf->e_shoff);
         netdump_print("                e_flags: %lx\n", elf->e_flags);
+	if ((elf->e_flags & DUMP_ELF_INCOMPLETE) && 
+	    (DUMPFILE_FORMAT(nd->flags) == KDUMP_ELF32))
+		pc->flags2 |= INCOMPLETE_DUMP;
         netdump_print("               e_ehsize: %x\n", elf->e_ehsize);
         netdump_print("            e_phentsize: %x\n", elf->e_phentsize);
         netdump_print("                e_phnum: %x\n", elf->e_phnum);
@@ -1447,6 +1450,9 @@ dump_Elf64_Ehdr(Elf64_Ehdr *elf)
         netdump_print("                e_phoff: %lx\n", elf->e_phoff);
         netdump_print("                e_shoff: %lx\n", elf->e_shoff);
         netdump_print("                e_flags: %lx\n", elf->e_flags);
+	if ((elf->e_flags & DUMP_ELF_INCOMPLETE) && 
+	    (DUMPFILE_FORMAT(nd->flags) == KDUMP_ELF64))
+		pc->flags2 |= INCOMPLETE_DUMP;
         netdump_print("               e_ehsize: %x\n", elf->e_ehsize);
         netdump_print("            e_phentsize: %x\n", elf->e_phentsize);
         netdump_print("                e_phnum: %x\n", elf->e_phnum);
--- a/netdump.h
+++ b/netdump.h
@@ -80,6 +80,8 @@ struct vmcore_data {
 	ulonglong backup_offset;
 };
 
+#define DUMP_ELF_INCOMPLETE  0x1   /* dumpfile is incomplete */
+
 /*
  *  ELF note types for Xen dom0/hypervisor kdumps.
  *  The comments below are from xen/include/public/elfnote.h.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]