On Sun, Jul 27, 2008 at 11:39:49PM +0200, Robert Millan wrote:
> On Sun, Jul 20, 2008 at 11:09:02AM +0200, Yoshinori K. Okuji wrote:
> > > IIRC this causes trouble when the loadee chose an address that precisely
> > > overwrites the loader, which is garanteed to happen when GRUB is loading
> > > itself, AFAICT.
> >
> > Sure. My recommendation is, in case where you might overwrite that part,
> > that
> > you should write relocatable code (which is rather easy for simple code on
> > i386) at anywhere (it could be in the startup), find out a safe region when
> > loading an OS image, copy the code to the safe region, and finalize the
> > bootstrap in that code (e.g. relocating the OS image, initializing
> > registers,
> > and jumping to it). On i386, we have a reserved region to temporarily load
> > an
> > OS image for the very reason, so this is not difficult.
>
> Ok. I've been looking at grub_multiboot_load_elf32() which contains the
> bound checks that make loading abort in first place; It seems that bounds
> are checked for every segment in the ELF image, in:
>
> /* Load every loadable segment in memory. */
> for (i = 0; i < ehdr->e_phnum; i++)
>
> so I'm wondering if it is safe to assume the segments are going to occupy
> a single block of memory (which can be relocated in one run) or it is allowed
> for them to be scattered.
>
> As for the safe region, AFAICT the OS load area is our only choice, or maybe
> the heap, but in both cases overlaps are a problem, as we don't want the
> relocator code to overwrite itself. In case of the OS load area, we could
> abort on situations where payload requested region overlaps with our area,
> and in case of the heap, we could play some ugly tricks in order to obtain
> a non-overlapped region from malloc.
>
> TBH I don't like either of the options. Do you have any other suggestions?
Let's try to revive this discussion. Here's a patch I made a while ago that
implements support for loading at any address. It works by having a "special"
version of malloc() that is told to allocate a chunk of memory that does
_not_ overlap with a specific region. It does so by iteratively reserving
memory (and keeping track of what was reserved, of course).
Then we use this function to allocate the asm relocator code in heap, asking
it to garantee that it won't overlap with our final destination. Finally,
our loadee can be put anywhere (e.g. in heap), and we just jump to the
relocator which will jump to the loadee.
But I really find the approach really ugly. What do you think? If we agree
that this is the way to go, I can do some cleanup & update it to current svn
for a merge.
--
Robert Millan
The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
how) you may access your data; but nobody's threatening your freedom: we
still allow you to remove your data and not access it at all."
diff -ur grub2/include/grub/i386/loader.h grub2.self/include/grub/i386/loader.h
--- grub2/include/grub/i386/loader.h 2007-10-17 11:38:55.000000000 +0200
+++ grub2.self/include/grub/i386/loader.h 2007-10-17 12:36:16.000000000 +0200
@@ -29,13 +29,23 @@
extern grub_addr_t EXPORT_VAR(grub_os_area_addr);
extern grub_size_t EXPORT_VAR(grub_os_area_size);
+extern grub_addr_t EXPORT_VAR(playground);
+extern grub_addr_t EXPORT_VAR(requested_addr);
+extern int EXPORT_VAR(playground_size);
+
void EXPORT_FUNC(grub_linux_boot_zimage) (void) __attribute__ ((noreturn));
void EXPORT_FUNC(grub_linux_boot_bzimage) (void) __attribute__ ((noreturn));
/* The asm part of the multiboot loader. */
+void EXPORT_FUNC(grub_multiboot_real_boot_1st) (grub_addr_t entry,
+ struct grub_multiboot_info *mbi,
+ void *next)
+ __attribute__ ((noreturn));
void EXPORT_FUNC(grub_multiboot_real_boot) (grub_addr_t entry,
struct grub_multiboot_info *mbi)
__attribute__ ((noreturn));
+extern int EXPORT_VAR(end_of_grub_multiboot_real_boot);
+
void EXPORT_FUNC(grub_multiboot2_real_boot) (grub_addr_t entry,
struct grub_multiboot_info *mbi)
__attribute__ ((noreturn));
diff -ur grub2/kern/i386/pc/startup.S grub2.self/kern/i386/pc/startup.S
--- grub2/kern/i386/pc/startup.S 2007-07-25 21:29:24.000000000 +0200
+++ grub2.self/kern/i386/pc/startup.S 2007-10-17 12:36:16.000000000 +0200
@@ -800,23 +800,59 @@
* This starts the multiboot kernel.
*/
-FUNCTION(grub_multiboot_real_boot)
+VARIABLE(playground)
+ .long 0
+VARIABLE(playground_size)
+ .long 0
+VARIABLE(requested_addr)
+ .long 0
+
+/* entry, mbi, next */
+FUNCTION(grub_multiboot_real_boot_1st)
/* Push the entry address on the stack. */
pushl %eax
/* Move the address of the multiboot information structure to ebx. */
movl %edx,%ebx
+ /* Push the grub_multiboot_real_boot address on the stack. */
+ pushl %ecx
/* Unload all modules and stop the floppy driver. */
call EXT_C(grub_dl_unload_all)
call EXT_C(grub_stop_floppy)
+ movl EXT_C(playground_size), %ecx
+ movl EXT_C(playground), %esi
+ movl EXT_C(requested_addr), %edi
+
/* Interrupts should be disabled. */
cli
-
- /* Move the magic value into eax and jump to the kernel. */
+
+ /* Jump to relocated grub_multiboot_real_boot */
+ popl %eax
+ jmp *%eax
+
+ /* External references stop working at this point */
+
+FUNCTION(grub_multiboot_real_boot)
+ /* Get this from stack now that we still can */
+ popl %edx
+
+ /* Relocate the payload. Overlaps are garanteed NOT to happen, so
+ we don't need to check for them (like in grub_memmove) */
+
+ /* %ecx, %esi and %edi were set in the previous function */
+ cld
+ rep
+ movsb
+
+ /* Stack may have stopped working at this point */
+
+ /* Move the magic value into eax, and jump to the kernel */
movl $MULTIBOOT_MAGIC2,%eax
- popl %ecx
- jmp *%ecx
+ jmp *%edx
+
+/* Nothing here, just used to relocate the above function */
+VARIABLE(end_of_grub_multiboot_real_boot)
/*
* This starts the multiboot 2 kernel.
diff -ur grub2/loader/i386/pc/multiboot.c grub2.self/loader/i386/pc/multiboot.c
--- grub2/loader/i386/pc/multiboot.c 2007-07-25 21:29:24.000000000 +0200
+++ grub2.self/loader/i386/pc/multiboot.c 2007-10-17 12:36:16.000000000 +0200
@@ -47,10 +47,48 @@
static struct grub_multiboot_info *mbi;
static grub_addr_t entry;
+extern grub_addr_t playground;
+extern int playground_size;
+extern grub_addr_t requested_addr;
+
+void *
+grub_protected_malloc (grub_size_t size, grub_addr_t protected_region_addr, grub_size_t protected_region_size)
+{
+ void **old, **tmp, **ret;
+ tmp = grub_malloc (size);
+ *tmp = NULL;
+
+ while ((tmp + size >= protected_region_addr) && (tmp <= protected_region_addr + protected_region_size))
+ {
+ /* Get a new region */
+ old = tmp;
+ tmp = grub_malloc (size);
+ /* Store the address of our previous region */
+ *tmp = old;
+ }
+
+ /* Save result */
+ ret = tmp;
+
+ /* Cleanup */
+ tmp = *tmp;
+ while (tmp)
+ {
+ old = *tmp;
+ grub_free (tmp);
+ tmp = old;
+ }
+
+ return ret;
+}
+
static grub_err_t
grub_multiboot_boot (void)
{
- grub_multiboot_real_boot (entry, mbi);
+ void *tmp = grub_protected_malloc ((long) &end_of_grub_multiboot_real_boot - (long) grub_multiboot_real_boot + 32,
+ requested_addr, playground_size);
+ grub_memcpy (tmp, grub_multiboot_real_boot, (long) &end_of_grub_multiboot_real_boot - (long) grub_multiboot_real_boot + 32);
+ grub_multiboot_real_boot_1st (entry, mbi, tmp);
/* Not reached. */
return GRUB_ERR_NONE;
@@ -94,7 +132,7 @@
grub_multiboot_load_elf32 (grub_file_t file, void *buffer)
{
Elf32_Ehdr *ehdr = (Elf32_Ehdr *) buffer;
- Elf32_Phdr *phdr;
+ void *phdr_base;
int i;
if (ehdr->e_ident[EI_CLASS] != ELFCLASS32)
@@ -112,35 +150,37 @@
entry = ehdr->e_entry;
+ phdr_base = (void *) buffer + ehdr->e_phoff;
+#define phdr(i) ((Elf32_Phdr *) (phdr_base + (i) * ehdr->e_phentsize))
+
+ requested_addr = phdr(0)->p_paddr;
+ playground_size = (phdr(ehdr->e_phnum - 1)->p_paddr + phdr(ehdr->e_phnum - 1)->p_memsz) - phdr(0)->p_paddr;
+ playground = grub_protected_malloc (playground_size, requested_addr, playground_size); /* FIXME: memleak */
+
/* Load every loadable segment in memory. */
for (i = 0; i < ehdr->e_phnum; i++)
{
- phdr = (Elf32_Phdr *) ((char *) buffer + ehdr->e_phoff
- + i * ehdr->e_phentsize);
- if (phdr->p_type == PT_LOAD)
+ if (phdr(i)->p_type == PT_LOAD)
{
- /* The segment should fit in the area reserved for the OS. */
- if ((phdr->p_paddr < grub_os_area_addr)
- || (phdr->p_paddr + phdr->p_memsz
- > grub_os_area_addr + grub_os_area_size))
- return grub_error (GRUB_ERR_BAD_OS,
- "segment doesn't fit in memory reserved for the OS");
+ void *playground_addr = playground + (phdr(i)->p_paddr - phdr(0)->p_paddr);
- if (grub_file_seek (file, (grub_off_t) phdr->p_offset)
+ if (grub_file_seek (file, (grub_off_t) phdr(i)->p_offset)
== (grub_off_t) -1)
return grub_error (GRUB_ERR_BAD_OS,
"invalid offset in program header");
- if (grub_file_read (file, (void *) phdr->p_paddr, phdr->p_filesz)
- != (grub_ssize_t) phdr->p_filesz)
+ if (grub_file_read (file, playground_addr, phdr(i)->p_filesz)
+ != (grub_ssize_t) phdr(i)->p_filesz)
return grub_error (GRUB_ERR_BAD_OS,
"couldn't read segment from file");
- if (phdr->p_filesz < phdr->p_memsz)
- grub_memset ((char *) phdr->p_paddr + phdr->p_filesz, 0,
- phdr->p_memsz - phdr->p_filesz);
+ if (phdr(i)->p_filesz < phdr(i)->p_memsz)
+ grub_memset ((char *) playground_addr + phdr(i)->p_filesz, 0,
+ phdr(i)->p_memsz - phdr(i)->p_filesz);
}
}
+
+#undef phdr
return grub_errno;
}
@@ -293,7 +333,7 @@
if (grub_multiboot_load_elf (file, buffer) != GRUB_ERR_NONE)
goto fail;
- mbi = grub_malloc (sizeof (struct grub_multiboot_info));
+ mbi = grub_protected_malloc (sizeof (struct grub_multiboot_info), requested_addr, playground_size);
if (! mbi)
goto fail;
@@ -306,7 +346,7 @@
for (i = 0, len = 0; i < argc; i++)
len += grub_strlen (argv[i]) + 1;
- cmdline = p = grub_malloc (len);
+ cmdline = p = grub_protected_malloc (len, requested_addr, playground_size);
if (! cmdline)
goto fail;
@@ -379,7 +419,7 @@
for (i = 0; i < argc; i++)
len += grub_strlen (argv[i]) + 1;
- cmdline = p = grub_malloc (len);
+ cmdline = p = grub_protected_malloc (len, requested_addr, playground_size);
if (! cmdline)
goto fail;
@@ -410,7 +450,7 @@
}
else
{
- struct grub_mod_list *modlist = grub_malloc (sizeof (struct grub_mod_list));
+ struct grub_mod_list *modlist = grub_protected_malloc (sizeof (struct grub_mod_list), requested_addr, playground_size);
if (! modlist)
goto fail;
modlist->mod_start = (grub_uint32_t) module;
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
