Re: [PATCH] fix multiboot/aout, cleanup relocators, etc

[Robert Millan]

Committed.

On Thu, Aug 07, 2008 at 01:27:53AM +0200, Robert Millan wrote:
> 
> Hi,
> 
> It seems when adding the relocators for ELF, I failed to notice both ELF and
> a.out loaders share the same grub_multiboot_real_boot function, and hence my
> code was trashing %eax.
> 
> Since this functionality is useful for the a.out loader anyway, I decided to
> implement it as well.  In the process I did some cleanup, mostly to reduce
> code duplication among both users of the relocators.
> 
> So here's the patch.  Please comment!
> 
> -- 
> 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."

> Index: kern/i386/loader.S
> ===================================================================
> --- kern/i386/loader.S        (revision 1787)
> +++ kern/i386/loader.S        (working copy)
> @@ -132,6 +132,36 @@
>  VARIABLE(grub_multiboot_payload_entry_offset)
>       .long   0
>  
> +/*
> + * The relocators below understand the following parameters:
> + * ecx:      Size of the block to be copied.
> + * esi:      Where to copy from (always lowest address, even if we're 
> relocating
> + *      backwards).
> + * edi:      Where to copy to (likewise).
> + * edx:      Offset of the entry point (relative to the beginning of the 
> block).
> + */
> +VARIABLE(grub_multiboot_forward_relocator)
> +     cld
> +     addl    %edi, %edx
> +     rep
> +     movsb
> +     jmp     *%edx
> +VARIABLE(grub_multiboot_forward_relocator_end)
> +
> +VARIABLE(grub_multiboot_backward_relocator)
> +     std
> +     addl    %ecx, %esi
> +     addl    %ecx, %edi
> +     /* backward movsb is implicitly off-by-one.  compensate that. */
> +     incl    %ecx
> +     rep
> +     movsb
> +     /* same problem again. */
> +     incl    %edi
> +     addl    %edi, %edx
> +     jmp     *%edx
> +VARIABLE(grub_multiboot_backward_relocator_end)
> +
>  FUNCTION(grub_multiboot_real_boot)
>       /* Push the entry address on the stack.  */
>       pushl   %eax
> @@ -149,11 +179,14 @@
>       movl    EXT_C(grub_multiboot_payload_size), %ecx
>       movl    EXT_C(grub_multiboot_payload_orig), %esi
>       movl    EXT_C(grub_multiboot_payload_dest), %edi
> -     movl    EXT_C(grub_multiboot_payload_entry_offset), %eax
> -                     
> +     movl    EXT_C(grub_multiboot_payload_entry_offset), %edx
> +
> +     /* Move the magic value into eax.  */
> +     movl    $MULTIBOOT_MAGIC2, %eax
> +                             
>       /* Jump to the relocator.  */
> -     popl    %edx
> -     jmp     *%edx
> +     popl    %ebp
> +     jmp     *%ebp
>       
>  /*
>   * This starts the multiboot 2 kernel.
> Index: include/grub/i386/loader.h
> ===================================================================
> --- include/grub/i386/loader.h        (revision 1787)
> +++ include/grub/i386/loader.h        (working copy)
> @@ -53,4 +53,11 @@
>  void grub_rescue_cmd_linux (int argc, char *argv[]);
>  void grub_rescue_cmd_initrd (int argc, char *argv[]);
>  
> +extern grub_uint8_t EXPORT_VAR(grub_multiboot_forward_relocator);
> +extern grub_uint8_t EXPORT_VAR(grub_multiboot_forward_relocator_end);
> +extern grub_uint8_t EXPORT_VAR(grub_multiboot_backward_relocator);
> +extern grub_uint8_t EXPORT_VAR(grub_multiboot_backward_relocator_end);
> +
> +#define RELOCATOR_SIZEOF(x)  (&grub_multiboot_##x##_relocator_end - 
> &grub_multiboot_##x##_relocator)
> +
>  #endif /* ! GRUB_LOADER_CPU_HEADER */
> Index: loader/i386/pc/multiboot.c
> ===================================================================
> --- loader/i386/pc/multiboot.c        (revision 1787)
> +++ loader/i386/pc/multiboot.c        (working copy)
> @@ -52,31 +52,6 @@
>  
>  static char *playground = NULL;
>  
> -static grub_uint8_t forward_relocator[] =
> -{
> -  0xfc,                              /* cld */
> -  0x89, 0xf2,                        /* movl %esi, %edx */
> -  0xf3, 0xa4,                        /* rep movsb */
> -  0x01, 0xc2,                        /* addl %eax, %edx */
> -  0xb8, 0x02, 0xb0, 0xad, 0x2b,      /* movl $MULTIBOOT_MAGIC2, %eax */
> -  0xff, 0xe2,                        /* jmp *%edx */
> -};
> -
> -static grub_uint8_t backward_relocator[] =
> -{
> -  0xfd,                              /* std */
> -  0x01, 0xce,                        /* addl %ecx, %esi */
> -  0x01, 0xcf,                        /* addl %ecx, %edi */
> -                             /* backward movsb is implicitly off-by-one.  
> compensate that. */
> -  0x41,                              /* incl %ecx */
> -  0xf3, 0xa4,                        /* rep movsb */
> -                             /* same problem again. */
> -  0x47,                              /* incl %edi */
> -  0x01, 0xc7,                        /* addl %eax, %edi */
> -  0xb8, 0x02, 0xb0, 0xad, 0x2b,      /* movl $MULTIBOOT_MAGIC2, %eax */
> -  0xff, 0xe7,                        /* jmp *%edi */
> -};
> -
>  static grub_err_t
>  grub_multiboot_boot (void)
>  {
> @@ -155,17 +130,12 @@
>    grub_multiboot_payload_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;
>  
> -  if (playground)
> -    grub_free (playground);
> -  playground = grub_malloc (sizeof (forward_relocator) + 
> grub_multiboot_payload_size + sizeof (backward_relocator));
> +  playground = grub_malloc (RELOCATOR_SIZEOF(forward) + 
> grub_multiboot_payload_size + RELOCATOR_SIZEOF(backward));
>    if (! playground)
>      return grub_errno;
>  
> -  grub_multiboot_payload_orig = (long) playground + sizeof 
> (forward_relocator);
> +  grub_multiboot_payload_orig = (long) playground + 
> RELOCATOR_SIZEOF(forward);
>  
> -  grub_memmove (playground, forward_relocator, sizeof (forward_relocator));
> -  grub_memmove ((char *) (grub_multiboot_payload_orig + 
> grub_multiboot_payload_size), backward_relocator, sizeof 
> (backward_relocator));
> -
>    /* Load every loadable segment in memory.  */
>    for (i = 0; i < ehdr->e_phnum; i++)
>      {
> @@ -196,16 +166,6 @@
>  
>  #undef phdr
>  
> -  if (grub_multiboot_payload_dest >= grub_multiboot_payload_orig)
> -    entry = (grub_addr_t) playground;
> -  else
> -    entry = (grub_addr_t) grub_multiboot_payload_orig + 
> grub_multiboot_payload_size;
> -
> -  grub_dprintf ("multiboot_loader", "dest=%p, size=0x%x, 
> entry_offset=0x%x\n",
> -             (void *) grub_multiboot_payload_dest,
> -             grub_multiboot_payload_size,
> -             grub_multiboot_payload_entry_offset);
> -
>    return grub_errno;
>  }
>  
> @@ -413,24 +373,66 @@
>        goto fail;
>      }
>  
> +  if (playground)
> +    {
> +      grub_free (playground);
> +      playground = NULL;
> +    }
> +
>    if (header->flags & MULTIBOOT_AOUT_KLUDGE)
>      {
> -      int ofs;
> +      int offset = ((char *) header - buffer -
> +                 (header->header_addr - header->load_addr));
> +      int load_size = ((header->load_end_addr == 0) ? file->size - offset :
> +                    header->load_end_addr - header->load_addr);
>  
> -      ofs = (char *) header - buffer -
> -            (header->header_addr - header->load_addr);
> -      if ((grub_aout_load (file, ofs, header->load_addr,
> -                           ((header->load_end_addr == 0) ? 0 :
> -                            header->load_end_addr - header->load_addr),
> -                           header->bss_end_addr))
> -          !=GRUB_ERR_NONE)
> -        goto fail;
> +      if (header->bss_end_addr)
> +     grub_multiboot_payload_size = (header->bss_end_addr - 
> header->load_addr);
> +      else
> +     grub_multiboot_payload_size = load_size;
> +      grub_multiboot_payload_dest = header->load_addr;
>  
> -      entry = header->entry_addr;
> +      playground = grub_malloc (RELOCATOR_SIZEOF(forward) + 
> grub_multiboot_payload_size + RELOCATOR_SIZEOF(backward));
> +      if (! playground)
> +     goto fail;
> +
> +      grub_multiboot_payload_orig = (long) playground + 
> RELOCATOR_SIZEOF(forward);
> +
> +      if ((grub_file_seek (file, offset)) == (grub_off_t) - 1)
> +     goto fail;
> +
> +      grub_file_read (file, grub_multiboot_payload_orig, load_size);
> +      if (grub_errno)
> +     goto fail;
> +      
> +      if (header->bss_end_addr)
> +     grub_memset (grub_multiboot_payload_orig + load_size, 0,
> +                  header->bss_end_addr - header->load_addr - load_size);
> +      
> +      grub_multiboot_payload_entry_offset = header->entry_addr - 
> header->load_addr;
> +
>      }
>    else if (grub_multiboot_load_elf (file, buffer) != GRUB_ERR_NONE)
>      goto fail;
>  
> +      
> +  if (grub_multiboot_payload_dest >= grub_multiboot_payload_orig)
> +    {
> +      grub_memmove (playground, &grub_multiboot_forward_relocator, 
> RELOCATOR_SIZEOF(forward));
> +      entry = (grub_addr_t) playground;
> +    }
> +  else
> +    {
> +      grub_memmove ((char *) (grub_multiboot_payload_orig + 
> grub_multiboot_payload_size),
> +                 &grub_multiboot_backward_relocator, 
> RELOCATOR_SIZEOF(backward));
> +      entry = (grub_addr_t) grub_multiboot_payload_orig + 
> grub_multiboot_payload_size;
> +    }
> +  
> +  grub_dprintf ("multiboot_loader", "dest=%p, size=0x%x, 
> entry_offset=0x%x\n",
> +             (void *) grub_multiboot_payload_dest,
> +             grub_multiboot_payload_size,
> +             grub_multiboot_payload_entry_offset);
> +
>    mbi = grub_malloc (sizeof (struct grub_multiboot_info));
>    if (! mbi)
>      goto fail;

> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
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."