[PATCH v1 0/3] contrib/elf2dmp: Windows Server 2022 support

[PATCH v1 0/3] contrib/elf2dmp: Windows Server 2022 support

[Viktor Prutyanov]

Hi,

For now, elf2dmp is unable to convert ELF-dump to DMP-dump made of
Windows Server 2022 guest. This patch series fixes it.

v1: improve code-style fix

Viktor Prutyanov (3):
  contrib/elf2dmp: fix code style
  contrib/elf2dmp: move PE dir search to pe_get_data_dir_entry
  contrib/elf2dmp: add PE name check and Windows Server 2022 support

 contrib/elf2dmp/addrspace.c |   1 +
 contrib/elf2dmp/main.c      | 103 +++++++++++++++++++++-----------
 contrib/elf2dmp/pe.h        | 115 ++++++++++++++++++++----------------
 3 files changed, 135 insertions(+), 84 deletions(-)

-- 
2.35.1

[PATCH v1 1/3] contrib/elf2dmp: fix code style

[Viktor Prutyanov]

Originally elf2dmp were added with some code style issues,
especially in pe.h header, and some were introduced by
2d0fc797faaa73fbc1d30f5f9e90407bf3dd93f0. Fix them now.

Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
---
 contrib/elf2dmp/addrspace.c |   1 +
 contrib/elf2dmp/main.c      |   9 ++--
 contrib/elf2dmp/pe.h        | 100 ++++++++++++++++++------------------
 3 files changed, 57 insertions(+), 53 deletions(-)

diff --git a/contrib/elf2dmp/addrspace.c b/contrib/elf2dmp/addrspace.c
index 53ded17061..0b04cba00e 100644
--- a/contrib/elf2dmp/addrspace.c
+++ b/contrib/elf2dmp/addrspace.c
@@ -11,6 +11,7 @@
 static struct pa_block *pa_space_find_block(struct pa_space *ps, uint64_t pa)
 {
     size_t i;
+
     for (i = 0; i < ps->block_nr; i++) {
         if (ps->block[i].paddr <= pa &&
                 pa <= ps->block[i].paddr + ps->block[i].size) {
diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index d77b8f98f7..9224764239 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -282,14 +282,16 @@ static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
     };
 
     for (i = 0; i < ps->block_nr; i++) {
-        h.PhysicalMemoryBlock.NumberOfPages += ps->block[i].size / ELF2DMP_PAGE_SIZE;
+        h.PhysicalMemoryBlock.NumberOfPages +=
+                ps->block[i].size / ELF2DMP_PAGE_SIZE;
         h.PhysicalMemoryBlock.Run[i] = (WinDumpPhyMemRun64) {
             .BasePage = ps->block[i].paddr / ELF2DMP_PAGE_SIZE,
             .PageCount = ps->block[i].size / ELF2DMP_PAGE_SIZE,
         };
     }
 
-    h.RequiredDumpSpace += h.PhysicalMemoryBlock.NumberOfPages << ELF2DMP_PAGE_BITS;
+    h.RequiredDumpSpace +=
+            h.PhysicalMemoryBlock.NumberOfPages << ELF2DMP_PAGE_BITS;
 
     *hdr = h;
 
@@ -299,7 +301,8 @@ static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
 static int fill_context(KDDEBUGGER_DATA64 *kdbg,
         struct va_space *vs, QEMU_Elf *qe)
 {
-        int i;
+    int i;
+
     for (i = 0; i < qe->state_nr; i++) {
         uint64_t Prcb;
         uint64_t Context;
diff --git a/contrib/elf2dmp/pe.h b/contrib/elf2dmp/pe.h
index c2a4a6ba7c..807d006364 100644
--- a/contrib/elf2dmp/pe.h
+++ b/contrib/elf2dmp/pe.h
@@ -33,70 +33,70 @@ typedef struct IMAGE_DOS_HEADER {
 } __attribute__ ((packed)) IMAGE_DOS_HEADER;
 
 typedef struct IMAGE_FILE_HEADER {
-  uint16_t  Machine;
-  uint16_t  NumberOfSections;
-  uint32_t  TimeDateStamp;
-  uint32_t  PointerToSymbolTable;
-  uint32_t  NumberOfSymbols;
-  uint16_t  SizeOfOptionalHeader;
-  uint16_t  Characteristics;
+    uint16_t  Machine;
+    uint16_t  NumberOfSections;
+    uint32_t  TimeDateStamp;
+    uint32_t  PointerToSymbolTable;
+    uint32_t  NumberOfSymbols;
+    uint16_t  SizeOfOptionalHeader;
+    uint16_t  Characteristics;
 } __attribute__ ((packed)) IMAGE_FILE_HEADER;
 
 typedef struct IMAGE_DATA_DIRECTORY {
-  uint32_t VirtualAddress;
-  uint32_t Size;
+    uint32_t VirtualAddress;
+    uint32_t Size;
 } __attribute__ ((packed)) IMAGE_DATA_DIRECTORY;
 
 #define IMAGE_NUMBEROF_DIRECTORY_ENTRIES 16
 
 typedef struct IMAGE_OPTIONAL_HEADER64 {
-  uint16_t  Magic; /* 0x20b */
-  uint8_t   MajorLinkerVersion;
-  uint8_t   MinorLinkerVersion;
-  uint32_t  SizeOfCode;
-  uint32_t  SizeOfInitializedData;
-  uint32_t  SizeOfUninitializedData;
-  uint32_t  AddressOfEntryPoint;
-  uint32_t  BaseOfCode;
-  uint64_t  ImageBase;
-  uint32_t  SectionAlignment;
-  uint32_t  FileAlignment;
-  uint16_t  MajorOperatingSystemVersion;
-  uint16_t  MinorOperatingSystemVersion;
-  uint16_t  MajorImageVersion;
-  uint16_t  MinorImageVersion;
-  uint16_t  MajorSubsystemVersion;
-  uint16_t  MinorSubsystemVersion;
-  uint32_t  Win32VersionValue;
-  uint32_t  SizeOfImage;
-  uint32_t  SizeOfHeaders;
-  uint32_t  CheckSum;
-  uint16_t  Subsystem;
-  uint16_t  DllCharacteristics;
-  uint64_t  SizeOfStackReserve;
-  uint64_t  SizeOfStackCommit;
-  uint64_t  SizeOfHeapReserve;
-  uint64_t  SizeOfHeapCommit;
-  uint32_t  LoaderFlags;
-  uint32_t  NumberOfRvaAndSizes;
-  IMAGE_DATA_DIRECTORY DataDirectory[IMAGE_NUMBEROF_DIRECTORY_ENTRIES];
+    uint16_t  Magic; /* 0x20b */
+    uint8_t   MajorLinkerVersion;
+    uint8_t   MinorLinkerVersion;
+    uint32_t  SizeOfCode;
+    uint32_t  SizeOfInitializedData;
+    uint32_t  SizeOfUninitializedData;
+    uint32_t  AddressOfEntryPoint;
+    uint32_t  BaseOfCode;
+    uint64_t  ImageBase;
+    uint32_t  SectionAlignment;
+    uint32_t  FileAlignment;
+    uint16_t  MajorOperatingSystemVersion;
+    uint16_t  MinorOperatingSystemVersion;
+    uint16_t  MajorImageVersion;
+    uint16_t  MinorImageVersion;
+    uint16_t  MajorSubsystemVersion;
+    uint16_t  MinorSubsystemVersion;
+    uint32_t  Win32VersionValue;
+    uint32_t  SizeOfImage;
+    uint32_t  SizeOfHeaders;
+    uint32_t  CheckSum;
+    uint16_t  Subsystem;
+    uint16_t  DllCharacteristics;
+    uint64_t  SizeOfStackReserve;
+    uint64_t  SizeOfStackCommit;
+    uint64_t  SizeOfHeapReserve;
+    uint64_t  SizeOfHeapCommit;
+    uint32_t  LoaderFlags;
+    uint32_t  NumberOfRvaAndSizes;
+    IMAGE_DATA_DIRECTORY DataDirectory[IMAGE_NUMBEROF_DIRECTORY_ENTRIES];
 } __attribute__ ((packed)) IMAGE_OPTIONAL_HEADER64;
 
 typedef struct IMAGE_NT_HEADERS64 {
-  uint32_t Signature;
-  IMAGE_FILE_HEADER FileHeader;
-  IMAGE_OPTIONAL_HEADER64 OptionalHeader;
+    uint32_t Signature;
+    IMAGE_FILE_HEADER FileHeader;
+    IMAGE_OPTIONAL_HEADER64 OptionalHeader;
 } __attribute__ ((packed)) IMAGE_NT_HEADERS64;
 
 typedef struct IMAGE_DEBUG_DIRECTORY {
-  uint32_t Characteristics;
-  uint32_t TimeDateStamp;
-  uint16_t MajorVersion;
-  uint16_t MinorVersion;
-  uint32_t Type;
-  uint32_t SizeOfData;
-  uint32_t AddressOfRawData;
-  uint32_t PointerToRawData;
+    uint32_t Characteristics;
+    uint32_t TimeDateStamp;
+    uint16_t MajorVersion;
+    uint16_t MinorVersion;
+    uint32_t Type;
+    uint32_t SizeOfData;
+    uint32_t AddressOfRawData;
+    uint32_t PointerToRawData;
 } __attribute__ ((packed)) IMAGE_DEBUG_DIRECTORY;
 
 #define IMAGE_DEBUG_TYPE_CODEVIEW   2
-- 
2.35.1

[PATCH v1 2/3] contrib/elf2dmp: move PE dir search to pe_get_data_dir_entry

[Viktor Prutyanov]

Move out PE directory search functionality to be reused not only
for Debug Directory processing but for arbitrary PE directory.

Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
---
 contrib/elf2dmp/main.c | 66 +++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index 9224764239..f3052b3c64 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -333,6 +333,40 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg,
     return 0;
 }
 
+static int pe_get_data_dir_entry(uint64_t base, void *start_addr, int idx,
+        void *entry, size_t size, struct va_space *vs)
+{
+    const char e_magic[2] = "MZ";
+    const char Signature[4] = "PE\0\0";
+    IMAGE_DOS_HEADER *dos_hdr = start_addr;
+    IMAGE_NT_HEADERS64 nt_hdrs;
+    IMAGE_FILE_HEADER *file_hdr = &nt_hdrs.FileHeader;
+    IMAGE_OPTIONAL_HEADER64 *opt_hdr = &nt_hdrs.OptionalHeader;
+    IMAGE_DATA_DIRECTORY *data_dir = nt_hdrs.OptionalHeader.DataDirectory;
+
+    if (memcmp(&dos_hdr->e_magic, e_magic, sizeof(e_magic))) {
+        return 1;
+    }
+
+    if (va_space_rw(vs, base + dos_hdr->e_lfanew,
+                &nt_hdrs, sizeof(nt_hdrs), 0)) {
+        return 1;
+    }
+
+    if (memcmp(&nt_hdrs.Signature, Signature, sizeof(Signature)) ||
+            file_hdr->Machine != 0x8664 || opt_hdr->Magic != 0x020b) {
+        return 1;
+    }
+
+    if (va_space_rw(vs,
+                base + data_dir[idx].VirtualAddress,
+                entry, size, 0)) {
+        return 1;
+    }
+
+    return 0;
+}
+
 static int write_dump(struct pa_space *ps,
         WinDumpHeader64 *hdr, const char *name)
 {
@@ -369,42 +403,16 @@ static int write_dump(struct pa_space *ps,
 static int pe_get_pdb_symstore_hash(uint64_t base, void *start_addr,
         char *hash, struct va_space *vs)
 {
-    const char e_magic[2] = "MZ";
-    const char Signature[4] = "PE\0\0";
     const char sign_rsds[4] = "RSDS";
-    IMAGE_DOS_HEADER *dos_hdr = start_addr;
-    IMAGE_NT_HEADERS64 nt_hdrs;
-    IMAGE_FILE_HEADER *file_hdr = &nt_hdrs.FileHeader;
-    IMAGE_OPTIONAL_HEADER64 *opt_hdr = &nt_hdrs.OptionalHeader;
-    IMAGE_DATA_DIRECTORY *data_dir = nt_hdrs.OptionalHeader.DataDirectory;
     IMAGE_DEBUG_DIRECTORY debug_dir;
     OMFSignatureRSDS rsds;
     char *pdb_name;
     size_t pdb_name_sz;
     size_t i;
 
-    QEMU_BUILD_BUG_ON(sizeof(*dos_hdr) >= ELF2DMP_PAGE_SIZE);
-
-    if (memcmp(&dos_hdr->e_magic, e_magic, sizeof(e_magic))) {
-        return 1;
-    }
-
-    if (va_space_rw(vs, base + dos_hdr->e_lfanew,
-                &nt_hdrs, sizeof(nt_hdrs), 0)) {
-        return 1;
-    }
-
-    if (memcmp(&nt_hdrs.Signature, Signature, sizeof(Signature)) ||
-            file_hdr->Machine != 0x8664 || opt_hdr->Magic != 0x020b) {
-        return 1;
-    }
-
-    printf("Debug Directory RVA = 0x%08"PRIx32"\n",
-            (uint32_t)data_dir[IMAGE_FILE_DEBUG_DIRECTORY].VirtualAddress);
-
-    if (va_space_rw(vs,
-                base + data_dir[IMAGE_FILE_DEBUG_DIRECTORY].VirtualAddress,
-                &debug_dir, sizeof(debug_dir), 0)) {
+    if (pe_get_data_dir_entry(base, start_addr, IMAGE_FILE_DEBUG_DIRECTORY,
+                &debug_dir, sizeof(debug_dir), vs)) {
+        eprintf("Failed to get Debug Directory\n");
         return 1;
     }
 
-- 
2.35.1

[PATCH v1 3/3] contrib/elf2dmp: add PE name check and Windows Server 2022 support

[Viktor Prutyanov]

Since its inception elf2dmp has checked MZ signatures within an
address space above IDT[0] interrupt vector and took first PE image
found as Windows Kernel.
But in Windows Server 2022 memory dump this address space range is
full of invalid PE fragments and the tool must check that PE image
is 'ntoskrnl.exe' actually.
So, introduce additional validation by checking image name from
Export Directory against 'ntoskrnl.exe'.

Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
Tested-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 contrib/elf2dmp/main.c | 28 ++++++++++++++++++++++++++--
 contrib/elf2dmp/pe.h   | 15 +++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index f3052b3c64..f7de82a03e 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -17,6 +17,7 @@
 
 #define SYM_URL_BASE    "https://msdl.microsoft.com/download/symbols/"
 #define PDB_NAME    "ntkrnlmp.pdb"
+#define PE_NAME     "ntoskrnl.exe"
 
 #define INITIAL_MXCSR   0x1f80
 
@@ -400,6 +401,25 @@ static int write_dump(struct pa_space *ps,
     return fclose(dmp_file);
 }
 
+static bool pe_check_export_name(uint64_t base, void *start_addr,
+        struct va_space *vs)
+{
+    IMAGE_EXPORT_DIRECTORY export_dir;
+    const char *pe_name;
+
+    if (pe_get_data_dir_entry(base, start_addr, IMAGE_FILE_EXPORT_DIRECTORY,
+                &export_dir, sizeof(export_dir), vs)) {
+        return false;
+    }
+
+    pe_name = va_space_resolve(vs, base + export_dir.Name);
+    if (!pe_name) {
+        return false;
+    }
+
+    return !strcmp(pe_name, PE_NAME);
+}
+
 static int pe_get_pdb_symstore_hash(uint64_t base, void *start_addr,
         char *hash, struct va_space *vs)
 {
@@ -484,6 +504,7 @@ int main(int argc, char *argv[])
     uint64_t KdDebuggerDataBlock;
     KDDEBUGGER_DATA64 *kdbg;
     uint64_t KdVersionBlock;
+    bool kernel_found = false;
 
     if (argc != 3) {
         eprintf("usage:\n\t%s elf_file dmp_file\n", argv[0]);
@@ -531,11 +552,14 @@ int main(int argc, char *argv[])
         }
 
         if (*(uint16_t *)nt_start_addr == 0x5a4d) { /* MZ */
-            break;
+            if (pe_check_export_name(KernBase, nt_start_addr, &vs)) {
+                kernel_found = true;
+                break;
+            }
         }
     }
 
-    if (!nt_start_addr) {
+    if (!kernel_found) {
         eprintf("Failed to find NT kernel image\n");
         err = 1;
         goto out_ps;
diff --git a/contrib/elf2dmp/pe.h b/contrib/elf2dmp/pe.h
index 807d006364..71126af1ac 100644
--- a/contrib/elf2dmp/pe.h
+++ b/contrib/elf2dmp/pe.h
@@ -88,6 +88,20 @@ typedef struct IMAGE_NT_HEADERS64 {
     IMAGE_OPTIONAL_HEADER64 OptionalHeader;
 } __attribute__ ((packed)) IMAGE_NT_HEADERS64;
 
+typedef struct IMAGE_EXPORT_DIRECTORY {
+    uint32_t    Characteristics;
+    uint32_t    TimeDateStamp;
+    uint16_t    MajorVersion;
+    uint16_t    MinorVersion;
+    uint32_t    Name;
+    uint32_t    Base;
+    uint32_t    NumberOfFunctions;
+    uint32_t    NumberOfNames;
+    uint32_t    AddressOfFunctions;
+    uint32_t    AddressOfNames;
+    uint32_t    AddressOfNameOrdinals;
+} __attribute__ ((packed)) IMAGE_EXPORT_DIRECTORY;
+
 typedef struct IMAGE_DEBUG_DIRECTORY {
     uint32_t Characteristics;
     uint32_t TimeDateStamp;
@@ -102,6 +116,7 @@ typedef struct IMAGE_DEBUG_DIRECTORY {
 #define IMAGE_DEBUG_TYPE_CODEVIEW   2
 #endif
 
+#define IMAGE_FILE_EXPORT_DIRECTORY 0
 #define IMAGE_FILE_DEBUG_DIRECTORY  6
 
 typedef struct guid_t {
-- 
2.35.1

Re: [PATCH v1 0/3] contrib/elf2dmp: Windows Server 2022 support

[Viktor Prutyanov]

On 11/30/22 3:03 AM, Viktor Prutyanov wrote:
> Hi,
> 
> For now, elf2dmp is unable to convert ELF-dump to DMP-dump made of
> Windows Server 2022 guest. This patch series fixes it.
> 
> v1: improve code-style fix
> 
> Viktor Prutyanov (3):
>    contrib/elf2dmp: fix code style
>    contrib/elf2dmp: move PE dir search to pe_get_data_dir_entry
>    contrib/elf2dmp: add PE name check and Windows Server 2022 support
> 
>   contrib/elf2dmp/addrspace.c |   1 +
>   contrib/elf2dmp/main.c      | 103 +++++++++++++++++++++-----------
>   contrib/elf2dmp/pe.h        | 115 ++++++++++++++++++++----------------
>   3 files changed, 135 insertions(+), 84 deletions(-)
> 

Hi Paolo,

Could you please add the series to your branch?
I've sent it from my other email. I can add
Signed-off-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>

Thanks,
Viktor Prutyanov

Re: [PATCH v1 0/3] contrib/elf2dmp: Windows Server 2022 support

[Yan Vugenfirer]

[-- Attachment #1: Type: text/plain, Size: 1005 bytes --]

Ping.



On Tue, Jan 10, 2023 at 6:10 PM Viktor Prutyanov <
viktor.prutyanov@phystech.edu> wrote:

> On 11/30/22 3:03 AM, Viktor Prutyanov wrote:
> > Hi,
> >
> > For now, elf2dmp is unable to convert ELF-dump to DMP-dump made of
> > Windows Server 2022 guest. This patch series fixes it.
> >
> > v1: improve code-style fix
> >
> > Viktor Prutyanov (3):
> >    contrib/elf2dmp: fix code style
> >    contrib/elf2dmp: move PE dir search to pe_get_data_dir_entry
> >    contrib/elf2dmp: add PE name check and Windows Server 2022 support
> >
> >   contrib/elf2dmp/addrspace.c |   1 +
> >   contrib/elf2dmp/main.c      | 103 +++++++++++++++++++++-----------
> >   contrib/elf2dmp/pe.h        | 115 ++++++++++++++++++++----------------
> >   3 files changed, 135 insertions(+), 84 deletions(-)
> >
>
> Hi Paolo,
>
> Could you please add the series to your branch?
> I've sent it from my other email. I can add
> Signed-off-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
>
> Thanks,
> Viktor Prutyanov
>
>

[-- Attachment #2: Type: text/html, Size: 1605 bytes --]

Re: [PATCH v1 2/3] contrib/elf2dmp: move PE dir search to pe_get_data_dir_entry

[Annie.li]

Hello Viktor,

See my following comments inline,

On 11/29/2022 7:03 PM, Viktor Prutyanov wrote:
> Move out PE directory search functionality to be reused not only
> for Debug Directory processing but for arbitrary PE directory.
>
> Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
> ---
>   contrib/elf2dmp/main.c | 66 +++++++++++++++++++++++-------------------
>   1 file changed, 37 insertions(+), 29 deletions(-)
>
> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
> index 9224764239..f3052b3c64 100644
> --- a/contrib/elf2dmp/main.c
> +++ b/contrib/elf2dmp/main.c
> @@ -333,6 +333,40 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg,
>       return 0;
>   }
>   
> +static int pe_get_data_dir_entry(uint64_t base, void *start_addr, int idx,
> +        void *entry, size_t size, struct va_space *vs)
> +{
> +    const char e_magic[2] = "MZ";
> +    const char Signature[4] = "PE\0\0";
> +    IMAGE_DOS_HEADER *dos_hdr = start_addr;
> +    IMAGE_NT_HEADERS64 nt_hdrs;
> +    IMAGE_FILE_HEADER *file_hdr = &nt_hdrs.FileHeader;
> +    IMAGE_OPTIONAL_HEADER64 *opt_hdr = &nt_hdrs.OptionalHeader;
> +    IMAGE_DATA_DIRECTORY *data_dir = nt_hdrs.OptionalHeader.DataDirectory;
> +
> +    if (memcmp(&dos_hdr->e_magic, e_magic, sizeof(e_magic))) {
> +        return 1;
> +    }
> +
> +    if (va_space_rw(vs, base + dos_hdr->e_lfanew,
> +                &nt_hdrs, sizeof(nt_hdrs), 0)) {
> +        return 1;
> +    }
> +
> +    if (memcmp(&nt_hdrs.Signature, Signature, sizeof(Signature)) ||
> +            file_hdr->Machine != 0x8664 || opt_hdr->Magic != 0x020b) {
> +        return 1;
> +    }
> +
> +    if (va_space_rw(vs,
> +                base + data_dir[idx].VirtualAddress,
> +                entry, size, 0)) {
> +        return 1;
> +    }
> +
> +    return 0;
> +}
> +
>   static int write_dump(struct pa_space *ps,
>           WinDumpHeader64 *hdr, const char *name)
>   {
> @@ -369,42 +403,16 @@ static int write_dump(struct pa_space *ps,
>   static int pe_get_pdb_symstore_hash(uint64_t base, void *start_addr,
>           char *hash, struct va_space *vs)
>   {
> -    const char e_magic[2] = "MZ";
> -    const char Signature[4] = "PE\0\0";
>       const char sign_rsds[4] = "RSDS";
> -    IMAGE_DOS_HEADER *dos_hdr = start_addr;
> -    IMAGE_NT_HEADERS64 nt_hdrs;
> -    IMAGE_FILE_HEADER *file_hdr = &nt_hdrs.FileHeader;
> -    IMAGE_OPTIONAL_HEADER64 *opt_hdr = &nt_hdrs.OptionalHeader;
> -    IMAGE_DATA_DIRECTORY *data_dir = nt_hdrs.OptionalHeader.DataDirectory;
>       IMAGE_DEBUG_DIRECTORY debug_dir;
>       OMFSignatureRSDS rsds;
>       char *pdb_name;
>       size_t pdb_name_sz;
>       size_t i;
>   
> -    QEMU_BUILD_BUG_ON(sizeof(*dos_hdr) >= ELF2DMP_PAGE_SIZE);

This BUG_ON gets removed due to encapsulating the code into function 
pe_get_data_dir_entry.

Any reason of not keeping this check in pe_get_data_dir_entry?
> -
> -    if (memcmp(&dos_hdr->e_magic, e_magic, sizeof(e_magic))) {
> -        return 1;
> -    }
> -
> -    if (va_space_rw(vs, base + dos_hdr->e_lfanew,
> -                &nt_hdrs, sizeof(nt_hdrs), 0)) {
> -        return 1;
> -    }
> -
> -    if (memcmp(&nt_hdrs.Signature, Signature, sizeof(Signature)) ||
> -            file_hdr->Machine != 0x8664 || opt_hdr->Magic != 0x020b) {
> -        return 1;
> -    }
> -
> -    printf("Debug Directory RVA = 0x%08"PRIx32"\n",
> -            (uint32_t)data_dir[IMAGE_FILE_DEBUG_DIRECTORY].VirtualAddress);

Or add common log for both Debug and PE directory instead of removing it?

Thanks

Annie

> -
> -    if (va_space_rw(vs,
> -                base + data_dir[IMAGE_FILE_DEBUG_DIRECTORY].VirtualAddress,
> -                &debug_dir, sizeof(debug_dir), 0)) {
> +    if (pe_get_data_dir_entry(base, start_addr, IMAGE_FILE_DEBUG_DIRECTORY,
> +                &debug_dir, sizeof(debug_dir), vs)) {
> +        eprintf("Failed to get Debug Directory\n");
>           return 1;
>       }
>   

Re: [PATCH v1 3/3] contrib/elf2dmp: add PE name check and Windows Server 2022 support

[Annie.li]

On 11/29/2022 7:03 PM, Viktor Prutyanov wrote:
> Since its inception elf2dmp has checked MZ signatures within an
> address space above IDT[0] interrupt vector and took first PE image
> found as Windows Kernel.
> But in Windows Server 2022 memory dump this address space range is
> full of invalid PE fragments and the tool must check that PE image
> is 'ntoskrnl.exe' actually.
> So, introduce additional validation by checking image name from
> Export Directory against 'ntoskrnl.exe'.
>
> Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
> Tested-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>   contrib/elf2dmp/main.c | 28 ++++++++++++++++++++++++++--
>   contrib/elf2dmp/pe.h   | 15 +++++++++++++++
>   2 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
> index f3052b3c64..f7de82a03e 100644
> --- a/contrib/elf2dmp/main.c
> +++ b/contrib/elf2dmp/main.c
> @@ -17,6 +17,7 @@
>   
>   #define SYM_URL_BASE    "https://msdl.microsoft.com/download/symbols/"
>   #define PDB_NAME    "ntkrnlmp.pdb"
> +#define PE_NAME     "ntoskrnl.exe"

As what has been clarified earlier in the meeting, this elf2dmp is only for
64bits systems, so the name "ntoskrnl.exe" suffices here. Otherwise, it 
won't work
for 32bits PAE systems.

A question about elf2dmp on ARM platform, has it been validated there?

Thanks

Annie

>   
>   #define INITIAL_MXCSR   0x1f80
>   
> @@ -400,6 +401,25 @@ static int write_dump(struct pa_space *ps,
>       return fclose(dmp_file);
>   }
>   
> +static bool pe_check_export_name(uint64_t base, void *start_addr,
> +        struct va_space *vs)
> +{
> +    IMAGE_EXPORT_DIRECTORY export_dir;
> +    const char *pe_name;
> +
> +    if (pe_get_data_dir_entry(base, start_addr, IMAGE_FILE_EXPORT_DIRECTORY,
> +                &export_dir, sizeof(export_dir), vs)) {
> +        return false;
> +    }
> +
> +    pe_name = va_space_resolve(vs, base + export_dir.Name);
> +    if (!pe_name) {
> +        return false;
> +    }
> +
> +    return !strcmp(pe_name, PE_NAME);
> +}
> +
>   static int pe_get_pdb_symstore_hash(uint64_t base, void *start_addr,
>           char *hash, struct va_space *vs)
>   {
> @@ -484,6 +504,7 @@ int main(int argc, char *argv[])
>       uint64_t KdDebuggerDataBlock;
>       KDDEBUGGER_DATA64 *kdbg;
>       uint64_t KdVersionBlock;
> +    bool kernel_found = false;
>   
>       if (argc != 3) {
>           eprintf("usage:\n\t%s elf_file dmp_file\n", argv[0]);
> @@ -531,11 +552,14 @@ int main(int argc, char *argv[])
>           }
>   
>           if (*(uint16_t *)nt_start_addr == 0x5a4d) { /* MZ */
> -            break;
> +            if (pe_check_export_name(KernBase, nt_start_addr, &vs)) {
> +                kernel_found = true;
> +                break;
> +            }
>           }
>       }
>   
> -    if (!nt_start_addr) {
> +    if (!kernel_found) {
>           eprintf("Failed to find NT kernel image\n");
>           err = 1;
>           goto out_ps;
> diff --git a/contrib/elf2dmp/pe.h b/contrib/elf2dmp/pe.h
> index 807d006364..71126af1ac 100644
> --- a/contrib/elf2dmp/pe.h
> +++ b/contrib/elf2dmp/pe.h
> @@ -88,6 +88,20 @@ typedef struct IMAGE_NT_HEADERS64 {
>       IMAGE_OPTIONAL_HEADER64 OptionalHeader;
>   } __attribute__ ((packed)) IMAGE_NT_HEADERS64;
>   
> +typedef struct IMAGE_EXPORT_DIRECTORY {
> +    uint32_t    Characteristics;
> +    uint32_t    TimeDateStamp;
> +    uint16_t    MajorVersion;
> +    uint16_t    MinorVersion;
> +    uint32_t    Name;
> +    uint32_t    Base;
> +    uint32_t    NumberOfFunctions;
> +    uint32_t    NumberOfNames;
> +    uint32_t    AddressOfFunctions;
> +    uint32_t    AddressOfNames;
> +    uint32_t    AddressOfNameOrdinals;
> +} __attribute__ ((packed)) IMAGE_EXPORT_DIRECTORY;
> +
>   typedef struct IMAGE_DEBUG_DIRECTORY {
>       uint32_t Characteristics;
>       uint32_t TimeDateStamp;
> @@ -102,6 +116,7 @@ typedef struct IMAGE_DEBUG_DIRECTORY {
>   #define IMAGE_DEBUG_TYPE_CODEVIEW   2
>   #endif
>   
> +#define IMAGE_FILE_EXPORT_DIRECTORY 0
>   #define IMAGE_FILE_DEBUG_DIRECTORY  6
>   
>   typedef struct guid_t {

Re: [PATCH v1 3/3] contrib/elf2dmp: add PE name check and Windows Server 2022 support

[Viktor Prutyanov]

On Wed, Feb 22, 2023 at 10:07 PM Annie.li <annie.li@oracle.com> wrote:
>
>
> On 11/29/2022 7:03 PM, Viktor Prutyanov wrote:
> > Since its inception elf2dmp has checked MZ signatures within an
> > address space above IDT[0] interrupt vector and took first PE image
> > found as Windows Kernel.
> > But in Windows Server 2022 memory dump this address space range is
> > full of invalid PE fragments and the tool must check that PE image
> > is 'ntoskrnl.exe' actually.
> > So, introduce additional validation by checking image name from
> > Export Directory against 'ntoskrnl.exe'.
> >
> > Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
> > Tested-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >   contrib/elf2dmp/main.c | 28 ++++++++++++++++++++++++++--
> >   contrib/elf2dmp/pe.h   | 15 +++++++++++++++
> >   2 files changed, 41 insertions(+), 2 deletions(-)
> >
> > diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
> > index f3052b3c64..f7de82a03e 100644
> > --- a/contrib/elf2dmp/main.c
> > +++ b/contrib/elf2dmp/main.c
> > @@ -17,6 +17,7 @@
> >
> >   #define SYM_URL_BASE    "https://msdl.microsoft.com/download/symbols/"
> >   #define PDB_NAME    "ntkrnlmp.pdb"
> > +#define PE_NAME     "ntoskrnl.exe"
>
> As what has been clarified earlier in the meeting, this elf2dmp is only for
> 64bits systems, so the name "ntoskrnl.exe" suffices here. Otherwise, it
> won't work
> for 32bits PAE systems.
>
> A question about elf2dmp on ARM platform, has it been validated there?
>
> Thanks
>
> Annie
>
> >
> >   #define INITIAL_MXCSR   0x1f80
> >
> > @@ -400,6 +401,25 @@ static int write_dump(struct pa_space *ps,
> >       return fclose(dmp_file);
> >   }
> >
> > +static bool pe_check_export_name(uint64_t base, void *start_addr,
> > +        struct va_space *vs)
> > +{
> > +    IMAGE_EXPORT_DIRECTORY export_dir;
> > +    const char *pe_name;
> > +
> > +    if (pe_get_data_dir_entry(base, start_addr, IMAGE_FILE_EXPORT_DIRECTORY,
> > +                &export_dir, sizeof(export_dir), vs)) {
> > +        return false;
> > +    }
> > +
> > +    pe_name = va_space_resolve(vs, base + export_dir.Name);
> > +    if (!pe_name) {
> > +        return false;
> > +    }
> > +
> > +    return !strcmp(pe_name, PE_NAME);
> > +}
> > +
> >   static int pe_get_pdb_symstore_hash(uint64_t base, void *start_addr,
> >           char *hash, struct va_space *vs)
> >   {
> > @@ -484,6 +504,7 @@ int main(int argc, char *argv[])
> >       uint64_t KdDebuggerDataBlock;
> >       KDDEBUGGER_DATA64 *kdbg;
> >       uint64_t KdVersionBlock;
> > +    bool kernel_found = false;
> >
> >       if (argc != 3) {
> >           eprintf("usage:\n\t%s elf_file dmp_file\n", argv[0]);
> > @@ -531,11 +552,14 @@ int main(int argc, char *argv[])
> >           }
> >
> >           if (*(uint16_t *)nt_start_addr == 0x5a4d) { /* MZ */
> > -            break;
> > +            if (pe_check_export_name(KernBase, nt_start_addr, &vs)) {
> > +                kernel_found = true;
> > +                break;
> > +            }
> >           }
> >       }
> >
> > -    if (!nt_start_addr) {
> > +    if (!kernel_found) {
> >           eprintf("Failed to find NT kernel image\n");
> >           err = 1;
> >           goto out_ps;
> > diff --git a/contrib/elf2dmp/pe.h b/contrib/elf2dmp/pe.h
> > index 807d006364..71126af1ac 100644
> > --- a/contrib/elf2dmp/pe.h
> > +++ b/contrib/elf2dmp/pe.h
> > @@ -88,6 +88,20 @@ typedef struct IMAGE_NT_HEADERS64 {
> >       IMAGE_OPTIONAL_HEADER64 OptionalHeader;
> >   } __attribute__ ((packed)) IMAGE_NT_HEADERS64;
> >
> > +typedef struct IMAGE_EXPORT_DIRECTORY {
> > +    uint32_t    Characteristics;
> > +    uint32_t    TimeDateStamp;
> > +    uint16_t    MajorVersion;
> > +    uint16_t    MinorVersion;
> > +    uint32_t    Name;
> > +    uint32_t    Base;
> > +    uint32_t    NumberOfFunctions;
> > +    uint32_t    NumberOfNames;
> > +    uint32_t    AddressOfFunctions;
> > +    uint32_t    AddressOfNames;
> > +    uint32_t    AddressOfNameOrdinals;
> > +} __attribute__ ((packed)) IMAGE_EXPORT_DIRECTORY;
> > +
> >   typedef struct IMAGE_DEBUG_DIRECTORY {
> >       uint32_t Characteristics;
> >       uint32_t TimeDateStamp;
> > @@ -102,6 +116,7 @@ typedef struct IMAGE_DEBUG_DIRECTORY {
> >   #define IMAGE_DEBUG_TYPE_CODEVIEW   2
> >   #endif
> >
> > +#define IMAGE_FILE_EXPORT_DIRECTORY 0
> >   #define IMAGE_FILE_DEBUG_DIRECTORY  6
> >
> >   typedef struct guid_t {

Hi Annie,

Thank you for the review!
At the moment, elf2dmp only addresses the x86_64 platform.

Best regards,
Viktor Prutyanov

Re: [PATCH v1 2/3] contrib/elf2dmp: move PE dir search to pe_get_data_dir_entry

[Viktor Prutyanov]

Hello,

On Wed, Feb 22, 2023 at 10:07 PM Annie.li <annie.li@oracle.com> wrote:
>
> Hello Viktor,
>
> See my following comments inline,
>
> On 11/29/2022 7:03 PM, Viktor Prutyanov wrote:
> > Move out PE directory search functionality to be reused not only
> > for Debug Directory processing but for arbitrary PE directory.
> >
> > Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
> > ---
> >   contrib/elf2dmp/main.c | 66 +++++++++++++++++++++++-------------------
> >   1 file changed, 37 insertions(+), 29 deletions(-)
> >
> > diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
> > index 9224764239..f3052b3c64 100644
> > --- a/contrib/elf2dmp/main.c
> > +++ b/contrib/elf2dmp/main.c
> > @@ -333,6 +333,40 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg,
> >       return 0;
> >   }
> >
> > +static int pe_get_data_dir_entry(uint64_t base, void *start_addr, int idx,
> > +        void *entry, size_t size, struct va_space *vs)
> > +{
> > +    const char e_magic[2] = "MZ";
> > +    const char Signature[4] = "PE\0\0";
> > +    IMAGE_DOS_HEADER *dos_hdr = start_addr;
> > +    IMAGE_NT_HEADERS64 nt_hdrs;
> > +    IMAGE_FILE_HEADER *file_hdr = &nt_hdrs.FileHeader;
> > +    IMAGE_OPTIONAL_HEADER64 *opt_hdr = &nt_hdrs.OptionalHeader;
> > +    IMAGE_DATA_DIRECTORY *data_dir = nt_hdrs.OptionalHeader.DataDirectory;
> > +
> > +    if (memcmp(&dos_hdr->e_magic, e_magic, sizeof(e_magic))) {
> > +        return 1;
> > +    }
> > +
> > +    if (va_space_rw(vs, base + dos_hdr->e_lfanew,
> > +                &nt_hdrs, sizeof(nt_hdrs), 0)) {
> > +        return 1;
> > +    }
> > +
> > +    if (memcmp(&nt_hdrs.Signature, Signature, sizeof(Signature)) ||
> > +            file_hdr->Machine != 0x8664 || opt_hdr->Magic != 0x020b) {
> > +        return 1;
> > +    }
> > +
> > +    if (va_space_rw(vs,
> > +                base + data_dir[idx].VirtualAddress,
> > +                entry, size, 0)) {
> > +        return 1;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >   static int write_dump(struct pa_space *ps,
> >           WinDumpHeader64 *hdr, const char *name)
> >   {
> > @@ -369,42 +403,16 @@ static int write_dump(struct pa_space *ps,
> >   static int pe_get_pdb_symstore_hash(uint64_t base, void *start_addr,
> >           char *hash, struct va_space *vs)
> >   {
> > -    const char e_magic[2] = "MZ";
> > -    const char Signature[4] = "PE\0\0";
> >       const char sign_rsds[4] = "RSDS";
> > -    IMAGE_DOS_HEADER *dos_hdr = start_addr;
> > -    IMAGE_NT_HEADERS64 nt_hdrs;
> > -    IMAGE_FILE_HEADER *file_hdr = &nt_hdrs.FileHeader;
> > -    IMAGE_OPTIONAL_HEADER64 *opt_hdr = &nt_hdrs.OptionalHeader;
> > -    IMAGE_DATA_DIRECTORY *data_dir = nt_hdrs.OptionalHeader.DataDirectory;
> >       IMAGE_DEBUG_DIRECTORY debug_dir;
> >       OMFSignatureRSDS rsds;
> >       char *pdb_name;
> >       size_t pdb_name_sz;
> >       size_t i;
> >
> > -    QEMU_BUILD_BUG_ON(sizeof(*dos_hdr) >= ELF2DMP_PAGE_SIZE);
>
> This BUG_ON gets removed due to encapsulating the code into function
> pe_get_data_dir_entry.
>
> Any reason of not keeping this check in pe_get_data_dir_entry?
> > -
> > -    if (memcmp(&dos_hdr->e_magic, e_magic, sizeof(e_magic))) {
> > -        return 1;
> > -    }
> > -
> > -    if (va_space_rw(vs, base + dos_hdr->e_lfanew,
> > -                &nt_hdrs, sizeof(nt_hdrs), 0)) {
> > -        return 1;
> > -    }
> > -
> > -    if (memcmp(&nt_hdrs.Signature, Signature, sizeof(Signature)) ||
> > -            file_hdr->Machine != 0x8664 || opt_hdr->Magic != 0x020b) {
> > -        return 1;
> > -    }
> > -
> > -    printf("Debug Directory RVA = 0x%08"PRIx32"\n",
> > -            (uint32_t)data_dir[IMAGE_FILE_DEBUG_DIRECTORY].VirtualAddress);
>
> Or add common log for both Debug and PE directory instead of removing it?

Sounds reasonable, I will send a new version.

Best regards,
Viktor Prutyanov

>
> Thanks
>
> Annie
>
> > -
> > -    if (va_space_rw(vs,
> > -                base + data_dir[IMAGE_FILE_DEBUG_DIRECTORY].VirtualAddress,
> > -                &debug_dir, sizeof(debug_dir), 0)) {
> > +    if (pe_get_data_dir_entry(base, start_addr, IMAGE_FILE_DEBUG_DIRECTORY,
> > +                &debug_dir, sizeof(debug_dir), vs)) {
> > +        eprintf("Failed to get Debug Directory\n");
> >           return 1;
> >       }
> >