From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1CxYwB-0004F4-WA for mharc-grub-devel@gnu.org; Sat, 05 Feb 2005 18:02:00 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1CxYw9-0004E8-9e for grub-devel@gnu.org; Sat, 05 Feb 2005 18:01:57 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1CxYw7-0004D9-AN for grub-devel@gnu.org; Sat, 05 Feb 2005 18:01:55 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1CxYw7-0004Cc-4g for grub-devel@gnu.org; Sat, 05 Feb 2005 18:01:55 -0500 Received: from [62.57.231.123] (helo=pulsar.hadrons.org) by monty-python.gnu.org with esmtp (Exim 4.34) id 1CxYgH-0002NN-TG for grub-devel@gnu.org; Sat, 05 Feb 2005 17:45:34 -0500 Received: from zulo.hadrons.org ([192.168.1.5]) by pulsar.hadrons.org with esmtp (Exim 3.36 #1 (Debian)) id 1CxYgJ-0002nO-00 for ; Sat, 05 Feb 2005 23:45:35 +0100 Received: from guillem by zulo.hadrons.org with local (Exim 4.44) id 1CxYgL-000608-Qq for grub-devel@gnu.org; Sat, 05 Feb 2005 23:45:37 +0100 Date: Sat, 5 Feb 2005 23:45:37 +0100 From: Guillem Jover To: grub-devel@gnu.org Message-ID: <20050205224537.GA17984@zulo.hadrons.org> References: <20050201044708.GB3969@zulo.hadrons.org> <87brb03qv4.fsf@marco.marco-g.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87brb03qv4.fsf@marco.marco-g.com> User-Agent: Mutt/1.5.6+20040907i Sender: Guillem Jover Subject: Re: consolidate ELF header checks X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Feb 2005 23:01:57 -0000 Hi, On Fri, Feb 04, 2005 at 10:25:03PM +0000, Marco Gerards wrote: > Guillem Jover writes: > > This patch merges common ELF header checks. Please test this on PPC > > as I've only tested on x86. > > I've done that. I could load linux using this patch after a small > change. Thanks! :) > As far as I am concerned there are no other issues with this patch. > If someone else has problems with this patch, please tell me. > Otherwise I will commit it soon, if Guillem can fix these issues... :) I've fixed the error messages, the missing & and the unused size variable warnings. regards, guillem 2005-02-05 Guillem Jover * include/grub/dl.h (grub_dl_check_header): New prototype. (grub_arch_dl_check_header): Change return type to grub_err_t and remove size parameter. Fix all callers. * kern/dl.c (grub_dl_check_header): New function. (grub_dl_load_core): Use grub_dl_check_header instead of grub_arch_dl_check_header. Check ELF type. Check sections to be inside the core. * kern/i386/dl.c (grub_arch_dl_check_header): Remove arch independent ELF header checks. * kern/powerpc/dl.c (grub_arch_dl_check_header): Likewise. * loader/i386/pc/multiboot.c (grub_rescue_cmd_multiboot): Use grub_dl_check_header instead of explicit checks. Check for ELF type. * loader/powerpc/ieee1275/linux.c (grub_rescue_cmd_linux): Use grub_dl_check_header instead of explicit checks. Remove arch specific ELF header checks. diff -Naupr -x CVS grub2-cvs/include/grub/dl.h grub2.dl/include/grub/dl.h --- grub2-cvs/include/grub/dl.h 2005-02-01 04:34:12.000000000 +0100 +++ grub2.dl/include/grub/dl.h 2005-02-01 04:03:26.000000000 +0100 @@ -70,6 +70,7 @@ struct grub_dl }; typedef struct grub_dl *grub_dl_t; +grub_err_t EXPORT_FUNC(grub_dl_check_header) (void *ehdr, grub_size_t size); grub_dl_t EXPORT_FUNC(grub_dl_load_file) (const char *filename); grub_dl_t EXPORT_FUNC(grub_dl_load) (const char *name); grub_dl_t grub_dl_load_core (void *addr, grub_size_t size); @@ -84,7 +85,7 @@ grub_err_t EXPORT_FUNC(grub_dl_register_ grub_dl_t mod); void *EXPORT_FUNC(grub_dl_resolve_symbol) (const char *name); -int grub_arch_dl_check_header (void *ehdr, grub_size_t size); +grub_err_t grub_arch_dl_check_header (void *ehdr); grub_err_t grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr); #endif /* ! GRUB_DL_H */ diff -Naupr -x CVS grub2-cvs/kern/dl.c grub2.dl/kern/dl.c --- grub2-cvs/kern/dl.c 2005-02-01 04:34:13.000000000 +0100 +++ grub2.dl/kern/dl.c 2005-02-01 04:05:45.000000000 +0100 @@ -238,6 +238,29 @@ return 0; } +/* Check if EHDR is a valid ELF header. */ +grub_err_t +grub_dl_check_header (void *ehdr, grub_size_t size) +{ + Elf_Ehdr *e = ehdr; + + /* Check the header size. */ + if (size < sizeof (Elf_Ehdr)) + return grub_error (GRUB_ERR_BAD_OS, "ELF header smaller than expected"); + + /* Check the magic numbers. */ + if (grub_arch_dl_check_header (ehdr) + || e->e_ident[EI_MAG0] != ELFMAG0 + || e->e_ident[EI_MAG1] != ELFMAG1 + || e->e_ident[EI_MAG2] != ELFMAG2 + || e->e_ident[EI_MAG3] != ELFMAG3 + || e->e_ident[EI_VERSION] != EV_CURRENT + || e->e_version != EV_CURRENT) + return grub_error (GRUB_ERR_BAD_OS, "Invalid arch independent ELF magic"); + + return GRUB_ERR_NONE; +} + /* Load all segments from memory specified by E. */ static grub_err_t grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e) @@ -497,12 +520,25 @@ grub_dl_load_core (void *addr, grub_size grub_dl_t mod; e = addr; - if (! grub_arch_dl_check_header (e, size)) + if (grub_dl_check_header (e, size)) { grub_error (GRUB_ERR_BAD_MODULE, "invalid ELF header"); return 0; } + if (e->e_type != ET_REL) + { + grub_error (GRUB_ERR_BAD_MODULE, "Invalid ELF file type\n"); + return 0; + } + + /* Make sure that every section is within the core. */ + if (size < e->e_shoff + e->e_shentsize * e->e_shnum) + { + grub_error (GRUB_ERR_BAD_OS, "ELF sections outside core"); + return 0; + } + mod = (grub_dl_t) grub_malloc (sizeof (*mod)); if (! mod) return 0; diff -Naupr -x CVS grub2-cvs/kern/i386/dl.c grub2.dl/kern/i386/dl.c --- grub2-cvs/kern/i386/dl.c 2005-02-01 04:34:13.000000000 +0100 +++ grub2.dl/kern/i386/dl.c 2005-02-01 04:03:26.000000000 +0100 @@ -24,32 +24,18 @@ #include /* Check if EHDR is a valid ELF header. */ -int -grub_arch_dl_check_header (void *ehdr, grub_size_t size) +grub_err_t +grub_arch_dl_check_header (void *ehdr) { Elf32_Ehdr *e = ehdr; - /* Check the header size. */ - if (size < sizeof (Elf32_Ehdr)) - return 0; - /* Check the magic numbers. */ - if (e->e_ident[EI_MAG0] != ELFMAG0 - || e->e_ident[EI_MAG1] != ELFMAG1 - || e->e_ident[EI_MAG2] != ELFMAG2 - || e->e_ident[EI_MAG3] != ELFMAG3 - || e->e_version != EV_CURRENT - || e->e_ident[EI_CLASS] != ELFCLASS32 + if (e->e_ident[EI_CLASS] != ELFCLASS32 || e->e_ident[EI_DATA] != ELFDATA2LSB - || e->e_machine != EM_386 - || e->e_type != ET_REL) - return 0; - - /* Make sure that every section is within the core. */ - if (size < e->e_shoff + e->e_shentsize * e->e_shnum) - return 0; + || e->e_machine != EM_386) + return grub_error (GRUB_ERR_BAD_OS, "Invalid arch specific ELF magic"); - return 1; + return GRUB_ERR_NONE; } /* Relocate symbols. */ diff -Naupr -x CVS grub2-cvs/kern/powerpc/dl.c grub2.dl/kern/powerpc/dl.c --- grub2-cvs/kern/powerpc/dl.c 2005-02-01 04:34:13.000000000 +0100 +++ grub2.dl/kern/powerpc/dl.c 2005-02-01 04:03:26.000000000 +0100 @@ -24,32 +24,18 @@ #include /* Check if EHDR is a valid ELF header. */ -int -grub_arch_dl_check_header (void *ehdr, grub_size_t size) +grub_err_t +grub_arch_dl_check_header (void *ehdr) { Elf32_Ehdr *e = ehdr; - /* Check the header size. */ - if (size < sizeof (Elf32_Ehdr)) - return 0; - /* Check the magic numbers. */ - if (!((e->e_ident[EI_MAG0] == ELFMAG0) - && (e->e_ident[EI_MAG1] == ELFMAG1) - && (e->e_ident[EI_MAG2] == ELFMAG2) - && (e->e_ident[EI_MAG3] == ELFMAG3) - && (e->e_ident[EI_CLASS] == ELFCLASS32) - && (e->e_ident[EI_DATA] == ELFDATA2MSB) - && (e->e_ident[EI_VERSION] == EV_CURRENT) - && (e->e_type == ET_REL) && (e->e_machine == EM_PPC) - && (e->e_version == EV_CURRENT))) - return 0; - - /* Make sure that every section is within the core. */ - if (size < e->e_shoff + e->e_shentsize * e->e_shnum) - return 0; + if (e->e_ident[EI_CLASS] != ELFCLASS32 + || e->e_ident[EI_DATA] != ELFDATA2MSB + || e->e_machine != EM_PPC) + return grub_error (GRUB_ERR_BAD_OS, "Invalid arch specific ELF magic"); - return 1; + return GRUB_ERR_NONE; } diff -Naupr -x CVS grub2-cvs/loader/i386/pc/multiboot.c grub2.dl/loader/i386/pc/multiboot.c --- grub2-cvs/loader/i386/pc/multiboot.c 2005-02-01 04:34:13.000000000 +0100 +++ grub2.dl/loader/i386/pc/multiboot.c 2005-02-01 04:35:25.000000000 +0100 @@ -140,20 +140,18 @@ grub_rescue_cmd_multiboot (int argc, cha ehdr = (Elf32_Ehdr *) buffer; - if (!((ehdr->e_ident[EI_MAG0] == ELFMAG0) - && (ehdr->e_ident[EI_MAG1] == ELFMAG1) - && (ehdr->e_ident[EI_MAG2] == ELFMAG2) - && (ehdr->e_ident[EI_MAG3] == ELFMAG3) - && (ehdr->e_ident[EI_CLASS] == ELFCLASS32) - && (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - && (ehdr->e_ident[EI_VERSION] == EV_CURRENT) - && (ehdr->e_type == ET_EXEC) && (ehdr->e_machine == EM_386) - && (ehdr->e_version == EV_CURRENT))) + if (grub_dl_check_header (ehdr, sizeof(*ehdr))) { grub_error (GRUB_ERR_UNKNOWN_OS, "No valid ELF header found"); goto fail; } + if (ehdr->e_type != ET_EXEC) + { + grub_error (GRUB_ERR_UNKNOWN_OS, "Invalid ELF file type\n"); + goto fail; + } + /* FIXME: Should we support program headers at strange locations? */ if (ehdr->e_phoff + ehdr->e_phnum * ehdr->e_phentsize > GRUB_MB_SEARCH) { diff -Naupr -x CVS grub2-cvs/loader/powerpc/ieee1275/linux.c grub2.dl/loader/powerpc/ieee1275/linux.c --- grub2-cvs/loader/powerpc/ieee1275/linux.c 2005-02-01 04:34:13.000000000 +0100 +++ grub2.dl/loader/powerpc/ieee1275/linux.c 2005-02-01 04:03:26.000000000 +0100 @@ -124,15 +124,7 @@ grub_rescue_cmd_linux (int argc, char *a goto fail; } - if (!((ehdr.e_ident[EI_MAG0] == ELFMAG0) - && (ehdr.e_ident[EI_MAG1] == ELFMAG1) - && (ehdr.e_ident[EI_MAG2] == ELFMAG2) - && (ehdr.e_ident[EI_MAG3] == ELFMAG3) - && (ehdr.e_ident[EI_CLASS] == ELFCLASS32) - && (ehdr.e_ident[EI_DATA] == ELFDATA2MSB) - && (ehdr.e_ident[EI_VERSION] == EV_CURRENT) - && (ehdr.e_type == ET_EXEC) && (ehdr.e_machine == EM_PPC) - && (ehdr.e_version == EV_CURRENT))) + if (grub_dl_check_header (&ehdr, sizeof(ehdr))) { grub_error (GRUB_ERR_UNKNOWN_OS, "No valid ELF header found"); goto fail; @@ -145,20 +137,6 @@ grub_rescue_cmd_linux (int argc, char *a goto fail; } - if (ehdr.e_machine != EM_PPC) - { - grub_error (GRUB_ERR_UNKNOWN_OS, - "This ELF file is not for the PPC32\n"); - goto fail; - } - - if (ehdr.e_version != EV_CURRENT) - { - grub_error (GRUB_ERR_UNKNOWN_OS, - "Invalid ELF version\n"); - goto fail; - } - /* Read the sections. */ entry = ehdr.e_entry; if (entry == 0xc0000000) diff -Naupr -x CVS grub2-cvs/util/grub-emu.c grub2.dl/util/grub-emu.c --- grub2-cvs/util/grub-emu.c 2005-02-01 04:34:13.000000000 +0100 +++ grub2.dl/util/grub-emu.c 2005-02-01 04:03:26.000000000 +0100 @@ -51,11 +51,10 @@ grub_arch_modules_addr (void) return 0; } -int -grub_arch_dl_check_header (void *ehdr, grub_size_t size) +grub_err_t +grub_arch_dl_check_header (void *ehdr) { (void) ehdr; - (void) size; return GRUB_ERR_BAD_MODULE; }