Re: [PATCH] fix for loading modules from read-only memory area (Re: clean patch for i386-qemu port (Re: [PATCH] i386-qemu port))

[Pavel Roskin <proski@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 977 bytes --]

On Fri, 2009-06-26 at 19:43 +0200, Robert Millan wrote:
> On Fri, Jun 26, 2009 at 01:16:57PM -0400, Pavel Roskin wrote:
> > > But as for 02-elf-renames.patch we could try to make this more consistent.  In
> > > efiemu I used 'W' (inspired by ElfW() macro in glibc), then in some places you
> > > use 'K' and in other places 'T'.  Can we use the same everywhere?  I don't
> > > care which letter, I'm not THAT much into bikeshed :-)
> > 
> > "T" stands for "target" and "K" stands for "kernel".  Yes, we can use
> > ElfW everywhere.  But I would try to avoid any mass renames.  It's very
> > easy to write Elf instead of ElfW somewhere in the code, and it would
> > compile, but it won't work.
> 
> Ok, I don't mind.  Unless you have any objections, I'll merge your two
> patches with mine and commit that.

Please consider following patches.  They avoid the massive rename.  ELF
header reorganization is split from adding support for modules in ROM.

-- 
Regards,
Pavel Roskin

[-- Attachment #2: 01-select-elf.patch --]
[-- Type: text/x-patch, Size: 6081 bytes --]

Select default ELF format in include/grub/elf.h

From: Pavel Roskin <proski@gnu.org>

ChangeLog:

	* include/grub/elf.h: Define symbols for 32-bit or 64-bit ELF
	based on DEFAULT_ELF_BITS.  Default to the target word size.
	* efiemu/loadcore32.c: Use DEFAULT_ELF_BITS instead of own
	definitions.
	* efiemu/loadcore64.c: Likewise.
	* loader/i386/bsd32.c: Likewise.
	* loader/i386/bsd64.c: Likewise.
	* kern/dl.c: Remove ELF definitions.
	* util/i386/efi/grub-mkimage.c: Likewise.
---

 efiemu/loadcore32.c          |    7 +------
 efiemu/loadcore64.c          |    7 +------
 include/grub/elf.h           |   44 ++++++++++++++++++++++++++++++++++++++++++
 kern/dl.c                    |   24 -----------------------
 loader/i386/bsd32.c          |    4 +---
 loader/i386/bsd64.c          |    4 +---
 util/i386/efi/grub-mkimage.c |   34 --------------------------------
 7 files changed, 48 insertions(+), 76 deletions(-)


diff --git a/efiemu/loadcore32.c b/efiemu/loadcore32.c
index b4f61c7..4e0fcc5 100644
--- a/efiemu/loadcore32.c
+++ b/efiemu/loadcore32.c
@@ -18,10 +18,5 @@
  */
 
 #define SUFFIX(x) x ## 32
-#define Elf_Ehdr Elf32_Ehdr
-#define Elf_Shdr Elf32_Shdr
-#define Elf_Sym Elf32_Sym
-#define Elf_Word Elf32_Word
-#define ELF_ST_TYPE ELF32_ST_TYPE
-#define ELF_ST_BIND ELF32_ST_BIND
+#define DEFAULT_ELF_BITS 32
 #include "loadcore.c"
diff --git a/efiemu/loadcore64.c b/efiemu/loadcore64.c
index 097276c..3c5cc54 100644
--- a/efiemu/loadcore64.c
+++ b/efiemu/loadcore64.c
@@ -18,10 +18,5 @@
  */
 
 #define SUFFIX(x) x ## 64
-#define Elf_Ehdr Elf64_Ehdr
-#define Elf_Shdr Elf64_Shdr
-#define Elf_Sym Elf64_Sym
-#define Elf_Word Elf64_Word
-#define ELF_ST_TYPE ELF64_ST_TYPE
-#define ELF_ST_BIND ELF64_ST_BIND
+#define DEFAULT_ELF_BITS 64
 #include "loadcore.c"
diff --git a/include/grub/elf.h b/include/grub/elf.h
index 319bc7c..23a8415 100644
--- a/include/grub/elf.h
+++ b/include/grub/elf.h
@@ -2330,4 +2330,48 @@ typedef Elf32_Addr Elf32_Conflict;
 
 #define R_X86_64_NUM		24
 
+#ifndef DEFAULT_ELF_BITS
+#define DEFAULT_ELF_BITS (8 * GRUB_TARGET_SIZEOF_VOID_P)
+#endif
+
+#if DEFAULT_ELF_BITS == 32
+
+typedef Elf32_Addr Elf_Addr;
+typedef Elf32_Ehdr Elf_Ehdr;
+typedef Elf32_Half Elf_Half;
+typedef Elf32_Off Elf_Off;
+typedef Elf32_Rel Elf_Rel;
+typedef Elf32_Rela Elf_Rela;
+typedef Elf32_Section Elf_Section;
+typedef Elf32_Shdr Elf_Shdr;
+typedef Elf32_Sym Elf_Sym;
+typedef Elf32_Word Elf_Word;
+
+#define ELF_ST_BIND(val)	ELF32_ST_BIND(val)
+#define ELF_ST_TYPE(val)	ELF32_ST_TYPE(val)
+#define ELF_R_SYM(val)		ELF32_R_SYM(val)
+#define ELF_R_TYPE(val)		ELF32_R_TYPE(val)
+#define ELF_R_INFO(sym, type)	ELF32_R_INFO(sym, type)
+
+#elif DEFAULT_ELF_BITS == 64
+
+typedef Elf64_Addr Elf_Addr;
+typedef Elf64_Ehdr Elf_Ehdr;
+typedef Elf64_Half Elf_Half;
+typedef Elf64_Off Elf_Off;
+typedef Elf64_Rel Elf_Rel;
+typedef Elf64_Rela Elf_Rela;
+typedef Elf64_Section Elf_Section;
+typedef Elf64_Shdr Elf_Shdr;
+typedef Elf64_Sym Elf_Sym;
+typedef Elf64_Word Elf_Word;
+
+#define ELF_ST_BIND(val)	ELF64_ST_BIND (val)
+#define ELF_ST_TYPE(val)	ELF64_ST_TYPE (val)
+#define ELF_R_SYM(val)		ELF64_R_SYM(val)
+#define ELF_R_TYPE(val)		ELF64_R_TYPE(val)
+#define ELF_R_INFO(sym, type)	ELF64_R_INFO(sym, type)
+
+#endif /* DEFAULT_ELF_BITS == 64 */
+
 #endif /* ! GRUB_ELF_H */
diff --git a/kern/dl.c b/kern/dl.c
index d729c08..cd34e31 100644
--- a/kern/dl.c
+++ b/kern/dl.c
@@ -29,30 +29,6 @@
 #include <grub/env.h>
 #include <grub/cache.h>
 
-#if GRUB_CPU_SIZEOF_VOID_P == 4
-
-typedef Elf32_Word Elf_Word;
-typedef Elf32_Addr Elf_Addr;
-typedef Elf32_Ehdr Elf_Ehdr;
-typedef Elf32_Shdr Elf_Shdr;
-typedef Elf32_Sym Elf_Sym;
-
-# define ELF_ST_BIND(val)	ELF32_ST_BIND (val)
-# define ELF_ST_TYPE(val)	ELF32_ST_TYPE (val)
-
-#elif GRUB_CPU_SIZEOF_VOID_P == 8
-
-typedef Elf64_Word Elf_Word;
-typedef Elf64_Addr Elf_Addr;
-typedef Elf64_Ehdr Elf_Ehdr;
-typedef Elf64_Shdr Elf_Shdr;
-typedef Elf64_Sym Elf_Sym;
-
-# define ELF_ST_BIND(val)	ELF64_ST_BIND (val)
-# define ELF_ST_TYPE(val)	ELF64_ST_TYPE (val)
-
-#endif
-
 \f
 
 struct grub_dl_list
diff --git a/loader/i386/bsd32.c b/loader/i386/bsd32.c
index 24dab6c..a4fa48c 100644
--- a/loader/i386/bsd32.c
+++ b/loader/i386/bsd32.c
@@ -1,7 +1,5 @@
 #define SUFFIX(x) x ## 32
-#define Elf_Ehdr Elf32_Ehdr
-#define Elf_Shdr Elf32_Shdr
-#define Elf_Sym Elf32_Sym
+#define DEFAULT_ELF_BITS 32
 #define OBJSYM 0
 #include <grub/types.h>
 typedef grub_uint32_t grub_freebsd_addr_t;
diff --git a/loader/i386/bsd64.c b/loader/i386/bsd64.c
index f4ff8b2..8e3eba9 100644
--- a/loader/i386/bsd64.c
+++ b/loader/i386/bsd64.c
@@ -1,7 +1,5 @@
 #define SUFFIX(x) x ## 64
-#define Elf_Ehdr Elf64_Ehdr
-#define Elf_Shdr Elf64_Shdr
-#define Elf_Sym Elf64_Sym
+#define DEFAULT_ELF_BITS 64
 #define OBJSYM 1
 #include <grub/types.h>
 typedef grub_uint64_t grub_freebsd_addr_t;
diff --git a/util/i386/efi/grub-mkimage.c b/util/i386/efi/grub-mkimage.c
index 2813e79..a92e6da 100644
--- a/util/i386/efi/grub-mkimage.c
+++ b/util/i386/efi/grub-mkimage.c
@@ -32,43 +32,9 @@
 #include <grub/machine/kernel.h>
 
 #if GRUB_TARGET_SIZEOF_VOID_P == 4
-
-typedef Elf32_Word Elf_Word;
-typedef Elf32_Addr Elf_Addr;
-typedef Elf32_Ehdr Elf_Ehdr;
-typedef Elf32_Shdr Elf_Shdr;
-typedef Elf32_Sym Elf_Sym;
-typedef Elf32_Half Elf_Half;
-typedef Elf32_Off Elf_Off;
-typedef Elf32_Section Elf_Section;
-typedef Elf32_Rel Elf_Rel;
-typedef Elf32_Rela Elf_Rela;
-
-#define ELF_R_SYM	ELF32_R_SYM
-#define ELF_R_TYPE	ELF32_R_TYPE
-#define ELF_R_INFO	ELF32_R_INFO
-
 #define grub_le_to_cpu	grub_le_to_cpu32
-
 #elif GRUB_TARGET_SIZEOF_VOID_P == 8
-
-typedef Elf64_Word Elf_Word;
-typedef Elf64_Addr Elf_Addr;
-typedef Elf64_Ehdr Elf_Ehdr;
-typedef Elf64_Shdr Elf_Shdr;
-typedef Elf64_Sym Elf_Sym;
-typedef Elf64_Half Elf_Half;
-typedef Elf64_Off Elf_Off;
-typedef Elf64_Section Elf_Section;
-typedef Elf64_Rel Elf_Rel;
-typedef Elf64_Rela Elf_Rela;
-
-#define ELF_R_SYM	ELF64_R_SYM
-#define ELF_R_TYPE	ELF64_R_TYPE
-#define ELF_R_INFO	ELF64_R_INFO
-
 #define grub_le_to_cpu	grub_le_to_cpu64
-
 #endif
 
 static const grub_uint8_t stub[] = GRUB_PE32_MSDOS_STUB;

[-- Attachment #3: 02-module-ro.patch --]
[-- Type: text/x-patch, Size: 7207 bytes --]

Allow modules in ROM

From: Pavel Roskin <proski@gnu.org>

ChangeLog:

	* include/grub/dl.h: Include grub/elf.h.
	(struct grub_dl): Add symtab field.
	* kern/dl.c [GRUB_MACHINE_QEMU]: Define
	GRUB_MODULES_MACHINE_READONLY.
	(grub_dl_resolve_symbols): Populate mod->symtab, making a copy
	of the header for read-only modules.
	(grub_dl_unload): Free mod->symtab for read-only modules.
	* kern/i386/dl.c: Use mod->symtab.
	* kern/powerpc/dl.c: Likewise.
	* kern/sparc64/dl.c: Likewise.
	* kern/x86_64/dl.c: Likewise.
---

 include/grub/dl.h |    4 +++-
 kern/dl.c         |   19 +++++++++++++++++--
 kern/i386/dl.c    |    4 +---
 kern/powerpc/dl.c |    4 +---
 kern/sparc64/dl.c |    4 +---
 kern/x86_64/dl.c  |    4 +---
 6 files changed, 24 insertions(+), 15 deletions(-)


diff --git a/include/grub/dl.h b/include/grub/dl.h
index 894da1d..bebb810 100644
--- a/include/grub/dl.h
+++ b/include/grub/dl.h
@@ -1,7 +1,7 @@
 /* dl.h - types and prototypes for loadable module support */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2004,2005,2007,2008  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2004,2005,2007,2008,2009  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -23,6 +23,7 @@
 #include <grub/symbol.h>
 #include <grub/err.h>
 #include <grub/types.h>
+#include <grub/elf.h>
 
 #define GRUB_MOD_INIT(name)	\
 static void grub_mod_init (grub_dl_t mod __attribute__ ((unused))) __attribute__ ((used)); \
@@ -78,6 +79,7 @@ struct grub_dl
   int ref_count;
   grub_dl_dep_t dep;
   grub_dl_segment_t segment;
+  Elf_Sym *symtab;
   void (*init) (struct grub_dl *mod);
   void (*fini) (void);
 };
diff --git a/kern/dl.c b/kern/dl.c
index cd34e31..af4e5aa 100644
--- a/kern/dl.c
+++ b/kern/dl.c
@@ -1,7 +1,7 @@
 /* dl.c - loadable module support */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2003,2004,2005,2007,2008  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2003,2004,2005,2007,2008,2009  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -28,6 +28,12 @@
 #include <grub/file.h>
 #include <grub/env.h>
 #include <grub/cache.h>
+#include <grub/machine/machine.h>
+
+/* Platforms where modules are in a readonly area of memory.  */
+#if defined(GRUB_MACHINE_QEMU)
+#define GRUB_MODULES_MACHINE_READONLY
+#endif
 
 \f
 
@@ -309,7 +315,13 @@ grub_dl_resolve_symbols (grub_dl_t mod, Elf_Ehdr *e)
   if (i == e->e_shnum)
     return grub_error (GRUB_ERR_BAD_MODULE, "no symbol table");
 
-  sym = (Elf_Sym *) ((char *) e + s->sh_offset);
+#ifdef GRUB_MODULES_MACHINE_READONLY
+  mod->symtab = grub_malloc (s->sh_size);
+  memcpy (mod->symtab, (char *) e + s->sh_offset, s->sh_size);
+#else
+  mod->symtab = (Elf_Sym *) ((char *) e + s->sh_offset);
+#endif
+  sym = mod->symtab;
   size = s->sh_size;
   entsize = s->sh_entsize;
 
@@ -671,6 +683,9 @@ grub_dl_unload (grub_dl_t mod)
     }
 
   grub_free (mod->name);
+#ifdef GRUB_MODULES_MACHINE_READONLY
+  grub_free (mod->symtab);
+#endif
   grub_free (mod);
   return 1;
 }
diff --git a/kern/i386/dl.c b/kern/i386/dl.c
index 978bfb1..a17f175 100644
--- a/kern/i386/dl.c
+++ b/kern/i386/dl.c
@@ -43,7 +43,6 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
 {
   Elf32_Ehdr *e = ehdr;
   Elf32_Shdr *s;
-  Elf32_Sym *symtab;
   Elf32_Word entsize;
   unsigned i;
 
@@ -57,7 +56,6 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
   if (i == e->e_shnum)
     return grub_error (GRUB_ERR_BAD_MODULE, "no symtab found");
 
-  symtab = (Elf32_Sym *) ((char *) e + s->sh_offset);
   entsize = s->sh_entsize;
 
   for (i = 0, s = (Elf32_Shdr *) ((char *) e + e->e_shoff);
@@ -89,7 +87,7 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
 				     "reloc offset is out of the segment");
 
 		addr = (Elf32_Word *) ((char *) seg->addr + rel->r_offset);
-		sym = (Elf32_Sym *) ((char *) symtab
+		sym = (Elf32_Sym *) ((char *) mod->symtab
 				     + entsize * ELF32_R_SYM (rel->r_info));
 
 		switch (ELF32_R_TYPE (rel->r_info))
diff --git a/kern/powerpc/dl.c b/kern/powerpc/dl.c
index ae987c0..9b1cb14 100644
--- a/kern/powerpc/dl.c
+++ b/kern/powerpc/dl.c
@@ -44,7 +44,6 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
 {
   Elf32_Ehdr *e = ehdr;
   Elf32_Shdr *s;
-  Elf32_Sym *symtab;
   Elf32_Word entsize;
   unsigned i;
 
@@ -58,7 +57,6 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
   if (i == e->e_shnum)
     return grub_error (GRUB_ERR_BAD_MODULE, "no symtab found");
 
-  symtab = (Elf32_Sym *) ((char *) e + s->sh_offset);
   entsize = s->sh_entsize;
 
   for (i = 0, s = (Elf32_Shdr *) ((char *) e + e->e_shoff);
@@ -91,7 +89,7 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
 				     "reloc offset is out of the segment");
 
 		addr = (Elf32_Word *) ((char *) seg->addr + rel->r_offset);
-		sym = (Elf32_Sym *) ((char *) symtab
+		sym = (Elf32_Sym *) ((char *) mod->symtab
 				     + entsize * ELF32_R_SYM (rel->r_info));
 
 		/* On the PPC the value does not have an explicit
diff --git a/kern/sparc64/dl.c b/kern/sparc64/dl.c
index d998daf..5334896 100644
--- a/kern/sparc64/dl.c
+++ b/kern/sparc64/dl.c
@@ -44,7 +44,6 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
 {
   Elf64_Ehdr *e = ehdr;
   Elf64_Shdr *s;
-  Elf64_Sym *symtab;
   Elf64_Word entsize;
   unsigned i;
 
@@ -58,7 +57,6 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
   if (i == e->e_shnum)
     return grub_error (GRUB_ERR_BAD_MODULE, "no symtab found");
 
-  symtab = (Elf64_Sym *) ((char *) e + s->sh_offset);
   entsize = s->sh_entsize;
 
   for (i = 0, s = (Elf64_Shdr *) ((char *) e + e->e_shoff);
@@ -91,7 +89,7 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
 				     "reloc offset is out of the segment");
 
 		addr = (Elf64_Word *) ((char *) seg->addr + rel->r_offset);
-		sym = (Elf64_Sym *) ((char *) symtab
+		sym = (Elf64_Sym *) ((char *) mod->symtab
 				     + entsize * ELF64_R_SYM (rel->r_info));
 
 		value = sym->st_value + rel->r_addend;
diff --git a/kern/x86_64/dl.c b/kern/x86_64/dl.c
index a606901..9e5de6e 100644
--- a/kern/x86_64/dl.c
+++ b/kern/x86_64/dl.c
@@ -43,7 +43,6 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
 {
   Elf64_Ehdr *e = ehdr;
   Elf64_Shdr *s;
-  Elf64_Sym *symtab;
   Elf64_Word entsize;
   unsigned i;
 
@@ -57,7 +56,6 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
   if (i == e->e_shnum)
     return grub_error (GRUB_ERR_BAD_MODULE, "no symtab found");
 
-  symtab = (Elf64_Sym *) ((char *) e + s->sh_offset);
   entsize = s->sh_entsize;
 
   for (i = 0, s = (Elf64_Shdr *) ((char *) e + e->e_shoff);
@@ -91,7 +89,7 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
 
 		addr32 = (Elf64_Word *) ((char *) seg->addr + rel->r_offset);
 		addr64 = (Elf64_Xword *) addr32;
-		sym = (Elf64_Sym *) ((char *) symtab
+		sym = (Elf64_Sym *) ((char *) mod->symtab
 				     + entsize * ELF64_R_SYM (rel->r_info));
 
 		switch (ELF64_R_TYPE (rel->r_info))