Skip to contentSkip to conversation

vmcore-dmesg: Determine file data size based on e_machine

PATCH In review Vivek Goyal 2 participants 3 messages Nov 12, 2012 – Nov 13, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 vmcore-dmesg: Determine file data size based on e_machine

A 32bit arch can prepare ELF64 headers. For example for PAE case to preresent file offsets 64bit but data size at the offset still remains 32bit. If we just base our decision based on EI_CLASS, then we will try to read 64bit data from file and can run into various issues.

We ran into following issue when we tried to run vmcore-dmesg on a 32bit PAE system vmcore which had 64bit elf headers.

No program header covering vaddr 0xc0a6a688c0b89100found kexec bug?

Basically we try to read value of log_buf variable from address log_buf_vaddr. We read in 64bit value and then pass that value again to vaddr_to_offset() in an attempt to get to actual log_buf start and get error message.

So determine the data size based on arch and read the bytes from file accordingly.

---
 vmcore-dmesg/vmcore-dmesg.c |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)
vmcore-dmesg/vmcore-dmesg.c +19 -1
Index: kexec-tools/vmcore-dmesg/vmcore-dmesg.c
===================================================================
@@ -89,6 +89,23 @@ static uint64_t vaddr_to_offset(uint64_t
 	exit(30);
 }
 
+/*
+ * For PAE arch, one can prepare 64bit ELF headers to make offsets 64bit in
+ * file but data size still remains 32bit. Determine data size based on arch
+ * (e_machine).
+ *
+ * returns 1 if arch passed in is 64bit
+ */
+static bool file_data_size_64bit(unsigned int e_machine)
+{
+	switch(e_machine) {
+		/* TODO: keep on adding 32bit arch to get vmcore-dmesg right */
+		case EM_386:
+			return 0;
+	}
+	return 1;
+}
+
 static void read_elf32(int fd)
 {
 	Elf32_Ehdr ehdr32;
@@ -389,7 +406,8 @@ static uint64_t read_file_pointer(int fd
 {
 	uint64_t result;
 	ssize_t ret;
-	if (ehdr.e_ident[EI_CLASS] == ELFCLASS64) {
+	if (ehdr.e_ident[EI_CLASS] == ELFCLASS64 &&
+	    file_data_size_64bit(ehdr.e_machine)) {
 		uint64_t scratch;
 		ret = pread(fd, &scratch, sizeof(scratch), addr);
 		if (ret != sizeof(scratch)) {

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#1 lore raw
Quoted 10 lines · Vivek Goyal

A 32bit arch can prepare ELF64 headers. For example for PAE case to preresent file offsets 64bit but data size at the offset still remains 32bit. If we just base our decision based on EI_CLASS, then we will try to read 64bit data from file and can run into various issues.

We ran into following issue when we tried to run vmcore-dmesg on a 32bit PAE system vmcore which had 64bit elf headers.

No program header covering vaddr 0xc0a6a688c0b89100found kexec bug?

Quoted 7 lines

Basically we try to read value of log_buf variable from address log_buf_vaddr. We read in 64bit value and then pass that value again to vaddr_to_offset() in an attempt to get to actual log_buf start and get error message.

So determine the data size based on arch and read the bytes from file accordingly.

The basic code change is sound. However the naming is problematic.

Let me suggest:

static unsigned machine_pointer_bits(void)

{
        uint8_t bits = 0;

        /* Default to the size of the elf class */
        switch(ehdr.e_ident[EI_CLASS]) {
        case ELFCLASS32:	bits = 32; break;
        case ELFCLASS64:	bits = 64; break;
        }

        /* Report the architectures pointer size */
        switch(ehdr.e_machine) {
        case EM_386:		bits = 32; break;
        }

        return bits;
}
Quoted 34 lines
---
 vmcore-dmesg/vmcore-dmesg.c |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

Index: kexec-tools/vmcore-dmesg/vmcore-dmesg.c
===================================================================

vmcore-dmesg/vmcore-dmesg.c +17 -0
@@ -89,6 +89,23 @@ static uint64_t vaddr_to_offset(uint64_t
 	exit(30);
 }
 
+/*
+ * For PAE arch, one can prepare 64bit ELF headers to make offsets 64bit in
+ * file but data size still remains 32bit. Determine data size based on arch
+ * (e_machine).
+ *
+ * returns 1 if arch passed in is 64bit
+ */
+static bool file_data_size_64bit(unsigned int e_machine)
+{
+	switch(e_machine) {
+		/* TODO: keep on adding 32bit arch to get vmcore-dmesg right */
+		case EM_386:
+			return 0;
+	}
+	return 1;
+}
+
 static void read_elf32(int fd)
 {
 	Elf32_Ehdr ehdr32;

vmcore-dmesg/vmcore-dmesg.c static uint64_t read_file_pointer(int fd · line 410

Quoted 6 lines
vmcore-dmesg/vmcore-dmesg.c +2 -1
@@ -389,7 +406,8 @@ static uint64_t read_file_pointer(int fd
 {
 	uint64_t result;
 	ssize_t ret;
-	if (ehdr.e_ident[EI_CLASS] == ELFCLASS64) {
+	if (ehdr.e_ident[EI_CLASS] == ELFCLASS64 &&
+	    file_data_size_64bit(ehdr.e_machine)) {

Then this test can become:

	if (machine_pointer_bits() == 64) {

vmcore-dmesg/vmcore-dmesg.c static uint64_t read_file_pointer(int fd · line 413

Quoted 3 lines
vmcore-dmesg/vmcore-dmesg.c +0 -0
 		uint64_t scratch;
 		ret = pread(fd, &scratch, sizeof(scratch), addr);
 		if (ret != sizeof(scratch)) {

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#2 lore raw
Quoted 25 lines · Eric W. Biederman
Quoted 10 lines · Vivek Goyal

A 32bit arch can prepare ELF64 headers. For example for PAE case to preresent file offsets 64bit but data size at the offset still remains 32bit. If we just base our decision based on EI_CLASS, then we will try to read 64bit data from file and can run into various issues.

We ran into following issue when we tried to run vmcore-dmesg on a 32bit PAE system vmcore which had 64bit elf headers.

No program header covering vaddr 0xc0a6a688c0b89100found kexec bug?

Quoted 7 lines

Basically we try to read value of log_buf variable from address log_buf_vaddr. We read in 64bit value and then pass that value again to vaddr_to_offset() in an attempt to get to actual log_buf start and get error message.

So determine the data size based on arch and read the bytes from file accordingly.

The basic code change is sound. However the naming is problematic.

Let me suggest:

static unsigned machine_pointer_bits(void)

Quoted 16 lines
{
        uint8_t bits = 0;

        /* Default to the size of the elf class */
        switch(ehdr.e_ident[EI_CLASS]) {
        case ELFCLASS32:	bits = 32; break;
        case ELFCLASS64:	bits = 64; break;
        }

        /* Report the architectures pointer size */
        switch(ehdr.e_machine) {
        case EM_386:		bits = 32; break;
        }

        return bits;
}

Thanks Eric. Version 2 is on the way.

Vivek

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec