10:17 a.m.
This small patch series enhances support for VMware vmss dump and core
files by fixing some bugs I ran into.
Version 2 of the series integrates Lianbo's feedback, as in:
- added separate define and comment for 128 byte "padding"
- changed commit message of patch 3 accordingly
- added missing '\n' to warning messages
- fixed whitespace issues
I meanwhile realized that the register dump of patch 2 can also be seen
via 'help -r' but its formating fixes are useful nonetheless.
Please apply!
Thanks,
Mathias
Mathias Krause (3):
vmware: vmss - support huge holes in vmem
vmware: vmss - beautify and extend debug log
Support core files with "unusual" layout
netdump.c | 90 ++++++++++++++++++++++++++++++++++-----------------
netdump.h | 8 +++++
vmware_vmss.c | 17 ++++++++--
3 files changed, 83 insertions(+), 32 deletions(-)
--
2.20.1
10:17 a.m.
New subject: [PATCH v2 1/3] vmware: vmss - support huge holes in vmem
If the hole is big enough, the multiplication by page size will truncate
as it's operating on a uint32_t.
Cast to uint64_t before to avoid that.
Signed-off-by: Mathias Krause <minipli(a)grsecurity.net>
---
vmware_vmss.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/vmware_vmss.c b/vmware_vmss.c
index eed948f46ee3..11b7b72bf503 100644
--- a/vmware_vmss.c
+++ b/vmware_vmss.c
@@ -498,12 +498,14 @@ read_vmware_vmss(int fd, void *bufptr, int cnt, ulong addr,
physaddr_t paddr)
int i;
for (i = 0; i < vmss.regionscount; i++) {
+ uint32_t hole;
+
if (ppn < vmss.regions[i].startppn)
break;
/* skip holes. */
- pos -= ((vmss.regions[i].startppn - vmss.regions[i].startpagenum)
- << VMW_PAGE_SHIFT);
+ hole = vmss.regions[i].startppn - vmss.regions[i].startpagenum;
+ pos -= (uint64_t)hole << VMW_PAGE_SHIFT;
}
}
--
2.20.1
10:17 a.m.
New subject: [PATCH v2 2/3] vmware: vmss - beautify and extend debug log
The parser's debug log is missing a few line breaks as well as some
crucial information, like control register dumps.
Add them for read- and debugability.
Signed-off-by: Mathias Krause <minipli(a)grsecurity.net>
---
vmware_vmss.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/vmware_vmss.c b/vmware_vmss.c
index 11b7b72bf503..2eee36c3daec 100644
--- a/vmware_vmss.c
+++ b/vmware_vmss.c
@@ -175,8 +175,10 @@ vmware_vmss_init(char *filename, FILE *ofp)
}
DEBUG_PARSE_PRINT((ofp, "[%d]", idx[j]));
}
- if (nextgroup)
+ if (nextgroup) {
+ DEBUG_PARSE_PRINT((ofp, "\n"));
break;
+ }
if (IS_BLOCK_TAG(tag)) {
uint64_t nbytes;
@@ -232,16 +234,21 @@ vmware_vmss_init(char *filename, FILE *ofp)
filename, errno, strerror(errno));
break;
}
+ DEBUG_PARSE_PRINT((ofp, "\n"));
vmss.vcpu_regs[cpu] |= REGS_PRESENT_GPREGS;
} else if (strcmp(name, "CR64") == 0 &&
nbytes == VMW_CR64_SIZE &&
idx[0] < vmss.num_vcpus) {
int cpu = idx[0];
+ DEBUG_PARSE_PRINT((ofp, "\t=> "));
if (fread(&vmss.regs64[cpu]->cr[0], VMW_CR64_SIZE, 1, fp) != 1) {
error(INFO, LOGPRX"Failed to read '%s': [Error %d] %s\n",
filename, errno, strerror(errno));
break;
}
+ for (j = 0; j < VMW_CR64_SIZE / 8; j++)
+ DEBUG_PARSE_PRINT((ofp, "%s%016llX", j ? " " : "",
vmss.regs64[cpu]->cr[j]));
+ DEBUG_PARSE_PRINT((ofp, "\n"));
vmss.vcpu_regs[cpu] |= REGS_PRESENT_CRS;
} else if (strcmp(name, "IDTR") == 0 &&
nbytes == VMW_IDTR_SIZE &&
@@ -258,6 +265,7 @@ vmware_vmss_init(char *filename, FILE *ofp)
filename, errno, strerror(errno));
break;
}
+ DEBUG_PARSE_PRINT((ofp, "\n"));
vmss.regs64[cpu]->idtr = idtr;
vmss.vcpu_regs[cpu] |= REGS_PRESENT_IDTR;
} else {
@@ -266,6 +274,7 @@ vmware_vmss_init(char *filename, FILE *ofp)
(ulonglong)(blockpos + nbytes));
break;
}
+ DEBUG_PARSE_PRINT((ofp, "\n"));
}
} else {
union {
--
2.20.1
10:17 a.m.
New subject: [PATCH v2 3/3] Support core files with "unusual" layout
The netdump code not only gets used for netdump/diskdump files, but also
for kdump core files. These can also be generated with the 'vmss2core'
tool that'll produce a slightly different format that isn't as densely
packed as we expect it to be. In fact, the implicit assumption that the
ELF program headers directly follow the ELF header isn't always true for
these files, as they may contain the section headers in-between -- which
is totally conforming in regards to the ELF spec. This "padding" in
combination with the implicit assumption of densely packed headers make
us interpret the section headers as program headers which is obviously
wrong.
Support these kind of core files too by not blindly assuming the program
headers follow the ELF header directly but by looking at the program
header offset in the ELF header and use that instead. Add some guarding
sanity checks to decline operating on obviously malicious or broken core
files.
To not needlessly make things too complicated, allow a "padding space"
of up to 128 bytes only.
Signed-off-by: Mathias Krause <minipli(a)grsecurity.net>
---
netdump.c | 90 +++++++++++++++++++++++++++++++++++++------------------
netdump.h | 8 +++++
2 files changed, 69 insertions(+), 29 deletions(-)
diff --git a/netdump.c b/netdump.c
index 406416af36bf..7bbb914be01d 100644
--- a/netdump.c
+++ b/netdump.c
@@ -132,7 +132,7 @@ is_netdump(char *file, ulong source_query)
}
}
- size = MIN_NETDUMP_ELF_HEADER_SIZE;
+ size = SAFE_NETDUMP_ELF_HEADER_SIZE;
if ((eheader = (char *)malloc(size)) == NULL) {
fprintf(stderr, "cannot malloc minimum ELF header buffer\n");
clean_exit(1);
@@ -219,8 +219,22 @@ is_netdump(char *file, ulong source_query)
source_query))
goto bailout;
- load32 = (Elf32_Phdr *)
- &eheader[sizeof(Elf32_Ehdr)+sizeof(Elf32_Phdr)];
+ if (elf32->e_phoff != sizeof(Elf32_Ehdr)) {
+ if (CRASHDEBUG(1))
+ error(WARNING, "%s: first PHdr not following "
+ "EHdr (PHdr offset = %u)\n", file,
+ elf32->e_phoff);
+ /* it's okay as long as we've read enough data */
+ if (elf32->e_phoff > size - 2 * sizeof(Elf32_Phdr)) {
+ error(WARNING, "%s: PHdr to far into file!\n",
+ file);
+ goto bailout;
+ }
+ }
+
+ /* skip the NOTE program header */
+ load32 = (Elf32_Phdr *)
+ &eheader[elf32->e_phoff+sizeof(Elf32_Phdr)];
if ((load32->p_offset & (MIN_PAGE_SIZE-1)) ||
(load32->p_align == 0))
@@ -291,8 +305,22 @@ is_netdump(char *file, ulong source_query)
source_query))
goto bailout;
- load64 = (Elf64_Phdr *)
- &eheader[sizeof(Elf64_Ehdr)+sizeof(Elf64_Phdr)];
+ if (elf64->e_phoff != sizeof(Elf64_Ehdr)) {
+ if (CRASHDEBUG(1))
+ error(WARNING, "%s: first PHdr not following "
+ "EHdr (PHdr offset = %u)\n", file,
+ elf64->e_phoff);
+ /* it's okay as long as we've read enough data */
+ if (elf64->e_phoff > size - 2 * sizeof(Elf64_Phdr)) {
+ error(WARNING, "%s: PHdr to far into file!\n",
+ file);
+ goto bailout;
+ }
+ }
+
+ /* skip the NOTE program header */
+ load64 = (Elf64_Phdr *)
+ &eheader[elf64->e_phoff+sizeof(Elf64_Phdr)];
if ((load64->p_offset & (MIN_PAGE_SIZE-1)) ||
(load64->p_align == 0))
@@ -352,10 +380,9 @@ is_netdump(char *file, ulong source_query)
fprintf(stderr, "cannot malloc PT_LOAD segment buffers\n");
clean_exit(1);
}
- nd->notes32 = (Elf32_Phdr *)
- &nd->elf_header[sizeof(Elf32_Ehdr)];
- nd->load32 = (Elf32_Phdr *)
- &nd->elf_header[sizeof(Elf32_Ehdr)+sizeof(Elf32_Phdr)];
+ nd->notes32 = (Elf32_Phdr *)
+ &nd->elf_header[nd->elf32->e_phoff];
+ nd->load32 = nd->notes32 + 1;
if (format == NETDUMP_ELF32)
nd->page_size = (uint)nd->load32->p_align;
dump_Elf32_Ehdr(nd->elf32);
@@ -391,10 +418,9 @@ is_netdump(char *file, ulong source_query)
fprintf(stderr, "cannot malloc PT_LOAD segment
buffers\n");
clean_exit(1);
}
- nd->notes64 = (Elf64_Phdr *)
- &nd->elf_header[sizeof(Elf64_Ehdr)];
- nd->load64 = (Elf64_Phdr *)
- &nd->elf_header[sizeof(Elf64_Ehdr)+sizeof(Elf64_Phdr)];
+ nd->notes64 = (Elf64_Phdr *)
+ &nd->elf_header[nd->elf64->e_phoff];
+ nd->load64 = nd->notes64 + 1;
if (format == NETDUMP_ELF64)
nd->page_size = (uint)nd->load64->p_align;
dump_Elf64_Ehdr(nd->elf64);
@@ -469,8 +495,8 @@ resize_elf_header(int fd, char *file, char **eheader_ptr, char
**sect0_ptr,
case NETDUMP_ELF32:
case KDUMP_ELF32:
num_pt_load_segments = elf32->e_phnum - 1;
- header_size = sizeof(Elf32_Ehdr) + sizeof(Elf32_Phdr) +
- (sizeof(Elf32_Phdr) * num_pt_load_segments);
+ header_size = MAX(sizeof(Elf32_Ehdr), elf32->e_phoff) +
+ (sizeof(Elf32_Phdr) * (num_pt_load_segments + 1));
break;
case NETDUMP_ELF64:
@@ -513,8 +539,8 @@ resize_elf_header(int fd, char *file, char **eheader_ptr, char
**sect0_ptr,
} else
num_pt_load_segments = elf64->e_phnum - 1;
- header_size = sizeof(Elf64_Ehdr) + sizeof(Elf64_Phdr) +
- (sizeof(Elf64_Phdr) * num_pt_load_segments);
+ header_size = MAX(sizeof(Elf64_Ehdr), elf64->e_phoff) +
+ (sizeof(Elf64_Phdr) * (num_pt_load_segments + 1));
break;
}
@@ -544,7 +570,7 @@ resize_elf_header(int fd, char *file, char **eheader_ptr, char
**sect0_ptr,
{
case NETDUMP_ELF32:
case KDUMP_ELF32:
- load32 = (Elf32_Phdr *)&eheader[sizeof(Elf32_Ehdr)+sizeof(Elf32_Phdr)];
+ load32 = (Elf32_Phdr *)&eheader[elf32->e_phoff+sizeof(Elf32_Phdr)];
p_offset32 = load32->p_offset;
for (i = 0; i < num_pt_load_segments; i++, load32 += 1) {
if (load32->p_offset &&
@@ -556,7 +582,7 @@ resize_elf_header(int fd, char *file, char **eheader_ptr, char
**sect0_ptr,
case NETDUMP_ELF64:
case KDUMP_ELF64:
- load64 = (Elf64_Phdr *)&eheader[sizeof(Elf64_Ehdr)+sizeof(Elf64_Phdr)];
+ load64 = (Elf64_Phdr *)&eheader[elf64->e_phoff+sizeof(Elf64_Phdr)];
p_offset64 = load64->p_offset;
for (i = 0; i < num_pt_load_segments; i++, load64 += 1) {
if (load64->p_offset &&
@@ -4459,8 +4485,12 @@ proc_kcore_init_32(FILE *fp, int kcore_fd)
close(fd);
elf32 = (Elf32_Ehdr *)&eheader[0];
- notes32 = (Elf32_Phdr *)&eheader[sizeof(Elf32_Ehdr)];
- load32 = (Elf32_Phdr *)&eheader[sizeof(Elf32_Ehdr)+sizeof(Elf32_Phdr)];
+ if (elf32->e_phoff > sizeof(eheader) - 2 * sizeof(Elf32_Phdr)) {
+ error(INFO, "/proc/kcore: ELF program header offset too big!\n");
+ return FALSE;
+ }
+ notes32 = (Elf32_Phdr *)&eheader[elf32->e_phoff];
+ load32 = notes32 + 1;
pkd->segments = elf32->e_phnum - 1;
@@ -4479,9 +4509,8 @@ proc_kcore_init_32(FILE *fp, int kcore_fd)
}
BCOPY(&eheader[0], &pkd->elf_header[0], pkd->header_size);
- pkd->notes32 = (Elf32_Phdr *)&pkd->elf_header[sizeof(Elf32_Ehdr)];
- pkd->load32 = (Elf32_Phdr *)
- &pkd->elf_header[sizeof(Elf32_Ehdr)+sizeof(Elf32_Phdr)];
+ pkd->notes32 = (Elf32_Phdr *)&pkd->elf_header[elf32->e_phoff];
+ pkd->load32 = pkd->notes32 + 1;
pkd->flags |= KCORE_ELF32;
kcore_memory_dump(CRASHDEBUG(1) ? fp : pc->nullfp);
@@ -4529,8 +4558,12 @@ proc_kcore_init_64(FILE *fp, int kcore_fd)
close(fd);
elf64 = (Elf64_Ehdr *)&eheader[0];
- notes64 = (Elf64_Phdr *)&eheader[sizeof(Elf64_Ehdr)];
- load64 = (Elf64_Phdr *)&eheader[sizeof(Elf64_Ehdr)+sizeof(Elf64_Phdr)];
+ if (elf64->e_phoff > sizeof(eheader) - 2 * sizeof(Elf64_Phdr)) {
+ error(INFO, "/proc/kcore: ELF program header offset too big!\n");
+ return FALSE;
+ }
+ notes64 = (Elf64_Phdr *)&eheader[elf64->e_phoff];
+ load64 = notes64 + 1;
pkd->segments = elf64->e_phnum - 1;
@@ -4550,9 +4583,8 @@ proc_kcore_init_64(FILE *fp, int kcore_fd)
}
BCOPY(&eheader[0], &pkd->elf_header[0], pkd->header_size);
- pkd->notes64 = (Elf64_Phdr *)&pkd->elf_header[sizeof(Elf64_Ehdr)];
- pkd->load64 = (Elf64_Phdr *)
- &pkd->elf_header[sizeof(Elf64_Ehdr)+sizeof(Elf64_Phdr)];
+ pkd->notes64 = (Elf64_Phdr *)&pkd->elf_header[elf64->e_phoff];
+ pkd->load64 = pkd->notes64 + 1;
pkd->flags |= KCORE_ELF64;
kcore_memory_dump(CRASHDEBUG(1) ? fp : pc->nullfp);
diff --git a/netdump.h b/netdump.h
index 7fa04f7c3a0f..ffd60b8d5021 100644
--- a/netdump.h
+++ b/netdump.h
@@ -26,6 +26,14 @@
#define MIN_NETDUMP_ELF_HEADER_SIZE \
MAX(MIN_NETDUMP_ELF32_HEADER_SIZE, MIN_NETDUMP_ELF64_HEADER_SIZE)
+#define NETDUMP_ELF_HEADER_SPARE_SIZE 128
+/*
+ * "Safe" size, as in covering the ELF header and the first two program
headers
+ * plus any "padding" in-between, like section headers.
+ */
+#define SAFE_NETDUMP_ELF_HEADER_SIZE \
+ (MIN_NETDUMP_ELF_HEADER_SIZE+NETDUMP_ELF_HEADER_SPARE_SIZE)
+
#define NT_TASKSTRUCT 4
#define NT_DISKDUMP 0x70000001
--
2.20.1
8:12 p.m.
在 2020年08月08日 23:17, Mathias Krause 写道:
This small patch series enhances support for VMware vmss dump and
core
files by fixing some bugs I ran into.
Version 2 of the series integrates Lianbo's feedback, as in:
- added separate define and comment for 128 byte "padding"
- changed commit message of patch 3 accordingly
- added missing '\n' to warning messages
- fixed whitespace issues
I meanwhile realized that the register dump of patch 2 can also be seen
via 'help -r' but its formating fixes are useful nonetheless.
Please apply!
Thank you for the patch series, Mathias.
The v2 series looks good to me.
Acked-by: Lianbo Jiang <lijinag(a)redhat.com>
Thanks.
Lianbo
Thanks,
Mathias
Mathias Krause (3):
vmware: vmss - support huge holes in vmem
vmware: vmss - beautify and extend debug log
Support core files with "unusual" layout
netdump.c | 90 ++++++++++++++++++++++++++++++++++-----------------
netdump.h | 8 +++++
vmware_vmss.c | 17 ++++++++--
3 files changed, 83 insertions(+), 32 deletions(-)
12:32 a.m.
New subject: [PATCH v2 0/3] vmss/core related fixes and enhancements
-----Original Message-----
From: crash-utility-bounces(a)redhat.com
<crash-utility-bounces(a)redhat.com> On Behalf Of Mathias Krause
Sent: Sunday, August 9, 2020 12:18 AM
To: crash-utility(a)redhat.com
Cc: lijiang(a)redhat.com
Subject: [Crash-utility] [PATCH v2 0/3] vmss/core related fixes and enhancements
This small patch series enhances support for VMware vmss dump and core
files by fixing some bugs I ran into.
Version 2 of the series integrates Lianbo's feedback, as in:
- added separate define and comment for 128 byte "padding"
- changed commit message of patch 3 accordingly
- added missing '\n' to warning messages
- fixed whitespace issues
I meanwhile realized that the register dump of patch 2 can also be seen
via 'help -r' but its formating fixes are useful nonetheless.
Please apply!
The v2 series looks good to me too. Applied.
Thanks,
Kazu
Thanks,
Mathias
Mathias Krause (3):
vmware: vmss - support huge holes in vmem
vmware: vmss - beautify and extend debug log
Support core files with "unusual" layout
netdump.c | 90 ++++++++++++++++++++++++++++++++++-----------------
netdump.h | 8 +++++
vmware_vmss.c | 17 ++++++++--
3 files changed, 83 insertions(+), 32 deletions(-)
--
2.20.1
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
1567
days inactive
1571
days old
devel@lists.crash-utility.osci.io
5 comments
3 participants
participants (3)
-
HAGIO KAZUHITO(萩尾 一仁) -
lijiang -
Mathias Krause