fix multiboot/aout, cleanup relocators, etc
PATCH 1/1 fix multiboot/aout, cleanup relocators, etc
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!
Signature · 5 lines
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."
i386/loader.S +37 -4
@@ -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.
grub/i386/loader.h +7 -0
@@ -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 */
i386/pc/multiboot.c +54 -52
142 lines not on this page · raw message
Quoted 7 lines · Robert Millan
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_bootfunction, and hence my code was trashing %eax.
Quoted 5 lines
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!
I think I should ellaborate a bit on this:
Quoted 33 lines
if (header->flags & MULTIBOOT_AOUT_KLUDGE) {i386/pc/multiboot.c +19
-10-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);
i386/pc/multiboot.c line 414
Quoted 10 lines
i386/pc/multiboot.c +9
-0+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; + }
The problem I had here is that grub_aout_load() was constrained to the api used by our BSD loaders, and the relocator approach requires to intermangle things, so I had to move it to multiboot.c and transform it.
Splitting it to a separate function (like we do for multiboot/ELF) wouldn't be hard if this is seen as a good idea, but making it global wouldn't be so sound IMHO: it relies on the relocator variables, so one would have to implement relocators in each a.out loader, which doesn't sound like such a bad idea, but the relocator code itself is biased towards multiboot (written to preserve %eax and %ebx), and so it's not as generic as it looks, as other load protocols might have conflicting constraints.
Signature · 5 lines
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."
Committed.
Quoted 251 lines · Robert Millan
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_bootfunction, 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!
Signature · 8 lines
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 ===================================================================i386/loader.S +37
-4@@ -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
===================================================================grub/i386/loader.h +7
-0@@ -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
===================================================================i386/pc/multiboot.c +54
-52@@ -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
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel
Signature · 5 lines
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."