Hi Kazu,
Thanks for review. Please see my comments below.
On Oct 6, 2020, at 6:30 PM, HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com> wrote:
Hi Alexey,
thanks for the patch, I've commented a few inline.
-----Original Message-----
> vmware_guestdump is extension to vmware_vmss with ability to debug
> debug.guest and debug.vmem files.
>
> debug.guest.gz and debug.vmem.gz can be obtained using following
> .vmx options from VM running in debug mode:
> monitor.mini-suspend_on_panic = TRUE
> monitor.suspend_on_triplefault = TRUE
>
> guestdump (debug.guest) is simplified version of *.vmss which does
> not contain full VM state, but minimal guest state, such as memory
> layout and CPUs state, needed for debugger. is_vmware_guestdump()
> and vmware_guestdump_init() functions parse guestdump header and
> populate vmss data structure (from vmware_vmss.c). As result, all
> handlers (except mempry_dump) from vmware_vmss.c can be reused.
>
> How to use: $ crash debug.guest vmlinux
> debug.vmem must be present in the same folder as debug.guest.
>
> Signed-off-by: Alexey Makhalov <amakhalov(a)vmware.com>
> ---
> Makefile | 7 +-
> defs.h | 8 ++
> filesys.c | 12 ++-
> main.c | 14 +++
> memory.c | 8 +-
> vmware_guestdump.c | 308 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> vmware_vmss.c | 8 +-
> vmware_vmss.h | 8 ++
> 8 files changed, 359 insertions(+), 14 deletions(-)
> create mode 100644 vmware_guestdump.c
>
> diff --git a/Makefile b/Makefile
> index 7455410..d185719 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -70,7 +70,7 @@ CFILES=main.c tools.c global_data.c memory.c filesys.c help.c
task.c \
> unwind_x86_32_64.c unwind_arm.c \
> xen_hyper.c xen_hyper_command.c xen_hyper_global_data.c \
> xen_hyper_dump_tables.c kvmdump.c qemu.c qemu-load.c sadump.c ipcs.c \
> - ramdump.c vmware_vmss.c \
> + ramdump.c vmware_vmss.c vmware_guestdump.c \
> xen_dom0.c kaslr_helper.c
>
> SOURCE_FILES=${CFILES} ${GENERIC_HFILES} ${MCORE_HFILES} \
> @@ -89,7 +89,7 @@ OBJECT_FILES=main.o tools.o global_data.o memory.o filesys.o help.o
task.o \
> unwind_x86_32_64.o unwind_arm.o \
> xen_hyper.o xen_hyper_command.o xen_hyper_global_data.o \
> xen_hyper_dump_tables.o kvmdump.o qemu.o qemu-load.o sadump.o ipcs.o \
> - ramdump.o vmware_vmss.o \
> + ramdump.o vmware_vmss.o vmware_guestdump.o \
> xen_dom0.o kaslr_helper.o
>
> MEMORY_DRIVER_FILES=memory_driver/Makefile memory_driver/crash.c
memory_driver/README
> @@ -518,6 +518,9 @@ ramdump.o: ${GENERIC_HFILES} ${REDHAT_HFILES} ramdump.c
> vmware_vmss.o: ${GENERIC_HFILES} ${VMWARE_HFILES} vmware_vmss.c
> ${CC} -c ${CRASH_CFLAGS} vmware_vmss.c ${WARNING_OPTIONS} ${WARNING_ERROR}
>
> +vmware_guestdump.o: ${GENERIC_HFILES} ${VMWARE_HFILES} vmware_guestdump.c
> + ${CC} -c ${CRASH_CFLAGS} vmware_guestdump.c ${WARNING_OPTIONS} ${WARNING_ERROR}
> +
> kaslr_helper.o: ${GENERIC_HFILES} kaslr_helper.c
> ${CC} -c ${CRASH_CFLAGS} kaslr_helper.c ${WARNING_OPTIONS} ${WARNING_ERROR}
>
> diff --git a/defs.h b/defs.h
> index c899fe2..f61f864 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -655,6 +655,7 @@ struct new_utsname {
> #define IRQ_DESC_TREE_RADIX (0x40ULL)
> #define IRQ_DESC_TREE_XARRAY (0x80ULL)
> #define KMOD_PAX (0x100ULL)
> +#define VMWARE_VMSS_GUESTDUMP (0x200ULL)
It seems that this flag list is for kt->flags2, not pc->flags2.
(yes it's confusing.. I'd like to add a comment here in another chance)
In
my understanding flags2 is extension to flags (which is full filled)
No actions from me here.
>
> #define XEN() (kt->flags & ARCH_XEN)
> #define OPENVZ() (kt->flags & ARCH_OPENVZ)
> @@ -6679,6 +6680,13 @@ int vmware_vmss_phys_base(ulong *phys_base);
> int vmware_vmss_set_phys_base(ulong);
>
> /*
> + * vmware_guestdump.c
> + */
> +int is_vmware_guestdump(char *filename);
> +int vmware_guestdump_init(char *filename, FILE *ofp);
> +int vmware_guestdump_memory_dump(FILE *);
> +
> +/*
> * kaslr_helper.c
> */
> int calc_kaslr_offset(ulong *, ulong *);
> diff --git a/filesys.c b/filesys.c
> index 2ec2b31..3361b6c 100644
> --- a/filesys.c
> +++ b/filesys.c
> @@ -253,9 +253,15 @@ memory_source_init(void)
> error(FATAL, "%s: initialization failed\n",
> pc->dumpfile);
> } else if (pc->flags & VMWARE_VMSS) {
> - if (!vmware_vmss_init(pc->dumpfile, fp))
> - error(FATAL, "%s: initialization failed\n",
> - pc->dumpfile);
> + if (pc->flags2 & VMWARE_VMSS_GUESTDUMP) {
> + if (!vmware_guestdump_init(pc->dumpfile, fp))
> + error(FATAL, "%s: initialization failed\n",
> + pc->dumpfile);
> + } else {
> + if (!vmware_vmss_init(pc->dumpfile, fp))
> + error(FATAL, "%s: initialization failed\n",
> + pc->dumpfile);
> + }
> }
> }
> }
> diff --git a/main.c b/main.c
> index 7f562e6..388ac46 100644
> --- a/main.c
> +++ b/main.c
> @@ -671,6 +671,18 @@ main(int argc, char **argv)
> pc->readmem = read_vmware_vmss;
> pc->writemem = write_vmware_vmss;
>
> + } else if (is_vmware_guestdump(argv[optind])) {
> + if (pc->flags & MEMORY_SOURCES) {
> + error(INFO,
> + "too many dumpfile
arguments\n");
> + program_usage(SHORT_FORM);
> + }
> + pc->flags |= VMWARE_VMSS;
> + pc->flags2 |= VMWARE_VMSS_GUESTDUMP;
> + pc->dumpfile = argv[optind];
> + pc->readmem = read_vmware_vmss;
> + pc->writemem = write_vmware_vmss;
> +
> } else {
> error(INFO,
> "%s: not a supported file format\n",
> @@ -1486,6 +1498,8 @@ dump_program_context(void)
> fprintf(fp, "%sMEMSRC_LOCAL", others++ ? "|" : "");
> if (pc->flags2 & REDZONE)
> fprintf(fp, "%sREDZONE", others++ ? "|" : "");
> + if (pc->flags2 & VMWARE_VMSS_GUESTDUMP)
> + fprintf(fp, "%sVMWARE_VMSS_GUESTDUMP", others++ ? "|" :
"");
> fprintf(fp, ")\n");
>
> fprintf(fp, " namelist: %s\n", pc->namelist);
> diff --git a/memory.c b/memory.c
> index c951827..0848097 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -17115,8 +17115,12 @@ dumpfile_memory(int cmd)
> retval = kcore_memory_dump(fp);
> else if (pc->flags & SADUMP)
> retval = sadump_memory_dump(fp);
> - else if (pc->flags & VMWARE_VMSS)
> - retval = vmware_vmss_memory_dump(fp);
> + else if (pc->flags & VMWARE_VMSS) {
> + if (pc->flags2 & VMWARE_VMSS_GUESTDUMP)
> + retval = vmware_guestdump_memory_dump(fp);
> + else
> + retval = vmware_vmss_memory_dump(fp);
> + }
> break;
>
> case DUMPFILE_ENVIRONMENT:
> diff --git a/vmware_guestdump.c b/vmware_guestdump.c
> new file mode 100644
> index 0000000..45acb7a
> --- /dev/null
> +++ b/vmware_guestdump.c
> @@ -0,0 +1,308 @@
> +/*
> + * vmware_guestdump.c
> + *
> + * Copyright (c) 2020 VMware, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * Author: Alexey Makhalov <amakhalov(a)vmware.com>
> + */
> +
> +#include "defs.h"
> +#include "vmware_vmss.h"
> +
> +#define LOGPRX "vmw: "
> +
> +#define GUESTDUMP_VERSION 4
> +#define GUESTDUMP_MAGIC1 1
> +#define GUESTDUMP_MAGIC2 0
> +
> +struct guestdumpheader {
> + uint32_t version;
> + uint32_t num_vcpus;
> + uint8_t magic1;
> + uint8_t reserved1;
> + uint32_t cpu_vendor;
> + uint64_t magic2;
> + uint64_t last_addr;
> + uint64_t memsize_in_pages;
> + uint32_t reserved2;
> + uint32_t mem_holes;
> + struct memhole {
> + uint64_t ppn;
> + uint64_t pages;
> + } holes[2];
> +} __attribute__((packed));
> +
> +struct vcpu_state {
> + uint32_t cr0;
> + uint64_t cr2;
> + uint64_t cr3;
> + uint64_t cr4;
> + uint64_t reserved1[10];
> + uint64_t idt_base;
> + uint16_t reserved2[21];
> + struct x86_64_pt_regs {
> + uint64_t r15;
> + uint64_t r14;
> + uint64_t r13;
> + uint64_t r12;
> + uint64_t rbp;
> + uint64_t rbx;
> + uint64_t r11;
> + uint64_t r10;
> + uint64_t r9;
> + uint64_t r8;
> + uint64_t rax;
> + uint64_t rcx;
> + uint64_t rdx;
> + uint64_t rsi;
> + uint64_t rdi;
> + uint64_t orig_rax;
> + uint64_t rip;
> + uint64_t cs;
> + uint64_t eflags;
> + uint64_t rsp;
> + uint64_t ss;
> + } regs64;
> + uint8_t reserved3[65];
> +} __attribute__((packed));
> +
> +
> +/*
> + * vmware_guestdump is an extension to vmware_vmss with ability to debug
> + * debug.guest and debug.vmem files.
> + *
> + * debug.guest.gz and debug.vmem.gz can be obtained using following
> + * .vmx options from VM running in debug mode:
> + * monitor.mini-suspend_on_panic = TRUE
> + * monitor.suspend_on_triplefault = TRUE
> + *
> + * guestdump (debug.guest) is simplified version of *.vmss which does
> + * not contain full VM state, but minimal guest state, such as memory
> + * layout and CPUs state, needed for debugger. is_vmware_guestdump()
> + * and vmware_guestdump_init() functions parse guestdump header and
> + * populate vmss data structure (from vmware_vmss.c). As result, all
> + * handlers (except mempry_dump) from vmware_vmss.c can be reused.
> + *
> + */
> +
> +int
> +is_vmware_guestdump(char *filename)
> +{
> + struct guestdumpheader hdr;
> + FILE *fp;
> + uint64_t filesize, holes_sum = 0;
> + int i;
> +
> + if (strcmp(basename(filename), "debug.guest"))
> + return FALSE;
Just out of curiosity, renaming is not accepted and is it ok?
If it's renamed, how can they notice need to rename back to "debug.guest”.
debug.guest does not have any special header of magic to detect its format.
So filename is one of checks for format validity in addition to atoner header fields
verification.
Will add comments here in v2 patch.
> +
> + if ((fp = fopen(filename, "r")) == NULL) {
> + error(INFO, LOGPRX"Failed to open '%s': [Error %d] %s\n",
> + filename, errno, strerror(errno));
> + return FALSE;
> + }
> +
> + if (fread(&hdr, sizeof(struct guestdumpheader), 1, fp) != 1) {
> + error(INFO, LOGPRX"Failed to read '%s': [Error %d] %s\n",
> + filename, errno, strerror(errno));
> + fclose(fp);
> + return FALSE;
> + }
> +
> + if (fseek(fp, 0L, SEEK_END) == -1) {
> + error(INFO, LOGPRX"Failed to fseek '%s': [Error %d] %s\n",
> + filename, errno, strerror(errno));
> + fclose(fp);
> + return FALSE;
> + }
> + filesize = ftell(fp);
> + fclose(fp);
> +
> + for (i = 0; i < hdr.mem_holes; i++) {
> + /* hole start page */
> + vmss.regions[i].startpagenum = hdr.holes[i].ppn;
> + /* hole end page */
> + vmss.regions[i].startppn = hdr.holes[i].ppn + hdr.holes[i].pages;
> + holes_sum += hdr.holes[i].pages;
> + }
(hmm.. ah ok, hdr.mem_holes is less than 3 as checked below.)
Will add check for
mem_holes value before accessing vmss.regions[i].
> +
> + if (hdr.version != GUESTDUMP_VERSION ||
> + hdr.magic1 != GUESTDUMP_MAGIC1 ||
> + hdr.magic2 != GUESTDUMP_MAGIC2 ||
> + hdr.mem_holes > 2 ||
> + (hdr.last_addr + 1) != ((hdr.memsize_in_pages + holes_sum) <<
VMW_PAGE_SHIFT) ||
> + filesize != sizeof(struct guestdumpheader) +
> + hdr.num_vcpus * (sizeof (struct vcpu_state) + VMW_PAGE_SIZE)) {
> + if (CRASHDEBUG(1))
> + error(INFO, LOGPRX"Unrecognized debug.guest file.\n");
> + return FALSE;
> + }
> +
> + vmss.memsize = hdr.memsize_in_pages << VMW_PAGE_SHIFT;
> + vmss.regionscount = hdr.mem_holes + 1;
> + vmss.memoffset = 0;
> + vmss.num_vcpus = hdr.num_vcpus;
> + return TRUE;
> +}
> +
> +int
> +vmware_guestdump_init(char *filename, FILE *ofp)
> +{
> + FILE *fp = NULL;
> + int i, result = TRUE;
> + char *vmem_filename = NULL;
> + struct vcpu_state vs;
> + char *p;
> +
> + if (!machine_type("X86") && !machine_type("X86_64")) {
> + error(INFO,
> + LOGPRX"Invalid or unsupported host architecture for .vmss file:
%s\n",
> + MACHINE_TYPE);
> + result = FALSE;
> + goto exit;
> + }
> +
> + if ((fp = fopen(filename, "r")) == NULL) {
> + error(INFO, LOGPRX"Failed to open '%s': [Error %d] %s\n",
> + filename, errno, strerror(errno));
> + result = FALSE;
> + goto exit;
> + }
> +
> + if (fseek(fp, sizeof(struct guestdumpheader), SEEK_SET) == -1) {
> + error(INFO, LOGPRX"Failed to fseek '%s': [Error %d] %s\n",
> + filename, errno, strerror(errno));
> + result = FALSE;
> + goto exit;
> + }
> +
> + vmss.vcpu_regs = malloc(vmss.num_vcpus * sizeof(uint32_t));
> + vmss.regs64 = calloc(vmss.num_vcpus, sizeof(void *));
> + if (!vmss.vcpu_regs || !vmss.regs64) {
> + error(INFO, LOGPRX"Failed to allocate memory\n");
> + result = FALSE;
> + goto exit;
> + }
> +
> + for (i = 0; i < vmss.num_vcpus; i++) {
> + if (fread(&vs, sizeof(struct vcpu_state), 1, fp) != 1) {
> + error(INFO, LOGPRX"Failed to read '%s': [Error %d] %s\n",
> + filename, errno, strerror(errno));
> + result = FALSE;
> + goto exit;
> + }
> + vmss.regs64[i] = calloc(1, sizeof(vmssregs64));
> + if (!vmss.regs64[i]) {
> + error(INFO, LOGPRX"Failed to allocate memory\n");
> + result = FALSE;
> + goto exit;
> + }
> + vmss.vcpu_regs[i] = 0;
> +
> + vmss.regs64[i]->rax = vs.regs64.rax;
> + vmss.regs64[i]->rcx = vs.regs64.rcx;
> + vmss.regs64[i]->rdx = vs.regs64.rdx;
> + vmss.regs64[i]->rbx = vs.regs64.rbx;
> + vmss.regs64[i]->rbp = vs.regs64.rbp;
> + vmss.regs64[i]->rsp = vs.regs64.rsp;
> + vmss.regs64[i]->rsi = vs.regs64.rsi;
> + vmss.regs64[i]->rdi = vs.regs64.rdi;
> + vmss.regs64[i]->r8 = vs.regs64.r8;
> + vmss.regs64[i]->r9 = vs.regs64.r9;
> + vmss.regs64[i]->r10 = vs.regs64.r10;
> + vmss.regs64[i]->r11 = vs.regs64.r11;
> + vmss.regs64[i]->r12 = vs.regs64.r12;
> + vmss.regs64[i]->r13 = vs.regs64.r13;
> + vmss.regs64[i]->r14 = vs.regs64.r14;
> + vmss.regs64[i]->r15 = vs.regs64.r15;
> + vmss.regs64[i]->idtr = vs.idt_base;
> + vmss.regs64[i]->cr[0] = vs.cr0;
> + vmss.regs64[i]->cr[2] = vs.cr2;
> + vmss.regs64[i]->cr[3] = vs.cr3;
> + vmss.regs64[i]->cr[4] = vs.cr4;
> + vmss.regs64[i]->rip = vs.regs64.rip;
> + vmss.regs64[i]->rflags = vs.regs64.eflags;
> +
> + vmss.vcpu_regs[i] = REGS_PRESENT_ALL;
> + }
> +
> +
> + fprintf(ofp, LOGPRX"Try to locate the companion vmem file ...\n");
> + vmem_filename = strdup(filename);
> + p = vmem_filename + strlen(vmem_filename) - 5;
> + if (strcmp(p, "guest") != 0) {
> + result = FALSE;
> + goto exit;
> + }
> + strcpy(p, "vmem");
> + if ((vmss.dfp = fopen(vmem_filename, "r")) == NULL) {
> + error(INFO, LOGPRX"%s: %s\n", vmem_filename, strerror(errno));
> + result = FALSE;
> + goto exit;
> + }
> + fseek(vmss.dfp, 0L, SEEK_END);
> + if (vmss.memsize != ftell(vmss.dfp)) {
> + error(INFO, LOGPRX"%s: unexpected size\n", vmem_filename);
> + result = FALSE;
> + goto exit;
> + }
> + fseek(vmss.dfp, 0L, SEEK_SET);
> + fprintf(ofp, LOGPRX"vmem file: %s\n\n", vmem_filename);
> +
> +exit:
> + if (fp)
> + fclose(fp);
> + if (vmem_filename)
> + free(vmem_filename);
> + if (result == FALSE) {
> + if (vmss.dfp)
> + fclose(vmss.dfp);
> + if (vmss.regs64) {
> + for (i = 0; i < vmss.num_vcpus; i++) {
> + if (vmss.regs64[i])
> + free(vmss.regs64[i]);
> + }
> + free(vmss.regs64);
> + }
> + if (vmss.vcpu_regs)
> + free(vmss.vcpu_regs);
> + }
> + return result;
> +}
> +
> +int
> +vmware_guestdump_memory_dump(FILE *ofp)
> +{
> + int result = TRUE;
The following warning is observed.
$ make clean
$ make warn
...
cc -c -g -DX86_64 -DSNAPPY -DLZO -DGDB_7_6 vmware_guestdump.c -Wall -O2
-Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security
vmware_guestdump.c: In function ‘vmware_guestdump_memory_dump’:
vmware_guestdump.c:286:6: warning: unused variable ‘result’ [-Wunused-variable]
int result = TRUE;
Will fix.
Regards,
—Alexey