From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1LhgCZ-0001c8-OA for mharc-grub-devel@gnu.org; Thu, 12 Mar 2009 04:23:39 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LhgCX-0001a0-Ix for grub-devel@gnu.org; Thu, 12 Mar 2009 04:23:37 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LhgCV-0001Yj-KV for grub-devel@gnu.org; Thu, 12 Mar 2009 04:23:36 -0400 Received: from [199.232.76.173] (port=40444 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LhgCV-0001Yg-AG for grub-devel@gnu.org; Thu, 12 Mar 2009 04:23:35 -0400 Received: from mail-bw0-f172.google.com ([209.85.218.172]:51641) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LhgCU-0005Io-LI for grub-devel@gnu.org; Thu, 12 Mar 2009 04:23:35 -0400 Received: by mail-bw0-f172.google.com with SMTP id 20so305220bwz.42 for ; Thu, 12 Mar 2009 01:23:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from :user-agent:mime-version:to:subject:references:in-reply-to :content-type; bh=KKhaDAUffFGCV7j5TgnjvQnwPbYWniOnx1jP03ha1qE=; b=NnG/0dUZjoPoRgmIDt4fL95Jqra6m3i0k/cxO10qazgkGbtfFtz+qH7jNbLWl6b4aH YRcLj4c8cbZ6XSbuIwJeRZtjlf9x7tOgNwUUun2lYhsxDCNpBljLXodWyhzegNdJUv1p W8Tas4nTwW/1S5TzeDFgVEiH+OoThf1sg3Yhw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:content-type; b=IUHpgaJSIOWmw02UhCNfLkva97lItAq+Oao/vc30Uh5tajSXT3+QrtS1Ztkoikygb5 LuBS0a9xFXpk7SfSMyRTkewQ07Xqy5trr8kLXmhLNayxPQkXEVvF2jShD8kjhmMc+9NF rHWP8yuGnZrUA9m3doEy0dkY4AS3pWxtjgJ+I= Received: by 10.223.126.10 with SMTP id a10mr7083626fas.17.1236846214135; Thu, 12 Mar 2009 01:23:34 -0700 (PDT) Received: from ?82.130.83.231? (hpx-public-dock-231-dhcp.ethz.ch [82.130.83.231]) by mx.google.com with ESMTPS id c28sm551619fka.26.2009.03.12.01.23.33 (version=SSLv3 cipher=RC4-MD5); Thu, 12 Mar 2009 01:23:33 -0700 (PDT) Message-ID: <49B8C686.5060508@gmail.com> Date: Thu, 12 Mar 2009 09:23:34 +0100 From: phcoder User-Agent: Thunderbird 2.0.0.19 (X11/20090105) MIME-Version: 1.0 To: The development of GRUB 2 References: <49AB29BA.6040609@gmail.com> <20090311211541.GC29956@thorin> <49B82B65.3080506@gmail.com> In-Reply-To: <49B82B65.3080506@gmail.com> Content-Type: multipart/mixed; boundary="------------040604000103020705010406" X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 2) Subject: Re: ELF bugfixes 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: Thu, 12 Mar 2009 08:23:38 -0000 This is a multi-part message in MIME format. --------------040604000103020705010406 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Fixed phcoder wrote: > Robert Millan wrote: >> On Mon, Mar 02, 2009 at 01:35:06AM +0100, phcoder wrote: >>> + * include/grub/elf.h: added missing attributes >> >> This should be a bit more descriptive. >> >>> for (i = 0; i < ehdr->e_phnum; i++) >>> if (phdr(i)->p_type == PT_LOAD && phdr(i)->p_filesz != 0) >>> { >>> - if (phdr(i)->p_paddr < phdr(lowest_segment)->p_paddr) >>> + if (lowest_segment == -1 + || phdr(i)->p_paddr < >>> phdr(lowest_segment)->p_paddr) >>> lowest_segment = i; >>> - if (phdr(i)->p_paddr > phdr(highest_segment)->p_paddr) >>> + if (highest_segment == -1 >>> + || phdr(i)->p_paddr > phdr(highest_segment)->p_paddr) >>> highest_segment = i; >>> } >> >> Why? > > Because if first segment doesn't have the PT_LOAD attribute set then it > should be considered in this comparison >> >>> - grub_multiboot_payload_entry_offset = ehdr->e_entry - >>> phdr(lowest_segment)->p_vaddr; >>> + grub_multiboot_payload_entry_offset = ehdr->e_entry - >>> phdr(lowest_segment)->p_paddr; >> >> Are you sure about this? IIRC e_entry is in the virtual address >> space. I >> think we had some trouble with this (with NetBSD?), which lead to the >> current >> use of p_vaddr in this line. >> > Actually now thinking I see that the problem is more deep. The section > which is loaded at the lowest address isn't necessarily the section > which contains entry point. I'll fix this part cleanly and will resubmit > the patch -- Regards Vladimir 'phcoder' Serbinenko --------------040604000103020705010406 Content-Type: text/x-diff; name="elffixes.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="elffixes.diff" Index: include/grub/elf.h =================================================================== --- include/grub/elf.h (revision 2036) +++ include/grub/elf.h (working copy) @@ -77,7 +77,7 @@ Elf32_Half e_shentsize; /* Section header table entry size */ Elf32_Half e_shnum; /* Section header table entry count */ Elf32_Half e_shstrndx; /* Section header string table index */ -} Elf32_Ehdr; +} __attribute__ ((packed)) Elf32_Ehdr; typedef struct { @@ -95,7 +95,7 @@ Elf64_Half e_shentsize; /* Section header table entry size */ Elf64_Half e_shnum; /* Section header table entry count */ Elf64_Half e_shstrndx; /* Section header string table index */ -} Elf64_Ehdr; +} __attribute__ ((packed)) Elf64_Ehdr; /* Fields in the e_ident array. The EI_* macros are indices into the array. The macros under each EI_* macro are the values the byte @@ -272,7 +272,7 @@ Elf32_Word sh_info; /* Additional section information */ Elf32_Word sh_addralign; /* Section alignment */ Elf32_Word sh_entsize; /* Entry size if section holds table */ -} Elf32_Shdr; +} __attribute__ ((packed)) Elf32_Shdr; typedef struct { @@ -286,7 +286,7 @@ Elf64_Word sh_info; /* Additional section information */ Elf64_Xword sh_addralign; /* Section alignment */ Elf64_Xword sh_entsize; /* Entry size if section holds table */ -} Elf64_Shdr; +} __attribute__ ((packed)) Elf64_Shdr; /* Special section indices. */ @@ -367,7 +367,7 @@ unsigned char st_info; /* Symbol type and binding */ unsigned char st_other; /* Symbol visibility */ Elf32_Section st_shndx; /* Section index */ -} Elf32_Sym; +} __attribute__ ((packed)) Elf32_Sym; typedef struct { @@ -377,7 +377,7 @@ Elf64_Section st_shndx; /* Section index */ Elf64_Addr st_value; /* Symbol value */ Elf64_Xword st_size; /* Symbol size */ -} Elf64_Sym; +} __attribute__ ((packed)) Elf64_Sym; /* The syminfo section if available contains additional information about every dynamic symbol. */ @@ -386,13 +386,13 @@ { Elf32_Half si_boundto; /* Direct bindings, symbol bound to */ Elf32_Half si_flags; /* Per symbol flags */ -} Elf32_Syminfo; +} __attribute__ ((packed)) Elf32_Syminfo; typedef struct { Elf64_Half si_boundto; /* Direct bindings, symbol bound to */ Elf64_Half si_flags; /* Per symbol flags */ -} Elf64_Syminfo; +} __attribute__ ((packed)) Elf64_Syminfo; /* Possible values for si_boundto. */ #define SYMINFO_BT_SELF 0xffff /* Symbol bound to self */ @@ -477,7 +477,7 @@ { Elf32_Addr r_offset; /* Address */ Elf32_Word r_info; /* Relocation type and symbol index */ -} Elf32_Rel; +} __attribute__ ((packed)) Elf32_Rel; /* I have seen two different definitions of the Elf64_Rel and Elf64_Rela structures, so we'll leave them out until Novell (or @@ -488,7 +488,7 @@ { Elf64_Addr r_offset; /* Address */ Elf64_Xword r_info; /* Relocation type and symbol index */ -} Elf64_Rel; +} __attribute__ ((packed)) Elf64_Rel; /* Relocation table entry with addend (in section of type SHT_RELA). */ @@ -497,14 +497,14 @@ Elf32_Addr r_offset; /* Address */ Elf32_Word r_info; /* Relocation type and symbol index */ Elf32_Sword r_addend; /* Addend */ -} Elf32_Rela; +} __attribute__ ((packed)) Elf32_Rela; typedef struct { Elf64_Addr r_offset; /* Address */ Elf64_Xword r_info; /* Relocation type and symbol index */ Elf64_Sxword r_addend; /* Addend */ -} Elf64_Rela; +} __attribute__ ((packed)) Elf64_Rela; /* How to extract and insert information held in the r_info field. */ @@ -528,7 +528,7 @@ Elf32_Word p_memsz; /* Segment size in memory */ Elf32_Word p_flags; /* Segment flags */ Elf32_Word p_align; /* Segment alignment */ -} Elf32_Phdr; +} __attribute__ ((packed)) Elf32_Phdr; typedef struct { @@ -540,7 +540,7 @@ Elf64_Xword p_filesz; /* Segment size in file */ Elf64_Xword p_memsz; /* Segment size in memory */ Elf64_Xword p_align; /* Segment alignment */ -} Elf64_Phdr; +} __attribute__ ((packed)) Elf64_Phdr; /* Legal values for p_type (segment type). */ @@ -604,7 +604,7 @@ Elf32_Word d_val; /* Integer value */ Elf32_Addr d_ptr; /* Address value */ } d_un; -} Elf32_Dyn; +} __attribute__ ((packed)) Elf32_Dyn; typedef struct { @@ -614,7 +614,7 @@ Elf64_Xword d_val; /* Integer value */ Elf64_Addr d_ptr; /* Address value */ } d_un; -} Elf64_Dyn; +} __attribute__ ((packed)) Elf64_Dyn; /* Legal values for d_tag (dynamic entry type). */ @@ -770,7 +770,7 @@ Elf32_Word vd_aux; /* Offset in bytes to verdaux array */ Elf32_Word vd_next; /* Offset in bytes to next verdef entry */ -} Elf32_Verdef; +} __attribute__ ((packed)) Elf32_Verdef; typedef struct { @@ -782,7 +782,7 @@ Elf64_Word vd_aux; /* Offset in bytes to verdaux array */ Elf64_Word vd_next; /* Offset in bytes to next verdef entry */ -} Elf64_Verdef; +} __attribute__ ((packed)) Elf64_Verdef; /* Legal values for vd_version (version revision). */ @@ -807,14 +807,14 @@ Elf32_Word vda_name; /* Version or dependency names */ Elf32_Word vda_next; /* Offset in bytes to next verdaux entry */ -} Elf32_Verdaux; +} __attribute__ ((packed)) Elf32_Verdaux; typedef struct { Elf64_Word vda_name; /* Version or dependency names */ Elf64_Word vda_next; /* Offset in bytes to next verdaux entry */ -} Elf64_Verdaux; +} __attribute__ ((packed)) Elf64_Verdaux; /* Version dependency section. */ @@ -828,7 +828,7 @@ Elf32_Word vn_aux; /* Offset in bytes to vernaux array */ Elf32_Word vn_next; /* Offset in bytes to next verneed entry */ -} Elf32_Verneed; +} __attribute__ ((packed)) Elf32_Verneed; typedef struct { @@ -839,7 +839,7 @@ Elf64_Word vn_aux; /* Offset in bytes to vernaux array */ Elf64_Word vn_next; /* Offset in bytes to next verneed entry */ -} Elf64_Verneed; +} __attribute__ ((packed)) Elf64_Verneed; /* Legal values for vn_version (version revision). */ @@ -857,7 +857,7 @@ Elf32_Word vna_name; /* Dependency name string offset */ Elf32_Word vna_next; /* Offset in bytes to next vernaux entry */ -} Elf32_Vernaux; +} __attribute__ ((packed)) Elf32_Vernaux; typedef struct { @@ -867,7 +867,7 @@ Elf64_Word vna_name; /* Dependency name string offset */ Elf64_Word vna_next; /* Offset in bytes to next vernaux entry */ -} Elf64_Vernaux; +} __attribute__ ((packed)) Elf64_Vernaux; /* Legal values for vna_flags. */ @@ -892,7 +892,7 @@ void *a_ptr; /* Pointer value */ void (*a_fcn) (void); /* Function pointer value */ } a_un; -} Elf32_auxv_t; +} __attribute__ ((packed)) Elf32_auxv_t; typedef struct { @@ -903,7 +903,7 @@ void *a_ptr; /* Pointer value */ void (*a_fcn) (void); /* Function pointer value */ } a_un; -} Elf64_auxv_t; +} __attribute__ ((packed)) Elf64_auxv_t; /* Legal values for a_type (entry type). */ @@ -951,14 +951,14 @@ Elf32_Word n_namesz; /* Length of the note's name. */ Elf32_Word n_descsz; /* Length of the note's descriptor. */ Elf32_Word n_type; /* Type of the note. */ -} Elf32_Nhdr; +} __attribute__ ((packed)) Elf32_Nhdr; typedef struct { Elf64_Word n_namesz; /* Length of the note's name. */ Elf64_Word n_descsz; /* Length of the note's descriptor. */ Elf64_Word n_type; /* Type of the note. */ -} Elf64_Nhdr; +} __attribute__ ((packed)) Elf64_Nhdr; /* Known names of notes. */ @@ -1000,7 +1000,7 @@ Elf32_Word m_poffset; /* Symbol offset. */ Elf32_Half m_repeat; /* Repeat count. */ Elf32_Half m_stride; /* Stride info. */ -} Elf32_Move; +} __attribute__ ((packed)) Elf32_Move; typedef struct { @@ -1009,7 +1009,7 @@ Elf64_Xword m_poffset; /* Symbol offset. */ Elf64_Half m_repeat; /* Repeat count. */ Elf64_Half m_stride; /* Stride info. */ -} Elf64_Move; +} __attribute__ ((packed)) Elf64_Move; /* Macro to construct move records. */ #define ELF32_M_SYM(info) ((info) >> 8) @@ -1369,7 +1369,7 @@ Elf32_Word gt_g_value; /* If this value were used for -G */ Elf32_Word gt_bytes; /* This many bytes would be used */ } gt_entry; /* Subsequent entries in section */ -} Elf32_gptab; +} __attribute__ ((packed)) Elf32_gptab; /* Entry found in sections of type SHT_MIPS_REGINFO. */ @@ -1378,7 +1378,7 @@ Elf32_Word ri_gprmask; /* General registers used */ Elf32_Word ri_cprmask[4]; /* Coprocessor registers used */ Elf32_Sword ri_gp_value; /* $gp register value */ -} Elf32_RegInfo; +} __attribute__ ((packed)) Elf32_RegInfo; /* Entries found in sections of type SHT_MIPS_OPTIONS. */ @@ -1390,7 +1390,7 @@ Elf32_Section section; /* Section header index of section affected, 0 for global options. */ Elf32_Word info; /* Kind-specific information. */ -} Elf_Options; +} __attribute__ ((packed)) Elf_Options; /* Values for `kind' field in Elf_Options. */ @@ -1437,7 +1437,7 @@ { Elf32_Word hwp_flags1; /* Extra flags. */ Elf32_Word hwp_flags2; /* Extra flags. */ -} Elf_Options_Hw; +} __attribute__ ((packed)) Elf_Options_Hw; /* Masks for `info' in ElfOptions for ODK_HWAND and ODK_HWOR entries. */ @@ -1579,7 +1579,7 @@ Elf32_Word l_checksum; /* Checksum */ Elf32_Word l_version; /* Interface version */ Elf32_Word l_flags; /* Flags */ -} Elf32_Lib; +} __attribute__ ((packed)) Elf32_Lib; typedef struct { @@ -1588,7 +1588,7 @@ Elf64_Word l_checksum; /* Checksum */ Elf64_Word l_version; /* Interface version */ Elf64_Word l_flags; /* Flags */ -} Elf64_Lib; +} __attribute__ ((packed)) Elf64_Lib; /* Legal values for l_flags. */ Index: ChangeLog =================================================================== --- ChangeLog (revision 2036) +++ ChangeLog (working copy) @@ -1,3 +1,12 @@ +2009-03-01 Vladimir Serbinenko + + Bugfixes in multiboot for bugs uncovered by solaris kernel + + * loader/i386/multiboot_elfxx.c (grub_multiboot_load_elf): corrected + limit detection + Use vaddr of correct segment for entry_point + * include/grub/elf.h: added missing __attribute__ ((packed)) + 2009-03-12 Vladimir Serbinenko Parttool Index: loader/i386/multiboot_elfxx.c =================================================================== --- loader/i386/multiboot_elfxx.c (revision 2036) +++ loader/i386/multiboot_elfxx.c (working copy) @@ -49,7 +49,7 @@ { Elf_Ehdr *ehdr = (Elf_Ehdr *) buffer; char *phdr_base; - int lowest_segment = 0, highest_segment = 0; + int lowest_segment = -1, highest_segment = -1; int i; if (ehdr->e_ident[EI_CLASS] != ELFCLASSXX) @@ -83,9 +83,11 @@ for (i = 0; i < ehdr->e_phnum; i++) if (phdr(i)->p_type == PT_LOAD && phdr(i)->p_filesz != 0) { - if (phdr(i)->p_paddr < phdr(lowest_segment)->p_paddr) + if (lowest_segment == -1 + || phdr(i)->p_paddr < phdr(lowest_segment)->p_paddr) lowest_segment = i; - if (phdr(i)->p_paddr > phdr(highest_segment)->p_paddr) + if (highest_segment == -1 + || phdr(i)->p_paddr > phdr(highest_segment)->p_paddr) highest_segment = i; } code_size = (phdr(highest_segment)->p_paddr + phdr(highest_segment)->p_memsz) - phdr(lowest_segment)->p_paddr; @@ -105,8 +107,8 @@ { char *load_this_module_at = (char *) (grub_multiboot_payload_orig + (long) (phdr(i)->p_paddr - phdr(lowest_segment)->p_paddr)); - grub_dprintf ("multiboot_loader", "segment %d: paddr=0x%lx, memsz=0x%lx\n", - i, (long) phdr(i)->p_paddr, (long) phdr(i)->p_memsz); + grub_dprintf ("multiboot_loader", "segment %d: paddr=0x%lx, memsz=0x%lx, vaddr=0x%lx\n", + i, (long) phdr(i)->p_paddr, (long) phdr(i)->p_memsz, (long) phdr(i)->p_vaddr); if (grub_file_seek (file, (grub_off_t) phdr(i)->p_offset) == (grub_off_t) -1) @@ -124,7 +126,11 @@ } } - grub_multiboot_payload_entry_offset = ehdr->e_entry - phdr(lowest_segment)->p_vaddr; + for (i = 0; i < ehdr->e_phnum; i++) + if (phdr(i)->p_vaddr <= ehdr->e_entry + && phdr(i)->p_vaddr + phdr(i)->p_memsz > ehdr->e_entry) + grub_multiboot_payload_entry_offset = (ehdr->e_entry - phdr(i)->p_vaddr) + + (phdr(i)->p_paddr - phdr(lowest_segment)->p_paddr); #undef phdr --------------040604000103020705010406--