5:46 a.m.
Currently, crash prints always
crash: vmcore: not a supported file format
if you try to open a dump file which is not supported. However, it can be
misleading if you have a valid ELF core dump, but just use crash for the wrong
architecture. In the case I observed the user had a ELF64 x86 dump file and
assumed it's x86-64. However, it just was a i386 core dump which was ELF64
because kexec was called with --elf64-core-headers which makes sense
if the i386 machine has PAE and possibly more than 4 GiB of physical RAM.
After that patch is applied, an example output is
Looks like a valid ELF dump, but host architecture (X86_64) \
doesn't match dump architecture (IA64).
or if I try to open a PPC64 dump on x86-64:
Looks like a valid ELF dump, but host endianess (LE) \
doesn't match target endianess (BE)
Please review and consider applying.
Signed-off-by: Bernhard Walle <bwalle(a)suse.de>
---
defs.h | 3 ++-
netdump.c | 48 +++++++++++++++++++++++++++++++++++++++++++-----
tools.c | 9 ++++++++-
3 files changed, 53 insertions(+), 7 deletions(-)
--- a/defs.h
+++ b/defs.h
@@ -3198,7 +3198,8 @@ void stall(ulong);
char *pages_to_size(ulong, char *);
int clean_arg(void);
int empty_list(ulong);
-int machine_type(char *);
+int machine_type(const char *);
+int is_big_endian(void);
void command_not_supported(void);
void option_not_supported(int);
void please_wait(char *);
--- a/netdump.c
+++ b/netdump.c
@@ -36,6 +36,32 @@ static void check_dumpfile_size(char *);
#define ELFREAD 0
#define MIN_PAGE_SIZE (4096)
+
+
+/*
+ * Checks if the machine type of the host matches required_type.
+ * If not, it prints a short error message for the user.
+ */
+static int machine_type_error(const char *required_type)
+{
+ if (machine_type(required_type))
+ return 1;
+ else {
+ fprintf(stderr, "Looks like a valid ELF dump, but host "
+ "architecture (%s) doesn't match dump "
+ "architecture (%s).\n",
+ MACHINE_TYPE, required_type);
+ return 0;
+ }
+}
+
+/*
+ * Returns endianess in a string
+ */
+static const char *endianess_to_string(int big_endian)
+{
+ return big_endian ? "BE" : "LE";
+}
/*
* Determine whether a file is a netdump/diskdump/kdump creation,
@@ -98,6 +124,18 @@ is_netdump(char *file, ulong source_quer
* If either kdump difference is seen, presume kdump -- this
* is obviously subject to change.
*/
+
+ /* check endianess */
+ if ((STRNEQ(elf32->e_ident, ELFMAG) || STRNEQ(elf64->e_ident, ELFMAG)) &&
+ (elf32->e_type == ET_CORE || elf64->e_type == ET_CORE) &&
+ (elf32->e_ident[EI_DATA] == ELFDATA2LSB && is_big_endian()) ||
+ (elf32->e_ident[EI_DATA] == ELFDATA2MSB && !is_big_endian()))
+ fprintf(stderr, "Looks like a valid ELF dump, but host "
+ "endianess (%s) doesn't match target "
+ "endianess (%s)\n",
+ endianess_to_string(is_big_endian()),
+ endianess_to_string(elf32->e_ident[EI_DATA] == ELFDATA2MSB));
+
if (STRNEQ(elf32->e_ident, ELFMAG) &&
(elf32->e_ident[EI_CLASS] == ELFCLASS32) &&
(elf32->e_ident[EI_DATA] == ELFDATA2LSB) &&
@@ -108,7 +146,7 @@ is_netdump(char *file, ulong source_quer
switch (elf32->e_machine)
{
case EM_386:
- if (machine_type("X86"))
+ if (machine_type_error("X86"))
break;
default:
goto bailout;
@@ -133,28 +171,28 @@ is_netdump(char *file, ulong source_quer
{
case EM_IA_64:
if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB) &&
- machine_type("IA64"))
+ machine_type_error("IA64"))
break;
else
goto bailout;
case EM_PPC64:
if ((elf64->e_ident[EI_DATA] == ELFDATA2MSB) &&
- machine_type("PPC64"))
+ machine_type_error("PPC64"))
break;
else
goto bailout;
case EM_X86_64:
if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB) &&
- machine_type("X86_64"))
+ machine_type_error("X86_64"))
break;
else
goto bailout;
case EM_386:
if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB) &&
- machine_type("X86"))
+ machine_type_error("X86"))
break;
else
goto bailout;
--- a/tools.c
+++ b/tools.c
@@ -4518,11 +4518,18 @@ empty_list(ulong list_head_addr)
}
int
-machine_type(char *type)
+machine_type(const char *type)
{
return STREQ(MACHINE_TYPE, type);
}
+int
+is_big_endian(void)
+{
+ unsigned short value = 0xff;
+ return *((unsigned char *)&value) != 0xff;
+}
+
void
command_not_supported()
{
8 a.m.
Bernhard Walle wrote:
Currently, crash prints always
crash: vmcore: not a supported file format
if you try to open a dump file which is not supported. However, it can be
misleading if you have a valid ELF core dump, but just use crash for the wrong
architecture. In the case I observed the user had a ELF64 x86 dump file and
assumed it's x86-64. However, it just was a i386 core dump which was ELF64
because kexec was called with --elf64-core-headers which makes sense
if the i386 machine has PAE and possibly more than 4 GiB of physical RAM.
After that patch is applied, an example output is
Looks like a valid ELF dump, but host architecture (X86_64) \
doesn't match dump architecture (IA64).
or if I try to open a PPC64 dump on x86-64:
Looks like a valid ELF dump, but host endianess (LE) \
doesn't match target endianess (BE)
Please review and consider applying.
I like the addition of the machine-type verification error message.
But why bother with the endian check? Using your ppc64/x86_64
example, an architecture check/error message would make far
more sense. The "endianness" error message implies that if
they re-compiled their ppc64 kernel little-endian that things
would work.
Dave
Signed-off-by: Bernhard Walle <bwalle(a)suse.de>
---
defs.h | 3 ++-
netdump.c | 48 +++++++++++++++++++++++++++++++++++++++++++-----
tools.c | 9 ++++++++-
3 files changed, 53 insertions(+), 7 deletions(-)
--- a/defs.h
+++ b/defs.h
@@ -3198,7 +3198,8 @@ void stall(ulong);
char *pages_to_size(ulong, char *);
int clean_arg(void);
int empty_list(ulong);
-int machine_type(char *);
+int machine_type(const char *);
+int is_big_endian(void);
void command_not_supported(void);
void option_not_supported(int);
void please_wait(char *);
--- a/netdump.c
+++ b/netdump.c
@@ -36,6 +36,32 @@ static void check_dumpfile_size(char *);
#define ELFREAD 0
#define MIN_PAGE_SIZE (4096)
+
+
+/*
+ * Checks if the machine type of the host matches required_type.
+ * If not, it prints a short error message for the user.
+ */
+static int machine_type_error(const char *required_type)
+{
+ if (machine_type(required_type))
+ return 1;
+ else {
+ fprintf(stderr, "Looks like a valid ELF dump, but host "
+ "architecture (%s) doesn't match dump "
+ "architecture (%s).\n",
+ MACHINE_TYPE, required_type);
+ return 0;
+ }
+}
+
+/*
+ * Returns endianess in a string
+ */
+static const char *endianess_to_string(int big_endian)
+{
+ return big_endian ? "BE" : "LE";
+}
/*
* Determine whether a file is a netdump/diskdump/kdump creation,
@@ -98,6 +124,18 @@ is_netdump(char *file, ulong source_quer
* If either kdump difference is seen, presume kdump -- this
* is obviously subject to change.
*/
+
+ /* check endianess */
+ if ((STRNEQ(elf32->e_ident, ELFMAG) || STRNEQ(elf64->e_ident, ELFMAG))
&&
+ (elf32->e_type == ET_CORE || elf64->e_type == ET_CORE) &&
+ (elf32->e_ident[EI_DATA] == ELFDATA2LSB && is_big_endian()) ||
+ (elf32->e_ident[EI_DATA] == ELFDATA2MSB && !is_big_endian()))
+ fprintf(stderr, "Looks like a valid ELF dump, but host "
+ "endianess (%s) doesn't match target "
+ "endianess (%s)\n",
+ endianess_to_string(is_big_endian()),
+ endianess_to_string(elf32->e_ident[EI_DATA] == ELFDATA2MSB));
+
if (STRNEQ(elf32->e_ident, ELFMAG) &&
(elf32->e_ident[EI_CLASS] == ELFCLASS32) &&
(elf32->e_ident[EI_DATA] == ELFDATA2LSB) &&
@@ -108,7 +146,7 @@ is_netdump(char *file, ulong source_quer
switch (elf32->e_machine)
{
case EM_386:
- if (machine_type("X86"))
+ if (machine_type_error("X86"))
break;
default:
goto bailout;
@@ -133,28 +171,28 @@ is_netdump(char *file, ulong source_quer
{
case EM_IA_64:
if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB) &&
- machine_type("IA64"))
+ machine_type_error("IA64"))
break;
else
goto bailout;
case EM_PPC64:
if ((elf64->e_ident[EI_DATA] == ELFDATA2MSB) &&
- machine_type("PPC64"))
+ machine_type_error("PPC64"))
break;
else
goto bailout;
case EM_X86_64:
if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB) &&
- machine_type("X86_64"))
+ machine_type_error("X86_64"))
break;
else
goto bailout;
case EM_386:
if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB) &&
- machine_type("X86"))
+ machine_type_error("X86"))
break;
else
goto bailout;
--- a/tools.c
+++ b/tools.c
@@ -4518,11 +4518,18 @@ empty_list(ulong list_head_addr)
}
int
-machine_type(char *type)
+machine_type(const char *type)
{
return STREQ(MACHINE_TYPE, type);
}
+int
+is_big_endian(void)
+{
+ unsigned short value = 0xff;
+ return *((unsigned char *)&value) != 0xff;
+}
+
void
command_not_supported()
{
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
8:07 a.m.
* Dave Anderson <anderson(a)redhat.com> [2007-12-21 15:00]:
I like the addition of the machine-type verification error message.
But why bother with the endian check? Using your ppc64/x86_64
example, an architecture check/error message would make far
more sense. The "endianness" error message implies that if
they re-compiled their ppc64 kernel little-endian that things
would work.
I added it because if the dump is BE (like PPC64) then the
elf64->e_type == ET_CORE check (also with ELF32) is always false and
the code never got into the switch that checks the machine type.
Thanks,
Bernhard
8:25 a.m.
New subject: [PATCH] Improve error handling when architecture doesn't match
Bernhard Walle wrote:
* Dave Anderson <anderson(a)redhat.com> [2007-12-21 15:00]:
>I like the addition of the machine-type verification error message.
>
>But why bother with the endian check? Using your ppc64/x86_64
>example, an architecture check/error message would make far
>more sense. The "endianness" error message implies that if
>they re-compiled their ppc64 kernel little-endian that things
>would work.
I added it because if the dump is BE (like PPC64) then the
elf64->e_type == ET_CORE check (also with ELF32) is always false and
the code never got into the switch that checks the machine type.
I don't follow -- the e_type is not ET_CORE?
Dave
8:21 a.m.
* Dave Anderson <anderson(a)redhat.com> [2007-12-21 15:25]:
Bernhard Walle wrote:
> * Dave Anderson <anderson(a)redhat.com> [2007-12-21 15:00]:
>
>> I like the addition of the machine-type verification error message.
>>
>> But why bother with the endian check? Using your ppc64/x86_64
>> example, an architecture check/error message would make far
>> more sense. The "endianness" error message implies that if
>> they re-compiled their ppc64 kernel little-endian that things
>> would work.
>
>
> I added it because if the dump is BE (like PPC64) then the
> elf64->e_type == ET_CORE check (also with ELF32) is always false and
> the code never got into the switch that checks the machine type.
I don't follow -- the e_type is not ET_CORE?
Well, it is, but not 0x??04 but 0x04??. But of course, it's also
possible to check the byte-toggled value. I'll send a new patch.
Thanks,
Bernhard
8:42 a.m.
New subject: [PATCH] Improve error handling when architecture doesn't match
Bernhard Walle wrote:
* Dave Anderson <anderson(a)redhat.com> [2007-12-21 15:25]:
>Bernhard Walle wrote:
>
>>* Dave Anderson <anderson(a)redhat.com> [2007-12-21 15:00]:
>>
>>
>>>I like the addition of the machine-type verification error message.
>>>
>>>But why bother with the endian check? Using your ppc64/x86_64
>>>example, an architecture check/error message would make far
>>>more sense. The "endianness" error message implies that if
>>>they re-compiled their ppc64 kernel little-endian that things
>>>would work.
>>
>>
>>I added it because if the dump is BE (like PPC64) then the
>>elf64->e_type == ET_CORE check (also with ELF32) is always false and
>>the code never got into the switch that checks the machine type.
>
>I don't follow -- the e_type is not ET_CORE?
Well, it is, but not 0x??04 but 0x04??. But of course, it's also
possible to check the byte-toggled value. I'll send a new patch.
Won't that also affect the e_machine, e_version, e_phnum fields
as well?
Dave
8:41 a.m.
* Dave Anderson <anderson(a)redhat.com> [2007-12-21 15:42]:
Bernhard Walle wrote:
> * Dave Anderson <anderson(a)redhat.com> [2007-12-21 15:25]:
>
>> Bernhard Walle wrote:
>>
>>> * Dave Anderson <anderson(a)redhat.com> [2007-12-21 15:00]:
>>>
>>>
>>>> I like the addition of the machine-type verification error message.
>>>>
>>>> But why bother with the endian check? Using your ppc64/x86_64
>>>> example, an architecture check/error message would make far
>>>> more sense. The "endianness" error message implies that if
>>>> they re-compiled their ppc64 kernel little-endian that things
>>>> would work.
>>>
>>>
>>> I added it because if the dump is BE (like PPC64) then the
>>> elf64->e_type == ET_CORE check (also with ELF32) is always false and
>>> the code never got into the switch that checks the machine type.
>>
>> I don't follow -- the e_type is not ET_CORE?
>
>
> Well, it is, but not 0x??04 but 0x04??. But of course, it's also
> possible to check the byte-toggled value. I'll send a new patch.
Won't that also affect the e_machine, e_version, e_phnum fields
as well?
Yeah.
Thanks,
Bernhard
9 a.m.
New subject: [PATCH] Improve error handling when architecture doesn't match
Bernhard Walle wrote:
* Dave Anderson <anderson(a)redhat.com> [2007-12-21 15:42]:
>Bernhard Walle wrote:
>
>>* Dave Anderson <anderson(a)redhat.com> [2007-12-21 15:25]:
>>
>>
>>>Bernhard Walle wrote:
>>>
>>>
>>>>* Dave Anderson <anderson(a)redhat.com> [2007-12-21 15:00]:
>>>>
>>>>
>>>>
>>>>>I like the addition of the machine-type verification error message.
>>>>>
>>>>>But why bother with the endian check? Using your ppc64/x86_64
>>>>>example, an architecture check/error message would make far
>>>>>more sense. The "endianness" error message implies that if
>>>>>they re-compiled their ppc64 kernel little-endian that things
>>>>>would work.
>>>>
>>>>
>>>>I added it because if the dump is BE (like PPC64) then the
>>>>elf64->e_type == ET_CORE check (also with ELF32) is always false and
>>>>the code never got into the switch that checks the machine type.
>>>
>>>I don't follow -- the e_type is not ET_CORE?
>>
>>
>>Well, it is, but not 0x??04 but 0x04??. But of course, it's also
>>possible to check the byte-toggled value. I'll send a new patch.
>
>Won't that also affect the e_machine, e_version, e_phnum fields
>as well?
Yeah.
Maybe it should cut down on the initial 32/64-bit verifier
to check only the ELFMAG and the bytes in the E_IDENT string.
Then inside of the 32 and 64 bit blocks, first check for
the architecture and bail out with a "arch-mismatch" error.
(although in doing so, the e_machine switch statement would
have to have cases for each arch and its byte-toggled version)
Then the current endian check could be made, and if it
doesn't match bail out with an "endian" error.
And then for sanity's sake, the remainder of header checks
could be done, which if we get that far, are pretty much
guaranteed to never happen.
Something like that...
Dave
9:45 a.m.
* Dave Anderson <anderson(a)redhat.com> [2007-12-21 16:00]:
Maybe it should cut down on the initial 32/64-bit verifier
to check only the ELFMAG and the bytes in the E_IDENT string.
Then inside of the 32 and 64 bit blocks, first check for
the architecture and bail out with a "arch-mismatch" error.
(although in doing so, the e_machine switch statement would
have to have cases for each arch and its byte-toggled version)
Then the current endian check could be made, and if it
doesn't match bail out with an "endian" error.
And then for sanity's sake, the remainder of header checks
could be done, which if we get that far, are pretty much
guaranteed to never happen.
Sounds sane. What about this:
---
defs.h | 3 +
netdump.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++----------------
tools.c | 9 ++++
3 files changed, 100 insertions(+), 31 deletions(-)
--- a/defs.h
+++ b/defs.h
@@ -3198,7 +3198,8 @@ void stall(ulong);
char *pages_to_size(ulong, char *);
int clean_arg(void);
int empty_list(ulong);
-int machine_type(char *);
+int machine_type(const char *);
+int is_big_endian(void);
void command_not_supported(void);
void option_not_supported(int);
void please_wait(char *);
--- a/netdump.c
+++ b/netdump.c
@@ -36,6 +36,47 @@ static void check_dumpfile_size(char *);
#define ELFREAD 0
#define MIN_PAGE_SIZE (4096)
+#define TOGGLE_ENDIAN_16(val) \
+ (((val >> 8) & 0xff) | ((val << 8) & 0xff00))
+
+
+
+/*
+ * Checks if the machine type of the host matches required_type.
+ * If not, it prints a short error message for the user.
+ */
+static int machine_type_error(const char *required_type)
+{
+ if (machine_type(required_type))
+ return 1;
+ else {
+ fprintf(stderr, "Looks like a valid ELF dump, but host "
+ "architecture (%s) doesn't match dump "
+ "architecture (%s).\n",
+ MACHINE_TYPE, required_type);
+ return 0;
+ }
+}
+
+/*
+ * Returns endianess in a string
+ */
+static const char *endianess_to_string(int big_endian)
+{
+ return big_endian ? "BE" : "LE";
+}
+
+/*
+ * Prints an endian error.
+ */
+static void endian_error(int target_big_endian)
+{
+ fprintf(stderr, "Looks like a valid ELF dump, but host "
+ "endianess (%s) doesn't match target "
+ "endianess (%s)\n",
+ endianess_to_string(is_big_endian()),
+ endianess_to_string(target_big_endian));
+}
/*
* Determine whether a file is a netdump/diskdump/kdump creation,
@@ -99,21 +140,34 @@ is_netdump(char *file, ulong source_quer
* is obviously subject to change.
*/
if (STRNEQ(elf32->e_ident, ELFMAG) &&
- (elf32->e_ident[EI_CLASS] == ELFCLASS32) &&
- (elf32->e_ident[EI_DATA] == ELFDATA2LSB) &&
- (elf32->e_ident[EI_VERSION] == EV_CURRENT) &&
- (elf32->e_type == ET_CORE) &&
- (elf32->e_version == EV_CURRENT) &&
- (elf32->e_phnum >= 2)) {
+ (elf32->e_ident[EI_CLASS] == ELFCLASS32)) {
+
+ /* check arch */
switch (elf32->e_machine)
{
case EM_386:
- if (machine_type("X86"))
- break;
+ case TOGGLE_ENDIAN_16(EM_386):
+ if (!machine_type_error("X86"))
+ goto bailout;
+ break;
default:
goto bailout;
}
+ /* check endianess */
+ if ((elf32->e_ident[EI_DATA] == ELFDATA2LSB && is_big_endian()) ||
+ (elf32->e_ident[EI_DATA] == ELFDATA2MSB && !is_big_endian())) {
+ endian_error(elf32->e_ident[EI_DATA] == ELFDATA2MSB);
+ goto bailout;
+ }
+
+ /* check other ELF headers */
+ if ((elf32->e_ident[EI_VERSION] != EV_CURRENT) ||
+ (elf32->e_type != ET_CORE) ||
+ (elf32->e_version != EV_CURRENT) ||
+ (elf32->e_phnum < 2))
+ goto bailout;
+
load32 = (Elf32_Phdr *)
&header[sizeof(Elf32_Ehdr)+sizeof(Elf32_Phdr)];
size = (size_t)load32->p_offset;
@@ -124,45 +178,52 @@ is_netdump(char *file, ulong source_quer
else
tmp_flags |= NETDUMP_ELF32;
} else if (STRNEQ(elf64->e_ident, ELFMAG) &&
- (elf64->e_ident[EI_CLASS] == ELFCLASS64) &&
- (elf64->e_ident[EI_VERSION] == EV_CURRENT) &&
- (elf64->e_type == ET_CORE) &&
- (elf64->e_version == EV_CURRENT) &&
- (elf64->e_phnum >= 2)) {
+ (elf64->e_ident[EI_CLASS] == ELFCLASS64)) {
+
+ /* check arch */
switch (elf64->e_machine)
{
case EM_IA_64:
- if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB) &&
- machine_type("IA64"))
- break;
- else
+ case TOGGLE_ENDIAN_16(EM_IA_64):
+ if (!machine_type_error("IA64"))
goto bailout;
+ break;
case EM_PPC64:
- if ((elf64->e_ident[EI_DATA] == ELFDATA2MSB) &&
- machine_type("PPC64"))
- break;
- else
+ case TOGGLE_ENDIAN_16(EM_PPC64):
+ if (!machine_type_error("PPC64"))
goto bailout;
+ break;
case EM_X86_64:
- if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB) &&
- machine_type("X86_64"))
- break;
- else
+ case TOGGLE_ENDIAN_16(EM_X86_64):
+ if (!machine_type_error("X86_64"))
goto bailout;
+ break;
case EM_386:
- if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB) &&
- machine_type("X86"))
- break;
- else
+ case TOGGLE_ENDIAN_16(EM_386):
+ if (!machine_type_error("X86"))
goto bailout;
+ break;
default:
goto bailout;
}
+ /* check endianess */
+ if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB && is_big_endian()) ||
+ (elf64->e_ident[EI_DATA] == ELFDATA2MSB && !is_big_endian())) {
+ endian_error(elf64->e_ident[EI_DATA] == ELFDATA2MSB);
+ goto bailout;
+ }
+
+ if ((elf64->e_ident[EI_VERSION] != EV_CURRENT) ||
+ (elf64->e_type != ET_CORE) ||
+ (elf64->e_version != EV_CURRENT) ||
+ (elf64->e_phnum < 2))
+ goto bailout;
+
load64 = (Elf64_Phdr *)
&header[sizeof(Elf64_Ehdr)+sizeof(Elf64_Phdr)];
size = (size_t)load64->p_offset;
--- a/tools.c
+++ b/tools.c
@@ -4518,11 +4518,18 @@ empty_list(ulong list_head_addr)
}
int
-machine_type(char *type)
+machine_type(const char *type)
{
return STREQ(MACHINE_TYPE, type);
}
+int
+is_big_endian(void)
+{
+ unsigned short value = 0xff;
+ return *((unsigned char *)&value) != 0xff;
+}
+
void
command_not_supported()
{
10:20 a.m.
New subject: [PATCH] Improve error handling when architecture doesn't match
Bernhard Walle wrote:
* Dave Anderson <anderson(a)redhat.com> [2007-12-21 16:00]:
>Maybe it should cut down on the initial 32/64-bit verifier
>to check only the ELFMAG and the bytes in the E_IDENT string.
>Then inside of the 32 and 64 bit blocks, first check for
>the architecture and bail out with a "arch-mismatch" error.
>(although in doing so, the e_machine switch statement would
>have to have cases for each arch and its byte-toggled version)
>
>Then the current endian check could be made, and if it
>doesn't match bail out with an "endian" error.
>
>And then for sanity's sake, the remainder of header checks
>could be done, which if we get that far, are pretty much
>guaranteed to never happen.
Sounds sane. What about this:
Looks reasonable.
BTW, since is_netdump() gets called twice, first
directly and then via is_kdump(), do the error
messages get printed twice?
---
defs.h | 3 +
netdump.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++----------------
tools.c | 9 ++++
3 files changed, 100 insertions(+), 31 deletions(-)
--- a/defs.h
+++ b/defs.h
@@ -3198,7 +3198,8 @@ void stall(ulong);
char *pages_to_size(ulong, char *);
int clean_arg(void);
int empty_list(ulong);
-int machine_type(char *);
+int machine_type(const char *);
+int is_big_endian(void);
void command_not_supported(void);
void option_not_supported(int);
void please_wait(char *);
--- a/netdump.c
+++ b/netdump.c
@@ -36,6 +36,47 @@ static void check_dumpfile_size(char *);
#define ELFREAD 0
#define MIN_PAGE_SIZE (4096)
+#define TOGGLE_ENDIAN_16(val) \
+ (((val >> 8) & 0xff) | ((val << 8) & 0xff00))
+
+
+
+/*
+ * Checks if the machine type of the host matches required_type.
+ * If not, it prints a short error message for the user.
+ */
+static int machine_type_error(const char *required_type)
+{
+ if (machine_type(required_type))
+ return 1;
+ else {
+ fprintf(stderr, "Looks like a valid ELF dump, but host "
+ "architecture (%s) doesn't match dump "
+ "architecture (%s).\n",
+ MACHINE_TYPE, required_type);
+ return 0;
+ }
+}
+
+/*
+ * Returns endianess in a string
+ */
+static const char *endianess_to_string(int big_endian)
+{
+ return big_endian ? "BE" : "LE";
+}
+
+/*
+ * Prints an endian error.
+ */
+static void endian_error(int target_big_endian)
+{
+ fprintf(stderr, "Looks like a valid ELF dump, but host "
+ "endianess (%s) doesn't match target "
+ "endianess (%s)\n",
+ endianess_to_string(is_big_endian()),
+ endianess_to_string(target_big_endian));
+}
/*
* Determine whether a file is a netdump/diskdump/kdump creation,
@@ -99,21 +140,34 @@ is_netdump(char *file, ulong source_quer
* is obviously subject to change.
*/
if (STRNEQ(elf32->e_ident, ELFMAG) &&
- (elf32->e_ident[EI_CLASS] == ELFCLASS32) &&
- (elf32->e_ident[EI_DATA] == ELFDATA2LSB) &&
- (elf32->e_ident[EI_VERSION] == EV_CURRENT) &&
- (elf32->e_type == ET_CORE) &&
- (elf32->e_version == EV_CURRENT) &&
- (elf32->e_phnum >= 2)) {
+ (elf32->e_ident[EI_CLASS] == ELFCLASS32)) {
+
+ /* check arch */
switch (elf32->e_machine)
{
case EM_386:
- if (machine_type("X86"))
- break;
+ case TOGGLE_ENDIAN_16(EM_386):
+ if (!machine_type_error("X86"))
+ goto bailout;
+ break;
default:
goto bailout;
}
+ /* check endianess */
+ if ((elf32->e_ident[EI_DATA] == ELFDATA2LSB && is_big_endian()) ||
+ (elf32->e_ident[EI_DATA] == ELFDATA2MSB && !is_big_endian())) {
+ endian_error(elf32->e_ident[EI_DATA] == ELFDATA2MSB);
+ goto bailout;
+ }
+
+ /* check other ELF headers */
+ if ((elf32->e_ident[EI_VERSION] != EV_CURRENT) ||
+ (elf32->e_type != ET_CORE) ||
+ (elf32->e_version != EV_CURRENT) ||
+ (elf32->e_phnum < 2))
+ goto bailout;
+
load32 = (Elf32_Phdr *)
&header[sizeof(Elf32_Ehdr)+sizeof(Elf32_Phdr)];
size = (size_t)load32->p_offset;
@@ -124,45 +178,52 @@ is_netdump(char *file, ulong source_quer
else
tmp_flags |= NETDUMP_ELF32;
} else if (STRNEQ(elf64->e_ident, ELFMAG) &&
- (elf64->e_ident[EI_CLASS] == ELFCLASS64) &&
- (elf64->e_ident[EI_VERSION] == EV_CURRENT) &&
- (elf64->e_type == ET_CORE) &&
- (elf64->e_version == EV_CURRENT) &&
- (elf64->e_phnum >= 2)) {
+ (elf64->e_ident[EI_CLASS] == ELFCLASS64)) {
+
+ /* check arch */
switch (elf64->e_machine)
{
case EM_IA_64:
- if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB) &&
- machine_type("IA64"))
- break;
- else
+ case TOGGLE_ENDIAN_16(EM_IA_64):
+ if (!machine_type_error("IA64"))
goto bailout;
+ break;
case EM_PPC64:
- if ((elf64->e_ident[EI_DATA] == ELFDATA2MSB) &&
- machine_type("PPC64"))
- break;
- else
+ case TOGGLE_ENDIAN_16(EM_PPC64):
+ if (!machine_type_error("PPC64"))
goto bailout;
+ break;
case EM_X86_64:
- if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB) &&
- machine_type("X86_64"))
- break;
- else
+ case TOGGLE_ENDIAN_16(EM_X86_64):
+ if (!machine_type_error("X86_64"))
goto bailout;
+ break;
case EM_386:
- if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB) &&
- machine_type("X86"))
- break;
- else
+ case TOGGLE_ENDIAN_16(EM_386):
+ if (!machine_type_error("X86"))
goto bailout;
+ break;
default:
goto bailout;
}
+ /* check endianess */
+ if ((elf64->e_ident[EI_DATA] == ELFDATA2LSB && is_big_endian()) ||
+ (elf64->e_ident[EI_DATA] == ELFDATA2MSB && !is_big_endian())) {
+ endian_error(elf64->e_ident[EI_DATA] == ELFDATA2MSB);
+ goto bailout;
+ }
+
+ if ((elf64->e_ident[EI_VERSION] != EV_CURRENT) ||
+ (elf64->e_type != ET_CORE) ||
+ (elf64->e_version != EV_CURRENT) ||
+ (elf64->e_phnum < 2))
+ goto bailout;
+
load64 = (Elf64_Phdr *)
&header[sizeof(Elf64_Ehdr)+sizeof(Elf64_Phdr)];
size = (size_t)load64->p_offset;
--- a/tools.c
+++ b/tools.c
@@ -4518,11 +4518,18 @@ empty_list(ulong list_head_addr)
}
int
-machine_type(char *type)
+machine_type(const char *type)
{
return STREQ(MACHINE_TYPE, type);
}
+int
+is_big_endian(void)
+{
+ unsigned short value = 0xff;
+ return *((unsigned char *)&value) != 0xff;
+}
+
void
command_not_supported()
{
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
10:32 a.m.
New subject: [PATCH] Improve error handling when architecture doesn't match
Bernhard Walle wrote:
* Dave Anderson <anderson(a)redhat.com> [2007-12-21 17:20]:
>Looks reasonable.
>
>BTW, since is_netdump() gets called twice, first
>directly and then via is_kdump(), do the error
>messages get printed twice?
Yes. But I don't think it's a problem, is it?
Thanks,
Bernhard
No, just looks funny. Anyway, I'll run with it
from here -- nice improvement.
Thanks,
Dave
3:30 p.m.
New subject: [PATCH] Improve error handling when architecture doesn't match
Dave Anderson wrote:
Bernhard Walle wrote:
> * Dave Anderson <anderson(a)redhat.com> [2007-12-21 17:20]:
>
>> Looks reasonable.
>>
>> BTW, since is_netdump() gets called twice, first
>> directly and then via is_kdump(), do the error
>> messages get printed twice?
>
>
>
> Yes. But I don't think it's a problem, is it?
>
>
> Thanks,
> Bernhard
No, just looks funny. Anyway, I'll run with it
from here -- nice improvement.
Thanks,
Dave
Actually this patch has just turned up different issues
that have to be handled, because the e_type and e_phnum
get deferred until after the e_machine and endianness
are checked.
Among them the fact that an i386 xen guest core file
taken by an x86_64 host has the e_machine type set
to x86_64 (don't ask me why they did that...), and has
an e_phnum of 0. Anyway, that requires the e_phnum
to be checked *before* the machine type and endianness.
And another, since the e_type doesn't get checked
until *after* the machine type and endianness,
it allows the vmlinux file (ET_EXEC) to get passed
through, which can generate a bogus error message
about the vmlinux file!
And there's probably others...
There was a method to my madness in the way it's written
now. I'm going to have to spend some more time with
this because I don't want to introduce false alarms
or print error messages that don't make any sense...
Dave
5:32 p.m.
* Dave Anderson <anderson(a)redhat.com> [2007-12-21 22:30]:
Actually this patch has just turned up different issues
that have to be handled, because the e_type and e_phnum
get deferred until after the e_machine and endianness
are checked.
Among them the fact that an i386 xen guest core file
taken by an x86_64 host has the e_machine type set
to x86_64 (don't ask me why they did that...), and has
an e_phnum of 0. Anyway, that requires the e_phnum
to be checked *before* the machine type and endianness.
And another, since the e_type doesn't get checked
until *after* the machine type and endianness,
it allows the vmlinux file (ET_EXEC) to get passed
through, which can generate a bogus error message
about the vmlinux file!
And there's probably others...
There was a method to my madness in the way it's written
now. I'm going to have to spend some more time with
this because I don't want to introduce false alarms
or print error messages that don't make any sense...
*Arrrg*, sorry for not taking all this into account. I only tested
with a few Kdump dumps from different architectures, but not with Xen
dumps.
You're right, and I'll send a new patch that tries to handle all this.
But probably next year ...
Merry Christmas,
Bernhard
6185
days inactive
6185
days old
devel@lists.crash-utility.osci.io
13 comments
2 participants
participants (2)
-
Bernhard Walle -
Dave Anderson