Delay processing of ld.so's own PT_GNU_RELRO section until after environment

processing and malloc initialization, and then move variables set by those
steps, including malloc's mopts structure, into the .data.rel.ro segment.
This protects more data and eliminates the extra padding of the mopts.

ok kettenis@

5 files changed, 70 insertions, 86 deletions

diff --git a/libexec/ld.so/boot.c b/libexec/ld.so/boot.c
index a4ed06ea062..0205b7d3103 100644
--- a/libexec/ld.so/boot.c
+++ b/libexec/ld.so/boot.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: boot.c,v 1.15 2017/01/09 22:51:04 kettenis Exp $ */
+/*	$OpenBSD: boot.c,v 1.16 2018/10/23 04:01:45 guenther Exp $ */
 
 /*
  * Copyright (c) 1998 Per Fogelstrom, Opsycon AB
@@ -87,8 +87,6 @@ _dl_boot_bind(const long sp, long *dl_data, Elf_Dyn *dynp)
 	long		loff;
 	Elf_Addr	i;
 	RELOC_TYPE	*rp;
-	Elf_Ehdr	*ehdp;
-	Elf_Phdr	*phdp;
 
 	/*
 	 * Scan argument and environment vectors. Find dynamic
@@ -186,35 +184,4 @@ _dl_boot_bind(const long sp, long *dl_data, Elf_Dyn *dynp)
 	}
 
 	RELOC_GOT(&dynld, loff);
-
-	/*
-	 * we have been fully relocated here, so most things no longer
-	 * need the loff adjustment
-	 */
-
-	/*
-	 * No further changes to the PLT and/or GOT are needed so make
-	 * them read-only.
-	 */
-
-	/* do any RWX -> RX fixups for executable PLTs and apply GNU_RELRO */
-	ehdp = (Elf_Ehdr *)loff;
-	phdp = (Elf_Phdr *)(loff + ehdp->e_phoff);
-	for (i = 0; i < ehdp->e_phnum; i++, phdp++) {
-		switch (phdp->p_type) {
-#if defined(__alpha__) || defined(__hppa__) || defined(__powerpc__) || \
-    defined(__sparc64__)
-		case PT_LOAD:
-			if ((phdp->p_flags & (PF_X | PF_W)) != (PF_X | PF_W))
-				break;
-			_dl_mprotect((void *)(phdp->p_vaddr + loff),
-			    phdp->p_memsz, PROT_READ);
-			break;
-#endif
-		case PT_GNU_RELRO:
-			_dl_mprotect((void *)(phdp->p_vaddr + loff),
-			    phdp->p_memsz, PROT_READ);
-			break;
-		}
-	}
 }
diff --git a/libexec/ld.so/loader.c b/libexec/ld.so/loader.c
index 8cb3f645162..625adb75412 100644
--- a/libexec/ld.so/loader.c
+++ b/libexec/ld.so/loader.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: loader.c,v 1.173 2018/10/22 01:59:08 guenther Exp $ */
+/*	$OpenBSD: loader.c,v 1.174 2018/10/23 04:01:45 guenther Exp $ */
 
 /*
  * Copyright (c) 1998 Per Fogelstrom, Opsycon AB
@@ -56,19 +56,19 @@ void _dl_fixup_user_env(void);
 void _dl_call_preinit(elf_object_t *);
 void _dl_call_init_recurse(elf_object_t *object, int initfirst);
 
-int _dl_pagesz;
-int _dl_bindnow;
-int _dl_traceld;
-int _dl_debug;
-
-char **_dl_libpath;
-
-char *_dl_preload;
-char *_dl_showmap;
-char *_dl_tracefmt1, *_dl_tracefmt2, *_dl_traceprog;
+int _dl_pagesz __relro = 4096;
+int _dl_bindnow __relro = 0;
+int _dl_debug __relro = 0;
+int _dl_trust __relro = 0;
+char **_dl_libpath __relro = NULL;
 
-int _dl_trust;
+/* XXX variables which are only used during boot */
+char *_dl_preload __relro = NULL;
+char *_dl_tracefmt1 __relro = NULL;
+char *_dl_tracefmt2 __relro = NULL;
+char *_dl_traceprog __relro = NULL;
 
+int _dl_traceld;
 struct r_debug *_dl_debug_map;
 
 void _dl_dopreload(char *paths);
@@ -367,6 +367,36 @@ _dl_load_dep_libs(elf_object_t *object, int flags, int booting)
 }
 
 
+/* do any RWX -> RX fixups for executable PLTs and apply GNU_RELRO */
+static inline void
+_dl_self_relro(long loff)
+{
+	Elf_Ehdr *ehdp;
+	Elf_Phdr *phdp;
+	int i;
+
+	ehdp = (Elf_Ehdr *)loff;
+	phdp = (Elf_Phdr *)(loff + ehdp->e_phoff);
+	for (i = 0; i < ehdp->e_phnum; i++, phdp++) {
+		switch (phdp->p_type) {
+#if defined(__alpha__) || defined(__hppa__) || defined(__powerpc__) || \
+    defined(__sparc64__)
+		case PT_LOAD:
+			if ((phdp->p_flags & (PF_X | PF_W)) != (PF_X | PF_W))
+				break;
+			_dl_mprotect((void *)(phdp->p_vaddr + loff),
+			    phdp->p_memsz, PROT_READ);
+			break;
+#endif
+		case PT_GNU_RELRO:
+			_dl_mprotect((void *)(phdp->p_vaddr + loff),
+			    phdp->p_memsz, PROT_READ);
+			break;
+		}
+	}
+}
+
+
 #define PFLAGS(X) ((((X) & PF_R) ? PROT_READ : 0) | \
 		   (((X) & PF_W) ? PROT_WRITE : 0) | \
 		   (((X) & PF_X) ? PROT_EXEC : 0))
@@ -398,15 +428,20 @@ _dl_boot(const char **argv, char **envp, const long dyn_loff, long *dl_data)
 
 	if (dl_data[AUX_pagesz] != 0)
 		_dl_pagesz = dl_data[AUX_pagesz];
-	else
-		_dl_pagesz = 4096;
+	_dl_malloc_init();
+	_dl_setup_env(argv[0], envp);
+
+	/*
+	 * Make read-only the GOT and PLT and variables initialized
+	 * during the ld.so setup above.
+	 */
+	_dl_self_relro(dyn_loff);
 
 	align = _dl_pagesz - 1;
 
 #define ROUND_PG(x) (((x) + align) & ~(align))
 #define TRUNC_PG(x) ((x) & ~(align))
 
-	_dl_setup_env(argv[0], envp);
 	if (_dl_bindnow) {
 		/* Lazy binding disabled, so disable kbind */
 		_dl___syscall(SYS_kbind, (void *)NULL, (size_t)0, (long long)0);
diff --git a/libexec/ld.so/malloc.c b/libexec/ld.so/malloc.c
index 8b59c3aea28..c0efb3b8975 100644
--- a/libexec/ld.so/malloc.c
+++ b/libexec/ld.so/malloc.c
@@ -138,12 +138,16 @@ struct malloc_readonly {
 	u_int32_t malloc_canary;	/* Matched against ones in g_pool */
 };
 
-/* This object is mapped PROT_READ after initialisation to prevent tampering */
-static union {
-	struct malloc_readonly mopts;
-	u_char _pad[MALLOC_PAGESIZE];
-} malloc_readonly __attribute__((aligned(MALLOC_PAGESIZE)));
-#define mopts	malloc_readonly.mopts
+/*
+ * malloc configuration, initialized with the defaults
+ */
+static struct malloc_readonly mopts __relro = {
+    .malloc_junk = 1,
+    .chunk_canaries = 1,
+    .malloc_guard = MALLOC_PAGESIZE,
+    .malloc_cache = MALLOC_DEFAULT_CACHE,
+};
+
 #define g_pool	mopts.g_pool
 
 static u_char getrbyte(struct dir_info *d);
@@ -200,24 +204,16 @@ getrbyte(struct dir_info *d)
 }
 
 /*
- * Initialize a dir_info, which should have been cleared by caller
+ * Initialize the malloc subsystem before relro processing.
  */
-static void
-omalloc_init(struct dir_info **dp)
+void
+_dl_malloc_init(void)
 {
 	char *p;
 	int i, j;
 	size_t d_avail, regioninfo_size, tmp;
 	struct dir_info *d;
 
-	/*
-	 * Default options
-	 */
-	mopts.malloc_junk = 1;
-	mopts.chunk_canaries = 1;
-	mopts.malloc_cache = MALLOC_DEFAULT_CACHE;
-	mopts.malloc_guard = MALLOC_PAGESIZE;
-
 	do {
 		_dl_arc4randombuf(&mopts.malloc_canary,
 		    sizeof(mopts.malloc_canary));
@@ -254,16 +250,7 @@ omalloc_init(struct dir_info **dp)
 	d->canary1 = mopts.malloc_canary ^ (u_int32_t)(uintptr_t)d;
 	d->canary2 = ~d->canary1;
 
-	*dp = d;
-
-	/*
-	 * Options have been set and will never be reset.
-	 * Prevent further tampering with them.
-	 */
-	if (((uintptr_t)&malloc_readonly & MALLOC_PAGEMASK) == 0)
-		_dl_mprotect(&malloc_readonly, sizeof(malloc_readonly),
-		    PROT_READ);
-
+	g_pool = d;
 }
 
 static int
@@ -893,8 +880,6 @@ _dl_malloc(size_t size)
 	lock_cb *cb;
 
 	cb = _dl_thread_kern_stop();
-	if (g_pool == NULL)
-		omalloc_init(&g_pool);
 	g_pool->func = "malloc():";
 	if (g_pool->active++) {
 		malloc_recurse();
@@ -1031,8 +1016,6 @@ _dl_calloc(size_t nmemb, size_t size)
 	lock_cb *cb;
 
 	cb = _dl_thread_kern_stop();
-	if (g_pool == NULL)
-		omalloc_init(&g_pool);
 	g_pool->func = "calloc():";
 	if ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) &&
 	    nmemb > 0 && SIZE_MAX / nmemb < size) {
@@ -1085,8 +1068,6 @@ _dl_realloc(void *ptr, size_t size)
 	lock_cb *cb;
 
 	cb = _dl_thread_kern_stop();
-	if (g_pool == NULL)
-		omalloc_init(&g_pool);
 	g_pool->func = "realloc():";
 	if (g_pool->active++) {
 		malloc_recurse();
@@ -1199,8 +1180,6 @@ _dl_aligned_alloc(size_t alignment, size_t size)
 		return NULL;
 
 	cb = _dl_thread_kern_stop();
-	if (g_pool == NULL)
-		omalloc_init(&g_pool);
 	g_pool->func = "aligned_alloc():";
 	if (g_pool->active++) {
 		malloc_recurse();
diff --git a/libexec/ld.so/resolve.h b/libexec/ld.so/resolve.h
index 392caeb558c..89e34b9cd85 100644
--- a/libexec/ld.so/resolve.h
+++ b/libexec/ld.so/resolve.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: resolve.h,v 1.84 2018/10/22 01:59:08 guenther Exp $ */
+/*	$OpenBSD: resolve.h,v 1.85 2018/10/23 04:01:45 guenther Exp $ */
 
 /*
  * Copyright (c) 1998 Per Fogelstrom, Opsycon AB
@@ -33,6 +33,8 @@
 #include <link.h>
 #include <dlfcn.h>
 
+#define __relro		__attribute__((section(".data.rel.ro")))
+
 /* Number of low tags that are used saved internally (0 .. DT_NUM-1) */
 #define DT_NUM	(DT_PREINIT_ARRAYSZ + 1)
 
diff --git a/libexec/ld.so/util.h b/libexec/ld.so/util.h
index 260817d422a..c16d9d3ef74 100644
--- a/libexec/ld.so/util.h
+++ b/libexec/ld.so/util.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: util.h,v 1.32 2017/12/01 23:30:05 guenther Exp $	*/
+/*	$OpenBSD: util.h,v 1.33 2018/10/23 04:01:45 guenther Exp $	*/
 
 /*
  * Copyright (c) 1998 Todd C. Miller <Todd.Miller@courtesan.com>
@@ -36,6 +36,7 @@
 #include <stddef.h>		/* for NULL */
 
 __BEGIN_HIDDEN_DECLS
+void _dl_malloc_init(void);
 void *_dl_malloc(size_t size);
 void *_dl_calloc(size_t nmemb, const size_t size);
 void *_dl_realloc(void *, size_t size);