[PATCH v2 0/5] elf2dmp: improve Win2022, Win11 and large dumps

[PATCH v2 0/5] elf2dmp: improve Win2022, Win11 and large dumps

[Viktor Prutyanov]

Windows Server 2022 and Windows 11 require more careful kernel PE image
search and handling of PDB than previous Windows versions.
Also, improve support of large ELF dump files, dumps with unaligned
memory ranges and with big number of ranges.

Viktor Prutyanov (5):
  elf2dmp: replace PE export name check with PDB name check
  elf2dmp: introduce physical block alignment
  elf2dmp: introduce merging of physical memory runs
  elf2dmp: use Linux mmap with MAP_NORESERVE when possible
  elf2dmp: rework PDB_STREAM_INDEXES::segments obtaining

 contrib/elf2dmp/addrspace.c |  31 +++++++-
 contrib/elf2dmp/addrspace.h |   1 +
 contrib/elf2dmp/main.c      | 154 ++++++++++++++++++++----------------
 contrib/elf2dmp/pdb.c       |  15 +---
 contrib/elf2dmp/pdb.h       |   2 +-
 contrib/elf2dmp/qemu_elf.c  |  68 +++++++++++++---
 contrib/elf2dmp/qemu_elf.h  |   2 +
 7 files changed, 177 insertions(+), 96 deletions(-)

-- 
2.21.0

[PATCH v2 1/5] elf2dmp: replace PE export name check with PDB name check

[Viktor Prutyanov]

PE export name check introduced in d399d6b179 isn't reliable enough,
because a page with the export directory may be not present for some
reason. On the other hand, elf2dmp retrieves the PDB name in any case.
It can be also used to check that a PE image is the kernel image. So,
check PDB name when searching for Windows kernel image.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2165917

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

diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index 6d4d18501a..bb6744c0cd 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -411,89 +411,64 @@ 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)
+static bool pe_check_pdb_name(uint64_t base, void *start_addr,
+        struct va_space *vs, OMFSignatureRSDS *rsds)
 {
     const char sign_rsds[4] = "RSDS";
     IMAGE_DEBUG_DIRECTORY debug_dir;
-    OMFSignatureRSDS rsds;
-    char *pdb_name;
-    size_t pdb_name_sz;
-    size_t i;
+    char pdb_name[sizeof(PDB_NAME)];
 
     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;
+        return false;
     }
 
     if (debug_dir.Type != IMAGE_DEBUG_TYPE_CODEVIEW) {
-        return 1;
+        eprintf("Debug Directory type is not CodeView\n");
+        return false;
     }
 
     if (va_space_rw(vs,
                 base + debug_dir.AddressOfRawData,
-                &rsds, sizeof(rsds), 0)) {
-        return 1;
+                rsds, sizeof(*rsds), 0)) {
+        eprintf("Failed to resolve OMFSignatureRSDS\n");
+        return false;
     }
 
-    printf("CodeView signature is \'%.4s\'\n", rsds.Signature);
-
-    if (memcmp(&rsds.Signature, sign_rsds, sizeof(sign_rsds))) {
-        return 1;
+    if (memcmp(&rsds->Signature, sign_rsds, sizeof(sign_rsds))) {
+        eprintf("CodeView signature is \'%.4s\', \'%s\' expected\n",
+                rsds->Signature, sign_rsds);
+        return false;
     }
 
-    pdb_name_sz = debug_dir.SizeOfData - sizeof(rsds);
-    pdb_name = malloc(pdb_name_sz);
-    if (!pdb_name) {
-        return 1;
+    if (debug_dir.SizeOfData - sizeof(*rsds) != sizeof(PDB_NAME)) {
+        eprintf("PDB name size doesn't match\n");
+        return false;
     }
 
     if (va_space_rw(vs, base + debug_dir.AddressOfRawData +
-                offsetof(OMFSignatureRSDS, name), pdb_name, pdb_name_sz, 0)) {
-        free(pdb_name);
-        return 1;
+                offsetof(OMFSignatureRSDS, name), pdb_name, sizeof(PDB_NAME),
+                0)) {
+        eprintf("Failed to resolve PDB name\n");
+        return false;
     }
 
     printf("PDB name is \'%s\', \'%s\' expected\n", pdb_name, PDB_NAME);
 
-    if (strcmp(pdb_name, PDB_NAME)) {
-        eprintf("Unexpected PDB name, it seems the kernel isn't found\n");
-        free(pdb_name);
-        return 1;
-    }
-
-    free(pdb_name);
+    return !strcmp(pdb_name, PDB_NAME);
+}
 
-    sprintf(hash, "%.08x%.04x%.04x%.02x%.02x", rsds.guid.a, rsds.guid.b,
-            rsds.guid.c, rsds.guid.d[0], rsds.guid.d[1]);
+static void pe_get_pdb_symstore_hash(OMFSignatureRSDS *rsds, char *hash)
+{
+    sprintf(hash, "%.08x%.04x%.04x%.02x%.02x", rsds->guid.a, rsds->guid.b,
+            rsds->guid.c, rsds->guid.d[0], rsds->guid.d[1]);
     hash += 20;
-    for (i = 0; i < 6; i++, hash += 2) {
-        sprintf(hash, "%.02x", rsds.guid.e[i]);
+    for (unsigned int i = 0; i < 6; i++, hash += 2) {
+        sprintf(hash, "%.02x", rsds->guid.e[i]);
     }
 
-    sprintf(hash, "%.01x", rsds.age);
-
-    return 0;
+    sprintf(hash, "%.01x", rsds->age);
 }
 
 int main(int argc, char *argv[])
@@ -515,6 +490,7 @@ int main(int argc, char *argv[])
     KDDEBUGGER_DATA64 *kdbg;
     uint64_t KdVersionBlock;
     bool kernel_found = false;
+    OMFSignatureRSDS rsds;
 
     if (argc != 3) {
         eprintf("usage:\n\t%s elf_file dmp_file\n", argv[0]);
@@ -562,7 +538,8 @@ int main(int argc, char *argv[])
         }
 
         if (*(uint16_t *)nt_start_addr == 0x5a4d) { /* MZ */
-            if (pe_check_export_name(KernBase, nt_start_addr, &vs)) {
+            printf("Checking candidate KernBase = 0x%016"PRIx64"\n", KernBase);
+            if (pe_check_pdb_name(KernBase, nt_start_addr, &vs, &rsds)) {
                 kernel_found = true;
                 break;
             }
@@ -578,11 +555,7 @@ int main(int argc, char *argv[])
     printf("KernBase = 0x%016"PRIx64", signature is \'%.2s\'\n", KernBase,
             (char *)nt_start_addr);
 
-    if (pe_get_pdb_symstore_hash(KernBase, nt_start_addr, pdb_hash, &vs)) {
-        eprintf("Failed to get PDB symbol store hash\n");
-        err = 1;
-        goto out_ps;
-    }
+    pe_get_pdb_symstore_hash(&rsds, pdb_hash);
 
     sprintf(pdb_url, "%s%s/%s/%s", SYM_URL_BASE, PDB_NAME, pdb_hash, PDB_NAME);
     printf("PDB URL is %s\n", pdb_url);
-- 
2.21.0

[PATCH v2 2/5] elf2dmp: introduce physical block alignment

[Viktor Prutyanov]

Physical memory ranges may not be aligned to page size in QEMU ELF, but
DMP can only contain page-aligned runs. So, align them.

Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
---
 contrib/elf2dmp/addrspace.c | 31 +++++++++++++++++++++++++++++--
 contrib/elf2dmp/addrspace.h |  1 +
 contrib/elf2dmp/main.c      |  5 +++--
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/contrib/elf2dmp/addrspace.c b/contrib/elf2dmp/addrspace.c
index 0b04cba00e..64b5d680ad 100644
--- a/contrib/elf2dmp/addrspace.c
+++ b/contrib/elf2dmp/addrspace.c
@@ -14,7 +14,7 @@ static struct pa_block *pa_space_find_block(struct pa_space *ps, uint64_t pa)
 
     for (i = 0; i < ps->block_nr; i++) {
         if (ps->block[i].paddr <= pa &&
-                pa <= ps->block[i].paddr + ps->block[i].size) {
+                pa < ps->block[i].paddr + ps->block[i].size) {
             return ps->block + i;
         }
     }
@@ -33,6 +33,30 @@ static uint8_t *pa_space_resolve(struct pa_space *ps, uint64_t pa)
     return block->addr + (pa - block->paddr);
 }
 
+static void pa_block_align(struct pa_block *b)
+{
+    uint64_t low_align = ((b->paddr - 1) | ELF2DMP_PAGE_MASK) + 1 - b->paddr;
+    uint64_t high_align = (b->paddr + b->size) & ELF2DMP_PAGE_MASK;
+
+    if (low_align == 0 && high_align == 0) {
+        return;
+    }
+
+    if (low_align + high_align < b->size) {
+        printf("Block 0x%"PRIx64"+:0x%"PRIx64" will be aligned to "
+                "0x%"PRIx64"+:0x%"PRIx64"\n", b->paddr, b->size,
+                b->paddr + low_align, b->size - low_align - high_align);
+        b->size -= low_align + high_align;
+    } else {
+        printf("Block 0x%"PRIx64"+:0x%"PRIx64" is too small to align\n",
+                b->paddr, b->size);
+        b->size = 0;
+    }
+
+    b->addr += low_align;
+    b->paddr += low_align;
+}
+
 int pa_space_create(struct pa_space *ps, QEMU_Elf *qemu_elf)
 {
     Elf64_Half phdr_nr = elf_getphdrnum(qemu_elf->map);
@@ -60,10 +84,13 @@ int pa_space_create(struct pa_space *ps, QEMU_Elf *qemu_elf)
                 .paddr = phdr[i].p_paddr,
                 .size = phdr[i].p_filesz,
             };
-            block_i++;
+            pa_block_align(&ps->block[block_i]);
+            block_i = ps->block[block_i].size ? (block_i + 1) : block_i;
         }
     }
 
+    ps->block_nr = block_i;
+
     return 0;
 }
 
diff --git a/contrib/elf2dmp/addrspace.h b/contrib/elf2dmp/addrspace.h
index 00b44c1218..039c70c5b0 100644
--- a/contrib/elf2dmp/addrspace.h
+++ b/contrib/elf2dmp/addrspace.h
@@ -12,6 +12,7 @@
 
 #define ELF2DMP_PAGE_BITS 12
 #define ELF2DMP_PAGE_SIZE (1ULL << ELF2DMP_PAGE_BITS)
+#define ELF2DMP_PAGE_MASK (ELF2DMP_PAGE_SIZE - 1)
 #define ELF2DMP_PFN_MASK (~(ELF2DMP_PAGE_SIZE - 1))
 
 #define INVALID_PA  UINT64_MAX
diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index bb6744c0cd..b7e3930164 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -400,9 +400,10 @@ static int write_dump(struct pa_space *ps,
     for (i = 0; i < ps->block_nr; i++) {
         struct pa_block *b = &ps->block[i];
 
-        printf("Writing block #%zu/%zu to file...\n", i, ps->block_nr);
+        printf("Writing block #%zu/%zu of %"PRIu64" bytes to file...\n", i,
+                ps->block_nr, b->size);
         if (fwrite(b->addr, b->size, 1, dmp_file) != 1) {
-            eprintf("Failed to write dump header\n");
+            eprintf("Failed to write block\n");
             fclose(dmp_file);
             return 1;
         }
-- 
2.21.0

[PATCH v2 3/5] elf2dmp: introduce merging of physical memory runs

[Viktor Prutyanov]

DMP supports 42 physical memory runs at most. So, merge adjacent
physical memory ranges from QEMU ELF when possible to minimize total
number of runs.

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

diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index b7e3930164..b4683575fd 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -20,6 +20,7 @@
 #define PE_NAME     "ntoskrnl.exe"
 
 #define INITIAL_MXCSR   0x1f80
+#define MAX_NUMBER_OF_RUNS  42
 
 typedef struct idt_desc {
     uint16_t offset1;   /* offset bits 0..15 */
@@ -234,6 +235,42 @@ static int fix_dtb(struct va_space *vs, QEMU_Elf *qe)
     return 1;
 }
 
+static void try_merge_runs(struct pa_space *ps,
+        WinDumpPhyMemDesc64 *PhysicalMemoryBlock)
+{
+    unsigned int merge_cnt = 0, run_idx = 0;
+
+    PhysicalMemoryBlock->NumberOfRuns = 0;
+
+    for (size_t idx = 0; idx < ps->block_nr; idx++) {
+        struct pa_block *blk = ps->block + idx;
+        struct pa_block *next = blk + 1;
+
+        PhysicalMemoryBlock->NumberOfPages += blk->size / ELF2DMP_PAGE_SIZE;
+
+        if (idx + 1 != ps->block_nr && blk->paddr + blk->size == next->paddr) {
+            printf("Block #%lu 0x%"PRIx64"+:0x%"PRIx64" and %u previous will be"
+                    " merged\n", idx, blk->paddr, blk->size, merge_cnt);
+            merge_cnt++;
+        } else {
+            struct pa_block *first_merged = blk - merge_cnt;
+
+            printf("Block #%lu 0x%"PRIx64"+:0x%"PRIx64" and %u previous will be"
+                    " merged to 0x%"PRIx64"+:0x%"PRIx64" (run #%u)\n",
+                    idx, blk->paddr, blk->size, merge_cnt, first_merged->paddr,
+                    blk->paddr + blk->size - first_merged->paddr, run_idx);
+            PhysicalMemoryBlock->Run[run_idx] = (WinDumpPhyMemRun64) {
+                .BasePage = first_merged->paddr / ELF2DMP_PAGE_SIZE,
+                .PageCount = (blk->paddr + blk->size - first_merged->paddr) /
+                        ELF2DMP_PAGE_SIZE,
+            };
+            PhysicalMemoryBlock->NumberOfRuns++;
+            run_idx++;
+            merge_cnt = 0;
+        }
+    }
+}
+
 static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
         struct va_space *vs, uint64_t KdDebuggerDataBlock,
         KDDEBUGGER_DATA64 *kdbg, uint64_t KdVersionBlock, int nr_cpus)
@@ -244,7 +281,6 @@ static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
             KUSD_OFFSET_PRODUCT_TYPE);
     DBGKD_GET_VERSION64 kvb;
     WinDumpHeader64 h;
-    size_t i;
 
     QEMU_BUILD_BUG_ON(KUSD_OFFSET_SUITE_MASK >= ELF2DMP_PAGE_SIZE);
     QEMU_BUILD_BUG_ON(KUSD_OFFSET_PRODUCT_TYPE >= ELF2DMP_PAGE_SIZE);
@@ -282,13 +318,17 @@ static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
         .RequiredDumpSpace = sizeof(h),
     };
 
-    for (i = 0; i < ps->block_nr; i++) {
-        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,
-        };
+    if (h.PhysicalMemoryBlock.NumberOfRuns <= MAX_NUMBER_OF_RUNS) {
+        for (size_t idx = 0; idx < ps->block_nr; idx++) {
+            h.PhysicalMemoryBlock.NumberOfPages +=
+                    ps->block[idx].size / ELF2DMP_PAGE_SIZE;
+            h.PhysicalMemoryBlock.Run[idx] = (WinDumpPhyMemRun64) {
+                .BasePage = ps->block[idx].paddr / ELF2DMP_PAGE_SIZE,
+                .PageCount = ps->block[idx].size / ELF2DMP_PAGE_SIZE,
+            };
+        }
+    } else {
+        try_merge_runs(ps, &h.PhysicalMemoryBlock);
     }
 
     h.RequiredDumpSpace +=
-- 
2.21.0

[PATCH v2 4/5] elf2dmp: use Linux mmap with MAP_NORESERVE when possible

[Viktor Prutyanov]

Glib's g_mapped_file_new maps file with PROT_READ|PROT_WRITE and
MAP_PRIVATE. This leads to premature physical memory allocation of dump
file size on Linux hosts and may fail. On Linux, mapping the file with
MAP_NORESERVE limits the allocation by available memory.

Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
---
 contrib/elf2dmp/qemu_elf.c | 68 +++++++++++++++++++++++++++++++-------
 contrib/elf2dmp/qemu_elf.h |  2 ++
 2 files changed, 58 insertions(+), 12 deletions(-)

diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
index ebda60dcb8..de6ad744c6 100644
--- a/contrib/elf2dmp/qemu_elf.c
+++ b/contrib/elf2dmp/qemu_elf.c
@@ -165,10 +165,40 @@ static bool check_ehdr(QEMU_Elf *qe)
     return true;
 }
 
-int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
+static int QEMU_Elf_map(QEMU_Elf *qe, const char *filename)
 {
+#ifdef CONFIG_LINUX
+    struct stat st;
+    int fd;
+
+    printf("Using Linux mmap\n");
+
+    fd = open(filename, O_RDONLY, 0);
+    if (fd == -1) {
+        eprintf("Failed to open ELF dump file \'%s\'\n", filename);
+        return 1;
+    }
+
+    if (fstat(fd, &st)) {
+        eprintf("Failed to get size of ELF dump file\n");
+        close(fd);
+        return 1;
+    }
+    qe->size = st.st_size;
+
+    qe->map = mmap(NULL, qe->size, PROT_READ | PROT_WRITE,
+            MAP_PRIVATE | MAP_NORESERVE, fd, 0);
+    if (qe->map == MAP_FAILED) {
+        eprintf("Failed to map ELF file\n");
+        close(fd);
+        return 1;
+    }
+
+    close(fd);
+#else
     GError *gerr = NULL;
-    int err = 0;
+
+    printf("Using GLib mmap\n");
 
     qe->gmf = g_mapped_file_new(filename, TRUE, &gerr);
     if (gerr) {
@@ -179,29 +209,43 @@ int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
 
     qe->map = g_mapped_file_get_contents(qe->gmf);
     qe->size = g_mapped_file_get_length(qe->gmf);
+#endif
+
+    return 0;
+}
+
+static void QEMU_Elf_unmap(QEMU_Elf *qe)
+{
+#ifdef CONFIG_LINUX
+    munmap(qe->map, qe->size);
+#else
+    g_mapped_file_unref(qe->gmf);
+#endif
+}
+
+int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
+{
+    if (QEMU_Elf_map(qe, filename)) {
+        return 1;
+    }
 
     if (!check_ehdr(qe)) {
         eprintf("Input file has the wrong format\n");
-        err = 1;
-        goto out_unmap;
+        QEMU_Elf_unmap(qe);
+        return 1;
     }
 
     if (init_states(qe)) {
         eprintf("Failed to extract QEMU CPU states\n");
-        err = 1;
-        goto out_unmap;
+        QEMU_Elf_unmap(qe);
+        return 1;
     }
 
     return 0;
-
-out_unmap:
-    g_mapped_file_unref(qe->gmf);
-
-    return err;
 }
 
 void QEMU_Elf_exit(QEMU_Elf *qe)
 {
     exit_states(qe);
-    g_mapped_file_unref(qe->gmf);
+    QEMU_Elf_unmap(qe);
 }
diff --git a/contrib/elf2dmp/qemu_elf.h b/contrib/elf2dmp/qemu_elf.h
index b2f0d9cbc9..afa75f10b2 100644
--- a/contrib/elf2dmp/qemu_elf.h
+++ b/contrib/elf2dmp/qemu_elf.h
@@ -32,7 +32,9 @@ typedef struct QEMUCPUState {
 int is_system(QEMUCPUState *s);
 
 typedef struct QEMU_Elf {
+#ifndef CONFIG_LINUX
     GMappedFile *gmf;
+#endif
     size_t size;
     void *map;
     QEMUCPUState **state;
-- 
2.21.0

[PATCH v2 5/5] elf2dmp: rework PDB_STREAM_INDEXES::segments obtaining

[Viktor Prutyanov]

PDB for Windows 11 kernel has slightly different structure compared to
previous versions. Since elf2dmp don't use the other fields, copy only
'segments' field from PDB_STREAM_INDEXES.

Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
---
 contrib/elf2dmp/pdb.c | 15 ++++-----------
 contrib/elf2dmp/pdb.h |  2 +-
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
index adcfa7e154..6ca5086f02 100644
--- a/contrib/elf2dmp/pdb.c
+++ b/contrib/elf2dmp/pdb.c
@@ -160,7 +160,7 @@ static void *pdb_ds_read_file(struct pdb_reader* r, uint32_t file_number)
 static int pdb_init_segments(struct pdb_reader *r)
 {
     char *segs;
-    unsigned stream_idx = r->sidx.segments;
+    unsigned stream_idx = r->segments;
 
     segs = pdb_ds_read_file(r, stream_idx);
     if (!segs) {
@@ -177,9 +177,6 @@ static int pdb_init_symbols(struct pdb_reader *r)
 {
     int err = 0;
     PDB_SYMBOLS *symbols;
-    PDB_STREAM_INDEXES *sidx = &r->sidx;
-
-    memset(sidx, -1, sizeof(*sidx));
 
     symbols = pdb_ds_read_file(r, 3);
     if (!symbols) {
@@ -188,15 +185,11 @@ static int pdb_init_symbols(struct pdb_reader *r)
 
     r->symbols = symbols;
 
-    if (symbols->stream_index_size != sizeof(PDB_STREAM_INDEXES)) {
-        err = 1;
-        goto out_symbols;
-    }
-
-    memcpy(sidx, (const char *)symbols + sizeof(PDB_SYMBOLS) +
+    r->segments = *(uint16_t *)((const char *)symbols + sizeof(PDB_SYMBOLS) +
             symbols->module_size + symbols->offset_size +
             symbols->hash_size + symbols->srcmodule_size +
-            symbols->pdbimport_size + symbols->unknown2_size, sizeof(*sidx));
+            symbols->pdbimport_size + symbols->unknown2_size +
+            offsetof(PDB_STREAM_INDEXES, segments));
 
     /* Read global symbol table */
     r->modimage = pdb_ds_read_file(r, symbols->gsym_file);
diff --git a/contrib/elf2dmp/pdb.h b/contrib/elf2dmp/pdb.h
index 4ea8925ee8..2a50da56ac 100644
--- a/contrib/elf2dmp/pdb.h
+++ b/contrib/elf2dmp/pdb.h
@@ -227,7 +227,7 @@ struct pdb_reader {
     } ds;
     uint32_t file_used[1024];
     PDB_SYMBOLS *symbols;
-    PDB_STREAM_INDEXES sidx;
+    uint16_t segments;
     uint8_t *modimage;
     char *segs;
     size_t segs_size;
-- 
2.21.0

Re: [PATCH v2 0/5] elf2dmp: improve Win2022, Win11 and large dumps

[Akihiko Odaki]

On 2023/09/16 2:01, Viktor Prutyanov wrote:
> Windows Server 2022 and Windows 11 require more careful kernel PE image
> search and handling of PDB than previous Windows versions.
> Also, improve support of large ELF dump files, dumps with unaligned
> memory ranges and with big number of ranges.
> 
> Viktor Prutyanov (5):
>    elf2dmp: replace PE export name check with PDB name check
>    elf2dmp: introduce physical block alignment
>    elf2dmp: introduce merging of physical memory runs
>    elf2dmp: use Linux mmap with MAP_NORESERVE when possible
>    elf2dmp: rework PDB_STREAM_INDEXES::segments obtaining
> 
>   contrib/elf2dmp/addrspace.c |  31 +++++++-
>   contrib/elf2dmp/addrspace.h |   1 +
>   contrib/elf2dmp/main.c      | 154 ++++++++++++++++++++----------------
>   contrib/elf2dmp/pdb.c       |  15 +---
>   contrib/elf2dmp/pdb.h       |   2 +-
>   contrib/elf2dmp/qemu_elf.c  |  68 +++++++++++++---
>   contrib/elf2dmp/qemu_elf.h  |   2 +
>   7 files changed, 177 insertions(+), 96 deletions(-)
> 

For the whole series:
Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v2 0/5] elf2dmp: improve Win2022, Win11 and large dumps

[Peter Maydell]

On Fri, 15 Sept 2023 at 18:13, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> On 2023/09/16 2:01, Viktor Prutyanov wrote:
> > Windows Server 2022 and Windows 11 require more careful kernel PE image
> > search and handling of PDB than previous Windows versions.
> > Also, improve support of large ELF dump files, dumps with unaligned
> > memory ranges and with big number of ranges.
> >
> > Viktor Prutyanov (5):
> >    elf2dmp: replace PE export name check with PDB name check
> >    elf2dmp: introduce physical block alignment
> >    elf2dmp: introduce merging of physical memory runs
> >    elf2dmp: use Linux mmap with MAP_NORESERVE when possible
> >    elf2dmp: rework PDB_STREAM_INDEXES::segments obtaining
> >
> >   contrib/elf2dmp/addrspace.c |  31 +++++++-
> >   contrib/elf2dmp/addrspace.h |   1 +
> >   contrib/elf2dmp/main.c      | 154 ++++++++++++++++++++----------------
> >   contrib/elf2dmp/pdb.c       |  15 +---
> >   contrib/elf2dmp/pdb.h       |   2 +-
> >   contrib/elf2dmp/qemu_elf.c  |  68 +++++++++++++---
> >   contrib/elf2dmp/qemu_elf.h  |   2 +
> >   7 files changed, 177 insertions(+), 96 deletions(-)
> >
>
> For the whole series:
> Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Thanks for reviewing this. I'll take the patches via the
arm tree, unless anybody would prefer them to take a different
route upstream.

-- PMM

Re: [PATCH v2 3/5] elf2dmp: introduce merging of physical memory runs

[Peter Maydell]

On Fri, 15 Sept 2023 at 18:02, Viktor Prutyanov <viktor@daynix.com> wrote:
>
> DMP supports 42 physical memory runs at most. So, merge adjacent
> physical memory ranges from QEMU ELF when possible to minimize total
> number of runs.
>
> Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
> ---
>  contrib/elf2dmp/main.c | 56 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 48 insertions(+), 8 deletions(-)
>
> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
> index b7e3930164..b4683575fd 100644
> --- a/contrib/elf2dmp/main.c
> +++ b/contrib/elf2dmp/main.c
> @@ -20,6 +20,7 @@
>  #define PE_NAME     "ntoskrnl.exe"
>
>  #define INITIAL_MXCSR   0x1f80
> +#define MAX_NUMBER_OF_RUNS  42
>
>  typedef struct idt_desc {
>      uint16_t offset1;   /* offset bits 0..15 */
> @@ -234,6 +235,42 @@ static int fix_dtb(struct va_space *vs, QEMU_Elf *qe)
>      return 1;
>  }
>
> +static void try_merge_runs(struct pa_space *ps,
> +        WinDumpPhyMemDesc64 *PhysicalMemoryBlock)
> +{
> +    unsigned int merge_cnt = 0, run_idx = 0;
> +
> +    PhysicalMemoryBlock->NumberOfRuns = 0;
> +
> +    for (size_t idx = 0; idx < ps->block_nr; idx++) {
> +        struct pa_block *blk = ps->block + idx;
> +        struct pa_block *next = blk + 1;
> +
> +        PhysicalMemoryBlock->NumberOfPages += blk->size / ELF2DMP_PAGE_SIZE;
> +
> +        if (idx + 1 != ps->block_nr && blk->paddr + blk->size == next->paddr) {
> +            printf("Block #%lu 0x%"PRIx64"+:0x%"PRIx64" and %u previous will be"
> +                    " merged\n", idx, blk->paddr, blk->size, merge_cnt);
> +            merge_cnt++;
> +        } else {
> +            struct pa_block *first_merged = blk - merge_cnt;
> +
> +            printf("Block #%lu 0x%"PRIx64"+:0x%"PRIx64" and %u previous will be"
> +                    " merged to 0x%"PRIx64"+:0x%"PRIx64" (run #%u)\n",
> +                    idx, blk->paddr, blk->size, merge_cnt, first_merged->paddr,
> +                    blk->paddr + blk->size - first_merged->paddr, run_idx);
> +            PhysicalMemoryBlock->Run[run_idx] = (WinDumpPhyMemRun64) {
> +                .BasePage = first_merged->paddr / ELF2DMP_PAGE_SIZE,
> +                .PageCount = (blk->paddr + blk->size - first_merged->paddr) /
> +                        ELF2DMP_PAGE_SIZE,
> +            };

Hi; this fails to build on 32-bit hosts because in both these printf()
statements the format string uses "%lu" to print a size_t. This
doesn't work if size_t is not a 'long'. The right format operator is "%zu".

I have squashed in the relevant change to this patch in target-arm.next.

thanks
-- PMM

Re: [PATCH v2 5/5] elf2dmp: rework PDB_STREAM_INDEXES::segments obtaining

[Peter Maydell]

On Fri, 15 Sept 2023 at 18:02, Viktor Prutyanov <viktor@daynix.com> wrote:
>
> PDB for Windows 11 kernel has slightly different structure compared to
> previous versions. Since elf2dmp don't use the other fields, copy only
> 'segments' field from PDB_STREAM_INDEXES.
>
> Signed-off-by: Viktor Prutyanov <viktor@daynix.com>

Hi; this patch has triggered Coverity to report an issue with
the code:

> ---
>  contrib/elf2dmp/pdb.c | 15 ++++-----------
>  contrib/elf2dmp/pdb.h |  2 +-
>  2 files changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
> index adcfa7e154..6ca5086f02 100644
> --- a/contrib/elf2dmp/pdb.c
> +++ b/contrib/elf2dmp/pdb.c
> @@ -160,7 +160,7 @@ static void *pdb_ds_read_file(struct pdb_reader* r, uint32_t file_number)
>  static int pdb_init_segments(struct pdb_reader *r)
>  {
>      char *segs;
> -    unsigned stream_idx = r->sidx.segments;
> +    unsigned stream_idx = r->segments;
>
>      segs = pdb_ds_read_file(r, stream_idx);
>      if (!segs) {

Here we set stream_idx from r->segments, and later in this
function we're going to call pdb_get_file_size(r, stream_idx),
which uses stream_idx as an index int o the toc->file_size[] array...

> @@ -177,9 +177,6 @@ static int pdb_init_symbols(struct pdb_reader *r)
>  {
>      int err = 0;
>      PDB_SYMBOLS *symbols;
> -    PDB_STREAM_INDEXES *sidx = &r->sidx;
> -
> -    memset(sidx, -1, sizeof(*sidx));
>
>      symbols = pdb_ds_read_file(r, 3);
>      if (!symbols) {
> @@ -188,15 +185,11 @@ static int pdb_init_symbols(struct pdb_reader *r)
>
>      r->symbols = symbols;
>
> -    if (symbols->stream_index_size != sizeof(PDB_STREAM_INDEXES)) {
> -        err = 1;
> -        goto out_symbols;
> -    }
> -
> -    memcpy(sidx, (const char *)symbols + sizeof(PDB_SYMBOLS) +
> +    r->segments = *(uint16_t *)((const char *)symbols + sizeof(PDB_SYMBOLS) +
>              symbols->module_size + symbols->offset_size +
>              symbols->hash_size + symbols->srcmodule_size +
> -            symbols->pdbimport_size + symbols->unknown2_size, sizeof(*sidx));
> +            symbols->pdbimport_size + symbols->unknown2_size +
> +            offsetof(PDB_STREAM_INDEXES, segments));

...but we initialized r->segments based on data from the file we're reading,
and we never do any kind of bounds checking on it. So we'll crash if the
file is corrupt/malicious.

Presumably there should be some sort of bounds check somewhere.

(This is CID 1521597.)

thanks
-- PMM

Re: [PATCH v2 1/5] elf2dmp: replace PE export name check with PDB name check

[Peter Maydell]

On Fri, 15 Sept 2023 at 18:02, Viktor Prutyanov <viktor@daynix.com> wrote:
>
> PE export name check introduced in d399d6b179 isn't reliable enough,
> because a page with the export directory may be not present for some
> reason. On the other hand, elf2dmp retrieves the PDB name in any case.
> It can be also used to check that a PE image is the kernel image. So,
> check PDB name when searching for Windows kernel image.
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2165917
>
> Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
> ---
>  contrib/elf2dmp/main.c | 93 +++++++++++++++---------------------------
>  1 file changed, 33 insertions(+), 60 deletions(-)

Hi; Coverity points out a bug in this code (CID 1521598):


> -static int pe_get_pdb_symstore_hash(uint64_t base, void *start_addr,
> -        char *hash, struct va_space *vs)
> +static bool pe_check_pdb_name(uint64_t base, void *start_addr,
> +        struct va_space *vs, OMFSignatureRSDS *rsds)
>  {
>      const char sign_rsds[4] = "RSDS";

sign_rsds[] is not NUL-terminated...

>      IMAGE_DEBUG_DIRECTORY debug_dir;
> -    OMFSignatureRSDS rsds;
> -    char *pdb_name;
> -    size_t pdb_name_sz;
> -    size_t i;
> +    char pdb_name[sizeof(PDB_NAME)];
>
>      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;
> +        return false;
>      }
>
>      if (debug_dir.Type != IMAGE_DEBUG_TYPE_CODEVIEW) {
> -        return 1;
> +        eprintf("Debug Directory type is not CodeView\n");
> +        return false;
>      }
>
>      if (va_space_rw(vs,
>                  base + debug_dir.AddressOfRawData,
> -                &rsds, sizeof(rsds), 0)) {
> -        return 1;
> +                rsds, sizeof(*rsds), 0)) {
> +        eprintf("Failed to resolve OMFSignatureRSDS\n");
> +        return false;
>      }
>
> -    printf("CodeView signature is \'%.4s\'\n", rsds.Signature);
> -
> -    if (memcmp(&rsds.Signature, sign_rsds, sizeof(sign_rsds))) {
> -        return 1;
> +    if (memcmp(&rsds->Signature, sign_rsds, sizeof(sign_rsds))) {
> +        eprintf("CodeView signature is \'%.4s\', \'%s\' expected\n",
> +                rsds->Signature, sign_rsds);

...but in this printf() we pass sign_rsds to a "%s" format
specifier, which requires NUL termination.

If you want to print a non-NUL-terminated string you need
to do the same thing we do for rsds->Signature, which is
give it a precision which is the correct length so printf
doesn't read off the end (i.e. "%.4s").

thanks
-- PMM