From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1LlA0S-0001Sz-1O for mharc-grub-devel@gnu.org; Sat, 21 Mar 2009 18:49:32 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LlA0N-0001Pu-Ty for grub-devel@gnu.org; Sat, 21 Mar 2009 18:49:27 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LlA0J-0001PT-7I for grub-devel@gnu.org; Sat, 21 Mar 2009 18:49:27 -0400 Received: from [199.232.76.173] (port=53262 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LlA0J-0001PQ-1r for grub-devel@gnu.org; Sat, 21 Mar 2009 18:49:23 -0400 Received: from fg-out-1718.google.com ([72.14.220.153]:51947) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LlA0I-0003GW-Eu for grub-devel@gnu.org; Sat, 21 Mar 2009 18:49:22 -0400 Received: by fg-out-1718.google.com with SMTP id l27so202278fgb.7 for ; Sat, 21 Mar 2009 15:49:20 -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=Hb+YkYpYbsfToVr1AFQCRH2B6Qa1mnCplC0hGnmI85Y=; b=KIW+5SI/WwhjAeDF5HxFFT+1c4m9TaVYfXFu/GHHjYohLmCeLXTjCe4G5s4+b6r0eM GD1L/wvff3Iy62f3Z2WWPQnYh3brH5GyBuhiocmu9qYOdA95N1kYaASE2xAW6cN/y2sF 0dqv2l26xO9DiehtbTKmi+GGc97TjRO+iGUXk= 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=w7x/TVENmdaWIbSgGBEJ2yDEGGXYQWGHaojsCLZEiYfNeQBvUQzsLPDnoqdwKubYVn XdYTKBH8Czx7Kga3faSYYsRgq9TBhvfyZ5jWtsIyAaMeeutzG06CCuxUtMaOWDEpw3dX 7+XcDeXEXWIgHJFfLur50HlX+4KMXeQXmlkZg= Received: by 10.86.79.19 with SMTP id c19mr2481371fgb.45.1237675760148; Sat, 21 Mar 2009 15:49:20 -0700 (PDT) Received: from ?192.168.1.25? (217-154.203-62.cust.bluewin.ch [62.203.154.217]) by mx.google.com with ESMTPS id 4sm1146599fge.28.2009.03.21.15.49.19 (version=SSLv3 cipher=RC4-MD5); Sat, 21 Mar 2009 15:49:19 -0700 (PDT) Message-ID: <49C56EEE.50809@gmail.com> Date: Sat, 21 Mar 2009 23:49:18 +0100 From: phcoder User-Agent: Thunderbird 2.0.0.21 (X11/20090318) MIME-Version: 1.0 To: The development of GRUB 2 References: <20090313191442.GC17068@thorin> <49BAC506.2030006@gmail.com> <20090313.134505.185970759.davem@davemloft.net> <49BAC797.9010200@gmail.com> <20090318101227.GB20072@thorin> <49C0F690.7060305@gmail.com> <20090321174629.GB18284@thorin> <49C52AE2.5070202@gmail.com> <20090321180315.GF18284@thorin> <49C52C63.4050904@gmail.com> <20090321220312.GA19598@thorin> In-Reply-To: <20090321220312.GA19598@thorin> Content-Type: multipart/mixed; boundary="------------070409010407060107070206" 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: Sat, 21 Mar 2009 22:49:28 -0000 This is a multi-part message in MIME format. --------------070409010407060107070206 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Robert Millan wrote: > On Sat, Mar 21, 2009 at 07:05:23PM +0100, phcoder wrote: >> Robert Millan wrote: >>> On Sat, Mar 21, 2009 at 06:58:58PM +0100, phcoder wrote: >>>> Robert Millan wrote: >>>>> On Wed, Mar 18, 2009 at 02:26:40PM +0100, phcoder wrote: >>>>>> Robert Millan wrote: >>>>>>> On Fri, Mar 13, 2009 at 09:52:39PM +0100, phcoder wrote: >>>>>>>> - 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); >>>>>>> You need to handle the case in which grub_multiboot_payload_entry_offset is left >>>>>>> uninitialized (it needs to be initialized each time the multiboot command is >>>>>>> run, not just when the module is loaded). >>>>>>> >>>>>> module? actually it's when loading image. Perhaps you mean that >>>>>> additional error check is necessary >>>>> I meant GRUB's multiboot.mod, not the payload's module. Sorry I wasn't clear. >>>>> >>>> With this error check if grub_multiboot_payload_entry_offset it can >>>> happen only if no image is loaded. And actually >>>> grub_multiboot_payload_entry_offset is set to 0 at multiboot.mod load >>>> So I don't really understand the problem >>> You can't rely on grub_multiboot_payload_entry_offset being set to 0, because >>> any subsequent call of "multiboot /something" has the potential to override >>> this. You must not assume the multiboot command is only going to be run once. >>> >> No but it always corresponds to the current image. It's set either in >> multiboot.c or in grub_multiboot_load_elf > > It is now, but your code makes this conditional. > -- Regards Vladimir 'phcoder' Serbinenko --------------070409010407060107070206 Content-Type: text/x-diff; name="elf.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="elf.diff" diff --git a/loader/i386/multiboot_elfxx.c b/loader/i386/multiboot_elfxx.c index 801800c..ce9e4fe 100644 --- a/loader/i386/multiboot_elfxx.c +++ b/loader/i386/multiboot_elfxx.c @@ -49,7 +49,7 @@ CONCAT(grub_multiboot_load_elf, XX) (grub_file_t file, void *buffer) { 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,11 +83,18 @@ CONCAT(grub_multiboot_load_elf, XX) (grub_file_t file, void *buffer) 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) + /* Beware that segment 0 isn't necessarily loadable */ + 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; } + + if (lowest_segment == -1) + return grub_error (GRUB_ERR_BAD_OS, "ELF contains no loadable segments"); + code_size = (phdr(highest_segment)->p_paddr + phdr(highest_segment)->p_memsz) - phdr(lowest_segment)->p_paddr; grub_multiboot_payload_dest = phdr(lowest_segment)->p_paddr; @@ -105,8 +112,8 @@ CONCAT(grub_multiboot_load_elf, XX) (grub_file_t file, void *buffer) { 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 +131,17 @@ CONCAT(grub_multiboot_load_elf, XX) (grub_file_t file, void *buffer) } } - 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); + break; + } + + if (i == ehdr->e_phnum) + return grub_error (GRUB_ERR_BAD_OS, "entry point isn't in a segment"); #undef phdr --------------070409010407060107070206--