From: kn <kn@openbsd.org>
Date: Tue, 25 May 2021 17:01:36 +0000 (+0000)
Subject: program headers: do not rely on DYNAMIC coming before GNU_RELRO
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=f9a87ca699b091bcf816a107ab66f59f69ee1bb1;p=openbsd

program headers: do not rely on DYNAMIC coming before GNU_RELRO

Except for some specific cases (thanks guenther) ELF mandates nothing
but the file header be at a fixed location, hence ld.so(1) must not
assume any specific order for headers, segments, etc.

Looping over the program header table to parse segment headers,
_dl_boot() creates the executable object upon DYNAMIC and expects it to
be set upon GNU_RELRO, resulting in a NULL dereference iff that order is
reversed.

Store relocation bits in temporary variables and update the executable
object once all segment headers are parsed to lift this dependency.

Under __mips__ _dl_boot() later on uses the same temporary variable, so
move nothing but the declaration out of MI code so as to not alter the
MD code's logic/behaviour.

Found while porting patchelf(1) from NixOS.

OK guenther
---

diff --git a/libexec/ld.so/loader.c b/libexec/ld.so/loader.c
index 18bd30af759..db66346e41c 100644
--- a/libexec/ld.so/loader.c
+++ b/libexec/ld.so/loader.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: loader.c,v 1.191 2021/03/16 18:03:06 kurt Exp $ */
+/*	$OpenBSD: loader.c,v 1.192 2021/05/25 17:01:36 kn Exp $ */
 
 /*
  * Copyright (c) 1998 Per Fogelstrom, Opsycon AB
@@ -464,6 +464,7 @@ _dl_boot(const char **argv, char **envp, const long dyn_loff, long *dl_data)
 	int failed;
 	struct dep_node *n;
 	Elf_Addr minva, maxva, exe_loff, exec_end, cur_exec_end;
+	Elf_Addr relro_addr = 0, relro_size = 0;
 	Elf_Phdr *ptls = NULL;
 	int align;
 
@@ -552,8 +553,8 @@ _dl_boot(const char **argv, char **envp, const long dyn_loff, long *dl_data)
 			ptls = phdp;
 			break;
 		case PT_GNU_RELRO:
-			exe_obj->relro_addr = phdp->p_vaddr + exe_loff;
-			exe_obj->relro_size = phdp->p_memsz;
+			relro_addr = phdp->p_vaddr + exe_loff;
+			relro_size = phdp->p_memsz;
 			break;
 		}
 		phdp++;
@@ -561,6 +562,8 @@ _dl_boot(const char **argv, char **envp, const long dyn_loff, long *dl_data)
 	exe_obj->load_list = load_list;
 	exe_obj->obj_flags |= DF_1_GLOBAL;
 	exe_obj->load_size = maxva - minva;
+	exe_obj->relro_addr = relro_addr;
+	exe_obj->relro_size = relro_size;
 	_dl_set_sod(exe_obj->load_name, &exe_obj->sod);
 
 #ifdef __i386__
@@ -638,7 +641,7 @@ _dl_boot(const char **argv, char **envp, const long dyn_loff, long *dl_data)
 		debug_map->r_ldbase = dyn_loff;
 		_dl_debug_map = debug_map;
 #ifdef __mips__
-		Elf_Addr relro_addr = exe_obj->relro_addr;
+		relro_addr = exe_obj->relro_addr;
 		if (dynp->d_tag == DT_DEBUG &&
 		    ((Elf_Addr)map_link + sizeof(*map_link) <= relro_addr ||
 		     (Elf_Addr)map_link >= relro_addr + exe_obj->relro_size)) {