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

[Robert Millan <rmh@aybabtu.com>]

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

On Thu, Jun 25, 2009 at 04:51:11PM -0400, Pavel Roskin wrote:
> 
> I'd like to avoid preprocessor conditionals in kern/ARCH/dl.c files if
> possible.

Alright, here's a new patch.  It follows your earlier suggestion to use
a wrapper to obtain the initialization address for symtab.  This way we
don't put any ifdefs in kern/ARCH/dl.c files.

I also avoid the #undefs in efiemu.  This required many changes in
loadcore.c, but they're just a sed search & replace run.

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

[-- Attachment #2: dl.diff --]
[-- Type: text/x-diff, Size: 14580 bytes --]

2009-06-26  Robert Millan  <rmh.grub@aybabtu.com>

	* kern/dl.c (Elf_Word, Elf_Addr, Elf_Ehdr, Elf_Shdr, Elf_Sym): Remove
	typedefs.
	(ELF_ST_BIND, ELF_ST_TYPE): Remove macros.
	* include/grub/elf.h (Elf_Word, Elf_Addr, Elf_Ehdr, Elf_Shdr)
	(Elf_Sym, ELF_ST_BIND, ELF_ST_TYPE): New macros.

	* efiemu/loadcore64.c: Include `<grub/elf.h>'.
	(Elf_Word, Elf_Ehdr, Elf_Shdr, Elf_Sym, ELF_ST_BIND)
	(ELF_ST_TYPE): Rename to ...
	(ElfW_Word, ElfW_Ehdr, ElfW_Shdr, ElfW_Sym, ELFW_ST_BIND)
	(ELFW_ST_TYPE): ... this.  Update all users.
	* efiemu/loadcore32.c: Likewise.

	* include/grub/dl.h: Include `<grub/elf.h>' and
	`<grub/machine/machine.h>'.
	[GRUB_MACHINE_QEMU] (GRUB_MODULES_MACHINE_READONLY): New macro.
	[GRUB_MODULES_MACHINE_READONLY] (struct grub_dl): Add `symtab' member.
	(get_symtab): New macro.

	* kern/dl.c [GRUB_MODULES_MACHINE_READONLY]
	(grub_dl_resolve_symbols): Copy symbol table to `mod->symtab', and use
	the copied structure for further writes	rather than the original.

	* kern/powerpc/dl.c (grub_arch_dl_relocate_symbols): Initialize
	`symtab' using get_symtab().
	* kern/sparc64/dl.c: Likewise.
	* kern/i386/dl.c: Likewise.
	* kern/x86_64/dl.c: Likewise.

Index: kern/powerpc/dl.c
===================================================================
--- kern/powerpc/dl.c	(revision 2358)
+++ kern/powerpc/dl.c	(working copy)
@@ -58,7 +58,7 @@ grub_arch_dl_relocate_symbols (grub_dl_t
   if (i == e->e_shnum)
     return grub_error (GRUB_ERR_BAD_MODULE, "no symtab found");
 
-  symtab = (Elf32_Sym *) ((char *) e + s->sh_offset);
+  symtab = get_symtab ();
   entsize = s->sh_entsize;
 
   for (i = 0, s = (Elf32_Shdr *) ((char *) e + e->e_shoff);
Index: kern/dl.c
===================================================================
--- kern/dl.c	(revision 2358)
+++ kern/dl.c	(working copy)
@@ -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
@@ -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
@@ -333,9 +309,13 @@ grub_dl_resolve_symbols (grub_dl_t mod, 
   if (i == e->e_shnum)
     return grub_error (GRUB_ERR_BAD_MODULE, "no symbol table");
 
-  sym = (Elf_Sym *) ((char *) e + s->sh_offset);
   size = s->sh_size;
   entsize = s->sh_entsize;
+#ifdef GRUB_MODULES_MACHINE_READONLY
+  mod->symtab = grub_malloc (size);
+  memcpy (mod->symtab, (char *) e + s->sh_offset, size);
+#endif
+  sym = get_symtab ();
 
   s = (Elf_Shdr *) ((char *) e + e->e_shoff + e->e_shentsize * s->sh_link);
   str = (char *) e + s->sh_offset;
@@ -695,6 +675,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;
 }
Index: kern/sparc64/dl.c
===================================================================
--- kern/sparc64/dl.c	(revision 2358)
+++ kern/sparc64/dl.c	(working copy)
@@ -58,7 +58,7 @@ grub_arch_dl_relocate_symbols (grub_dl_t
   if (i == e->e_shnum)
     return grub_error (GRUB_ERR_BAD_MODULE, "no symtab found");
 
-  symtab = (Elf64_Sym *) ((char *) e + s->sh_offset);
+  symtab = get_symtab ();
   entsize = s->sh_entsize;
 
   for (i = 0, s = (Elf64_Shdr *) ((char *) e + e->e_shoff);
Index: kern/i386/dl.c
===================================================================
--- kern/i386/dl.c	(revision 2358)
+++ kern/i386/dl.c	(working copy)
@@ -57,7 +57,7 @@ grub_arch_dl_relocate_symbols (grub_dl_t
   if (i == e->e_shnum)
     return grub_error (GRUB_ERR_BAD_MODULE, "no symtab found");
 
-  symtab = (Elf32_Sym *) ((char *) e + s->sh_offset);
+  symtab = get_symtab ();
   entsize = s->sh_entsize;
 
   for (i = 0, s = (Elf32_Shdr *) ((char *) e + e->e_shoff);
Index: kern/x86_64/dl.c
===================================================================
--- kern/x86_64/dl.c	(revision 2358)
+++ kern/x86_64/dl.c	(working copy)
@@ -57,7 +57,7 @@ grub_arch_dl_relocate_symbols (grub_dl_t
   if (i == e->e_shnum)
     return grub_error (GRUB_ERR_BAD_MODULE, "no symtab found");
 
-  symtab = (Elf64_Sym *) ((char *) e + s->sh_offset);
+  symtab = get_symtab ();
   entsize = s->sh_entsize;
 
   for (i = 0, s = (Elf64_Shdr *) ((char *) e + e->e_shoff);
Index: efiemu/loadcore64.c
===================================================================
--- efiemu/loadcore64.c	(revision 2358)
+++ efiemu/loadcore64.c	(working copy)
@@ -18,10 +18,14 @@
  */
 
 #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
+
+#include <grub/elf.h>
+
+#define ElfW_Ehdr	Elf64_Ehdr
+#define ElfW_Shdr	Elf64_Shdr
+#define ElfW_Sym	Elf64_Sym
+#define ElfW_Word	Elf64_Word
+#define ELFW_ST_TYPE	ELF64_ST_TYPE
+#define ELFW_ST_BIND	ELF64_ST_BIND
+
 #include "loadcore.c"
Index: efiemu/loadcore.c
===================================================================
--- efiemu/loadcore.c	(revision 2358)
+++ efiemu/loadcore.c	(working copy)
@@ -59,10 +59,10 @@ SUFFIX (grub_efiemu_loadcore_unload) (vo
 int
 SUFFIX (grub_efiemu_check_header) (void *ehdr, grub_size_t size)
 {
-  Elf_Ehdr *e = ehdr;
+  ElfW_Ehdr *e = ehdr;
 
   /* Check the header size.  */
-  if (size < sizeof (Elf_Ehdr))
+  if (size < sizeof (ElfW_Ehdr))
     return 0;
 
   /* Check the magic numbers.  */
@@ -80,16 +80,16 @@ SUFFIX (grub_efiemu_check_header) (void 
 
 /* Load all segments from memory specified by E.  */
 static grub_err_t
-grub_efiemu_load_segments (grub_efiemu_segment_t segs, const Elf_Ehdr *e)
+grub_efiemu_load_segments (grub_efiemu_segment_t segs, const ElfW_Ehdr *e)
 {
-  Elf_Shdr *s;
+  ElfW_Shdr *s;
   grub_efiemu_segment_t cur;
 
   grub_dprintf ("efiemu", "loading segments\n");
 
   for (cur=segs; cur; cur = cur->next)
     {
-      s = (Elf_Shdr *)cur->srcptr;
+      s = (ElfW_Shdr *)cur->srcptr;
 
       if ((s->sh_flags & SHF_ALLOC) && s->sh_size)
 	{
@@ -115,14 +115,14 @@ grub_efiemu_load_segments (grub_efiemu_s
 
 /* Get a string at offset OFFSET from strtab */
 static char *
-grub_efiemu_get_string (unsigned offset, const Elf_Ehdr *e)
+grub_efiemu_get_string (unsigned offset, const ElfW_Ehdr *e)
 {
   unsigned i;
-  Elf_Shdr *s;
+  ElfW_Shdr *s;
 
-  for (i = 0, s = (Elf_Shdr *)((char *) e + e->e_shoff);
+  for (i = 0, s = (ElfW_Shdr *)((char *) e + e->e_shoff);
        i < e->e_shnum;
-       i++, s = (Elf_Shdr *)((char *) s + e->e_shentsize))
+       i++, s = (ElfW_Shdr *)((char *) s + e->e_shentsize))
     if (s->sh_type == SHT_STRTAB && offset < s->sh_size)
       return (char *) e + s->sh_offset + offset;
   return 0;
@@ -130,14 +130,14 @@ grub_efiemu_get_string (unsigned offset,
 
 /* Request memory for segments and fill segments info */
 static grub_err_t
-grub_efiemu_init_segments (grub_efiemu_segment_t *segs, const Elf_Ehdr *e)
+grub_efiemu_init_segments (grub_efiemu_segment_t *segs, const ElfW_Ehdr *e)
 {
   unsigned i;
-  Elf_Shdr *s;
+  ElfW_Shdr *s;
 
-  for (i = 0, s = (Elf_Shdr *)((char *) e + e->e_shoff);
+  for (i = 0, s = (ElfW_Shdr *)((char *) e + e->e_shoff);
        i < e->e_shnum;
-       i++, s = (Elf_Shdr *)((char *) s + e->e_shentsize))
+       i++, s = (ElfW_Shdr *)((char *) s + e->e_shentsize))
     {
       if (s->sh_flags & SHF_ALLOC)
 	{
@@ -180,16 +180,16 @@ grub_efiemu_init_segments (grub_efiemu_s
 
 /* Count symbols and relocators and allocate/request memory for them */
 static grub_err_t
-grub_efiemu_count_symbols (const Elf_Ehdr *e)
+grub_efiemu_count_symbols (const ElfW_Ehdr *e)
 {
   unsigned i;
-  Elf_Shdr *s;
+  ElfW_Shdr *s;
   int num = 0;
 
   /* Symbols */
-  for (i = 0, s = (Elf_Shdr *) ((char *) e + e->e_shoff);
+  for (i = 0, s = (ElfW_Shdr *) ((char *) e + e->e_shoff);
        i < e->e_shnum;
-       i++, s = (Elf_Shdr *) ((char *) s + e->e_shentsize))
+       i++, s = (ElfW_Shdr *) ((char *) s + e->e_shentsize))
     if (s->sh_type == SHT_SYMTAB)
       break;
 
@@ -201,9 +201,9 @@ grub_efiemu_count_symbols (const Elf_Ehd
     grub_malloc (sizeof (struct grub_efiemu_elf_sym) * grub_efiemu_nelfsyms);
 
   /* Relocators */
-  for (i = 0, s = (Elf_Shdr *) ((char *) e + e->e_shoff);
+  for (i = 0, s = (ElfW_Shdr *) ((char *) e + e->e_shoff);
        i < e->e_shnum;
-       i++, s = (Elf_Shdr *) ((char *) s + e->e_shentsize))
+       i++, s = (ElfW_Shdr *) ((char *) s + e->e_shentsize))
     if (s->sh_type == SHT_REL || s->sh_type == SHT_RELA)
       num += ((unsigned) s->sh_size) / ((unsigned) s->sh_entsize);
 
@@ -214,38 +214,38 @@ grub_efiemu_count_symbols (const Elf_Ehd
 
 /* Fill grub_efiemu_elfsyms with symbol values */
 static grub_err_t
-grub_efiemu_resolve_symbols (grub_efiemu_segment_t segs, Elf_Ehdr *e)
+grub_efiemu_resolve_symbols (grub_efiemu_segment_t segs, ElfW_Ehdr *e)
 {
   unsigned i;
-  Elf_Shdr *s;
-  Elf_Sym *sym;
+  ElfW_Shdr *s;
+  ElfW_Sym *sym;
   const char *str;
-  Elf_Word size, entsize;
+  ElfW_Word size, entsize;
 
   grub_dprintf ("efiemu", "resolving symbols\n");
 
-  for (i = 0, s = (Elf_Shdr *) ((char *) e + e->e_shoff);
+  for (i = 0, s = (ElfW_Shdr *) ((char *) e + e->e_shoff);
        i < e->e_shnum;
-       i++, s = (Elf_Shdr *) ((char *) s + e->e_shentsize))
+       i++, s = (ElfW_Shdr *) ((char *) s + e->e_shentsize))
     if (s->sh_type == SHT_SYMTAB)
       break;
 
   if (i == e->e_shnum)
     return grub_error (GRUB_ERR_BAD_OS, "no symbol table");
 
-  sym = (Elf_Sym *) ((char *) e + s->sh_offset);
+  sym = (ElfW_Sym *) ((char *) e + s->sh_offset);
   size = s->sh_size;
   entsize = s->sh_entsize;
 
-  s = (Elf_Shdr *) ((char *) e + e->e_shoff + e->e_shentsize * s->sh_link);
+  s = (ElfW_Shdr *) ((char *) e + e->e_shoff + e->e_shentsize * s->sh_link);
   str = (char *) e + s->sh_offset;
 
   for (i = 0;
        i < size / entsize;
-       i++, sym = (Elf_Sym *) ((char *) sym + entsize))
+       i++, sym = (ElfW_Sym *) ((char *) sym + entsize))
     {
-      unsigned char type = ELF_ST_TYPE (sym->st_info);
-      unsigned char bind = ELF_ST_BIND (sym->st_info);
+      unsigned char type = ELFW_ST_TYPE (sym->st_info);
+      unsigned char bind = ELFW_ST_BIND (sym->st_info);
       int handle;
       grub_off_t off;
       grub_err_t err;
@@ -325,7 +325,7 @@ grub_err_t
 SUFFIX (grub_efiemu_loadcore_init) (void *core, grub_size_t core_size,
 				    grub_efiemu_segment_t *segments)
 {
-  Elf_Ehdr *e = (Elf_Ehdr *) core;
+  ElfW_Ehdr *e = (ElfW_Ehdr *) core;
   grub_err_t err;
 
   if (e->e_type != ET_REL)
Index: efiemu/loadcore32.c
===================================================================
--- efiemu/loadcore32.c	(revision 2358)
+++ efiemu/loadcore32.c	(working copy)
@@ -18,10 +18,14 @@
  */
 
 #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
+
+#include <grub/elf.h>
+
+#define ElfW_Ehdr	Elf32_Ehdr
+#define ElfW_Shdr	Elf32_Shdr
+#define ElfW_Sym	Elf32_Sym
+#define ElfW_Word	Elf32_Word
+#define ELFW_ST_TYPE	ELF32_ST_TYPE
+#define ELFW_ST_BIND	ELF32_ST_BIND
+
 #include "loadcore.c"
Index: include/grub/elf.h
===================================================================
--- include/grub/elf.h	(revision 2358)
+++ include/grub/elf.h	(working copy)
@@ -1,5 +1,5 @@
 /* This file defines standard ELF types, structures, and macros.
-   Copyright (C) 1995-1999, 2000, 2001, 2002,2008 Free Software Foundation, Inc.
+   Copyright (C) 1995-1999,2000,2001,2002,2008,2009 Free Software Foundation, Inc.
    This file was part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -2330,4 +2330,28 @@ typedef Elf32_Addr Elf32_Conflict;
 
 #define R_X86_64_NUM		24
 
+#if GRUB_CPU_SIZEOF_VOID_P == 4
+
+#define Elf_Word Elf32_Word
+#define Elf_Addr Elf32_Addr
+#define Elf_Ehdr Elf32_Ehdr
+#define Elf_Shdr Elf32_Shdr
+#define Elf_Sym Elf32_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
+
+#define Elf_Word Elf64_Word
+#define Elf_Addr Elf64_Addr
+#define Elf_Ehdr Elf64_Ehdr
+#define Elf_Shdr Elf64_Shdr
+#define Elf_Sym Elf64_Sym
+
+# define ELF_ST_BIND(val)	ELF64_ST_BIND (val)
+# define ELF_ST_TYPE(val)	ELF64_ST_TYPE (val)
+
+#endif
+
 #endif /* ! GRUB_ELF_H */
Index: include/grub/dl.h
===================================================================
--- include/grub/dl.h	(revision 2358)
+++ include/grub/dl.h	(working copy)
@@ -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,13 @@
 #include <grub/symbol.h>
 #include <grub/err.h>
 #include <grub/types.h>
+#include <grub/elf.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
 
 #define GRUB_MOD_INIT(name)	\
 static void grub_mod_init (grub_dl_t mod __attribute__ ((unused))) __attribute__ ((used)); \
@@ -78,6 +85,12 @@ struct grub_dl
   int ref_count;
   grub_dl_dep_t dep;
   grub_dl_segment_t segment;
+#ifdef GRUB_MODULES_MACHINE_READONLY
+  Elf_Sym *symtab;
+#define get_symtab()	mod->symtab
+#else
+#define get_symtab()	((Elf_Sym *) ((char *) e + s->sh_offset))
+#endif
   void (*init) (struct grub_dl *mod);
   void (*fini) (void);
 };