Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header
From: Daniel Wagner <hidden>
Date: 2016-02-01 16:07:14
Also in:
lkml
Subsystem:
mips, the rest · Maintainers:
Thomas Bogendoerfer, Linus Torvalds
On 02/01/2016 01:52 AM, Maciej W. Rozycki wrote:
On Fri, 29 Jan 2016, Daniel Wagner wrote:quoted
Depending on the configuration either the 32 or 64 bit version of elf_check_arch() is defined. parse_crash_elf32_headers() does some basic verification of the ELF header via elf_check_arch(). parse_crash_elf64_headers() does it via vmcore_elf64_check_arch() which expands to the same elf_check_check(). In file included from include/linux/elf.h:4:0, from fs/proc/vmcore.c:13: fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':quoted
quoted
arch/mips/include/asm/elf.h:228:23: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]struct elfhdr *__h = (hdr); \ ^ include/linux/crash_dump.h:41:37: note: in expansion of macro 'elf_check_arch' #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x)) ^ fs/proc/vmcore.c:1015:4: note: in expansion of macro 'vmcore_elf64_check_arch' !vmcore_elf64_check_arch(&ehdr) || ^ Since the MIPS ELF header for 32 bit and 64 bit differ we need to check accordingly.I fail to see how it can work as it stands given that `elf_check_arch' is called from the same source file both on a pointer to `Elf32_Ehdr' and one to `Elf64_Ehdr'. However the MIPS implementations of `elf_check_arch' only use an auxiliary variable to avoid multiple evaluation of a macro argument and therefore instead I recommend the use of the usual approach taken in such a situation within a statement expression, that is to declare the variable with `typeof' rather than an explicit type. As an upside this will minimise code disruption as well.
Good point on the type for hdr. Thought elf_check_arch() implementation differ on 32 bit and 64 bit implementation. I played a bit around and the simplest version I found was this here:
diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index b01a6ff..8c88238 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h@@ -205,8 +205,6 @@ struct mips_elf_abiflags_v0 { #define MIPS_ABI_FP_64 6 /* -mips32r2 -mfp64 */ #define MIPS_ABI_FP_64A 7 /* -mips32r2 -mfp64 -mno-odd-spreg */ -#ifdef CONFIG_32BIT - /* * In order to be sure that we don't attempt to execute an O32 binary which * requires 64 bit FP (FR=1) on a system which does not support it we refuse
@@ -225,23 +223,30 @@ struct mips_elf_abiflags_v0 { #define elf_check_arch(hdr) \ ({ \ int __res = 1; \ - struct elfhdr *__h = (hdr); \ + typeof(*(hdr)) *__h = (hdr); \ \ if (__h->e_machine != EM_MIPS) \ __res = 0; \ - if (__h->e_ident[EI_CLASS] != ELFCLASS32) \ - __res = 0; \ - if ((__h->e_flags & EF_MIPS_ABI2) != 0) \ - __res = 0; \ - if (((__h->e_flags & EF_MIPS_ABI) != 0) && \ - ((__h->e_flags & EF_MIPS_ABI) != EF_MIPS_ABI_O32)) \ - __res = 0; \ - if (__h->e_flags & __MIPS_O32_FP64_MUST_BE_ZERO) \ - __res = 0; \ + if (__same_type(hdr, Elf32_Ehdr *)) { \ + if (__h->e_ident[EI_CLASS] != ELFCLASS32) \ + __res = 0; \ + if ((__h->e_flags & EF_MIPS_ABI2) != 0) \ + __res = 0; \ + if (((__h->e_flags & EF_MIPS_ABI) != 0) && \ + ((__h->e_flags & EF_MIPS_ABI) != EF_MIPS_ABI_O32)) \ + __res = 0; \ + if (__h->e_flags & __MIPS_O32_FP64_MUST_BE_ZERO) \ + __res = 0; \ + } else if (__same_type(hdr, Elf64_Ehdr *)) { \ + if (__h->e_ident[EI_CLASS] != ELFCLASS64) \ + __res = 0; \ + } \ \ __res; \ }) +#ifdef CONFIG_32BIT + /* * These are used to set parameters in the core dumps. */
@@ -250,21 +255,6 @@ struct mips_elf_abiflags_v0 { #endif /* CONFIG_32BIT */ #ifdef CONFIG_64BIT -/* - * This is used to ensure we don't load something for the wrong architecture. - */ -#define elf_check_arch(hdr) \ -({ \ - int __res = 1; \ - struct elfhdr *__h = (hdr); \ - \ - if (__h->e_machine != EM_MIPS) \ - __res = 0; \ - if (__h->e_ident[EI_CLASS] != ELFCLASS64) \ - __res = 0; \ - \ - __res; \ -}) /* * These are used to set parameters in the core dumps.
Not sure if that is what you had in mind. cheers, daniel