[Crash-utility] [PATCH] Improve error handling when architecture doesn't match

Dave Anderson anderson at redhat.com
Fri Dec 21 16:20:45 UTC 2007


Bernhard Walle wrote:
> * Dave Anderson <anderson at 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility





More information about the Crash-utility mailing list