Re: [PATCH] vmcore-dmesg: Determine file data size based on e_machine

[ebiederm@xmission.com (Eric W. Biederman)]

Vivek Goyal <vgoyal@redhat.com> writes:

> 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.

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;
}


> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  vmcore-dmesg/vmcore-dmesg.c |   20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> Index: kexec-tools/vmcore-dmesg/vmcore-dmesg.c
> ===================================================================
> --- kexec-tools.orig/vmcore-dmesg/vmcore-dmesg.c	2012-08-03 15:46:54.000000000 -0400
> +++ kexec-tools/vmcore-dmesg/vmcore-dmesg.c	2012-11-08 05:30:17.918099858 -0500
> @@ -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)) {
Then this test can become:
	if (machine_pointer_bits() == 64) {
>  		uint64_t scratch;
>  		ret = pread(fd, &scratch, sizeof(scratch), addr);
>  		if (ret != sizeof(scratch)) {

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec