Package: thunderbird / 1:60.8.0-1~deb9u1

fixes/Bug-1470701-Use-run-time-page-size-when-changing-map.patch Patch series | download
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
From: Mike Hommey <mh@glandium.org>
Date: Sun, 24 Jun 2018 09:02:38 +0900
Subject: Bug 1470701 - Use run-time page size when changing mapping
 permissions in elfhack injected code. r?froydnj

When a binary has a PT_GNU_RELRO segment, the elfhack injected code
uses mprotect to add the writable flag to relocated pages before
applying relocations, removing it afterwards. To do so, the elfhack
program uses the location and size of the PT_GNU_RELRO segment, and
adjusts it to be aligned according to the PT_LOAD alignment.

The problem here is that the PT_LOAD alignment doesn't necessarily match
the actual page alignment, and the resulting mprotect may end up not
covering the full extent of what the dynamic linker has protected
read-only according to the PT_GNU_RELRO segment. In turn, this can lead
to a crash on startup when trying to apply relocations to the still
read-only locations.

Practically speaking, this doesn't end up being a problem on x86, where
the PT_LOAD alignment is usually 4096, which happens to be the page
size, but on Debian armhf, it is 64k, while the run time page size can be
4k.
---
 build/unix/elfhack/elfhack.cpp | 98 ++++++++++++++++++++++++------------------
 build/unix/elfhack/inject.c    | 31 +++++++------
 build/unix/elfhack/test.c      |  4 ++
 3 files changed, 78 insertions(+), 55 deletions(-)

diff --git a/build/unix/elfhack/elfhack.cpp b/build/unix/elfhack/elfhack.cpp
index 4d270af..e7e2fa5 100644
--- a/build/unix/elfhack/elfhack.cpp
+++ b/build/unix/elfhack/elfhack.cpp
@@ -87,12 +87,13 @@ class ElfRelHackCode_Section : public ElfSection {
  public:
   ElfRelHackCode_Section(Elf_Shdr &s, Elf &e,
                          ElfRelHack_Section &relhack_section, unsigned int init,
-                         unsigned int mprotect_cb)
+                         unsigned int mprotect_cb, unsigned int sysconf_cb)
       : ElfSection(s, nullptr, nullptr),
         parent(e),
         relhack_section(relhack_section),
         init(init),
-        mprotect_cb(mprotect_cb) {
+        mprotect_cb(mprotect_cb),
+        sysconf_cb(sysconf_cb) {
     std::string file(rundir);
     file += "/inject/";
     switch (parent.getMachine()) {
@@ -130,7 +131,6 @@ class ElfRelHackCode_Section : public ElfSection {
           "Couldn't find a symbol table for the injected code");
 
     relro = parent.getSegmentByType(PT_GNU_RELRO);
-    align = parent.getSegmentByType(PT_LOAD)->getAlign();
 
     // Find the init symbol
     entry_point = -1;
@@ -357,12 +357,12 @@ class ElfRelHackCode_Section : public ElfSection {
           addr = init;
         } else if (relro && strcmp(name, "mprotect_cb") == 0) {
           addr = mprotect_cb;
+        } else if (relro && strcmp(name, "sysconf_cb") == 0) {
+          addr = sysconf_cb;
         } else if (relro && strcmp(name, "relro_start") == 0) {
-          // Align relro segment start to the start of the page it starts in.
-          addr = relro->getAddr() & ~(align - 1);
-          // Align relro segment end to the start of the page it ends into.
+          addr = relro->getAddr();
         } else if (relro && strcmp(name, "relro_end") == 0) {
-          addr = (relro->getAddr() + relro->getMemSize()) & ~(align - 1);
+          addr = (relro->getAddr() + relro->getMemSize());
         } else if (strcmp(name, "_GLOBAL_OFFSET_TABLE_") == 0) {
           // We actually don't need a GOT, but need it as a reference for
           // GOTOFF relocations. We'll just use the start of the ELF file
@@ -418,9 +418,9 @@ class ElfRelHackCode_Section : public ElfSection {
   std::vector<ElfSection *> code;
   unsigned int init;
   unsigned int mprotect_cb;
+  unsigned int sysconf_cb;
   int entry_point;
   ElfSegment *relro;
-  unsigned int align;
 };
 
 unsigned int get_addend(Elf_Rel *rel, Elf *elf) {
@@ -727,6 +727,7 @@ int do_relocation_section(Elf *elf, unsigned int rel_type,
   }
 
   unsigned int mprotect_cb = 0;
+  unsigned int sysconf_cb = 0;
   // If there is a relro segment, our injected code will run after the linker
   // sets the corresponding pages read-only. We need to make our code change
   // that to read-write before applying relocations, which means it needs to
@@ -740,37 +741,48 @@ int do_relocation_section(Elf *elf, unsigned int rel_type,
   // section temporarily (it will be restored to a null value before any code
   // can actually use it)
   if (elf->getSegmentByType(PT_GNU_RELRO)) {
-    Elf_SymValue *mprotect = symtab->lookup("mprotect", STT(FUNC));
-    if (!mprotect) {
-      symtab->syms.emplace_back();
-      mprotect = &symtab->syms.back();
-      symtab->grow(symtab->syms.size() * symtab->getEntSize());
-      mprotect->name =
-          ((ElfStrtab_Section *)symtab->getLink())->getStr("mprotect");
-      mprotect->info = ELF32_ST_INFO(STB_GLOBAL, STT_FUNC);
-      mprotect->other = STV_DEFAULT;
-      new (&mprotect->value) ElfLocation(nullptr, 0, ElfLocation::ABSOLUTE);
-      mprotect->size = 0;
-      mprotect->defined = false;
-
-      // The DT_VERSYM data (in the .gnu.version section) has the same number of
-      // entries as the symbols table. Since we added one entry there, we need
-      // to add one entry here. Zeroes in the extra data means no version for
-      // that symbol, which is the simplest thing to do.
-      ElfSection *gnu_versym = dyn->getSectionForType(DT_VERSYM);
-      if (gnu_versym) {
-        gnu_versym->grow(gnu_versym->getSize() + gnu_versym->getEntSize());
+    ElfSection *gnu_versym = dyn->getSectionForType(DT_VERSYM);
+    auto lookup = [&symtab, &gnu_versym](const char *symbol) {
+      Elf_SymValue *sym_value = symtab->lookup(symbol, STT(FUNC));
+      if (!sym_value) {
+        symtab->syms.emplace_back();
+        sym_value = &symtab->syms.back();
+        symtab->grow(symtab->syms.size() * symtab->getEntSize());
+        sym_value->name =
+            ((ElfStrtab_Section *)symtab->getLink())->getStr(symbol);
+        sym_value->info = ELF32_ST_INFO(STB_GLOBAL, STT_FUNC);
+        sym_value->other = STV_DEFAULT;
+        new (&sym_value->value) ElfLocation(nullptr, 0, ElfLocation::ABSOLUTE);
+        sym_value->size = 0;
+        sym_value->defined = false;
+
+        // The DT_VERSYM data (in the .gnu.version section) has the same number
+        // of entries as the symbols table. Since we added one entry there, we
+        // need to add one entry here. Zeroes in the extra data means no version
+        // for that symbol, which is the simplest thing to do.
+        if (gnu_versym) {
+          gnu_versym->grow(gnu_versym->getSize() + gnu_versym->getEntSize());
+        }
       }
-    }
+      return sym_value;
+    };
 
-    // Add a relocation for the mprotect symbol.
-    new_rels.emplace_back();
-    Rel_Type &rel = new_rels.back();
-    memset(&rel, 0, sizeof(rel));
-    rel.r_info = ELF32_R_INFO(
-        std::distance(symtab->syms.begin(),
-                      std::vector<Elf_SymValue>::iterator(mprotect)),
-        rel_type2);
+    Elf_SymValue *mprotect = lookup("mprotect");
+    Elf_SymValue *sysconf = lookup("sysconf");
+
+    // Add relocations for the mprotect and sysconf symbols.
+    auto add_relocation_to = [&new_rels, &symtab, rel_type2](
+                                 Elf_SymValue *symbol, unsigned int location) {
+      new_rels.emplace_back();
+      Rel_Type &rel = new_rels.back();
+      memset(&rel, 0, sizeof(rel));
+      rel.r_info = ELF32_R_INFO(
+          std::distance(symtab->syms.begin(),
+                        std::vector<Elf_SymValue>::iterator(symbol)),
+          rel_type2);
+      rel.r_offset = location;
+      return location;
+    };
 
     // Find the beginning of the bss section, and use an aligned location in
     // there for the relocation.
@@ -782,13 +794,14 @@ int do_relocation_section(Elf *elf, unsigned int rel_type,
       size_t ptr_size = Elf_Addr::size(elf->getClass());
       size_t usable_start = (s->getAddr() + ptr_size - 1) & ~(ptr_size - 1);
       size_t usable_end = (s->getAddr() + s->getSize()) & ~(ptr_size - 1);
-      if (usable_end - usable_start >= ptr_size) {
-        mprotect_cb = rel.r_offset = usable_start;
+      if (usable_end - usable_start >= 2 * ptr_size) {
+        mprotect_cb = add_relocation_to(mprotect, usable_start);
+        sysconf_cb = add_relocation_to(sysconf, usable_start + ptr_size);
         break;
       }
     }
 
-    if (mprotect_cb == 0) {
+    if (mprotect_cb == 0 || sysconf_cb == 0) {
       fprintf(stderr, "Couldn't find .bss. Skipping\n");
       return -1;
     }
@@ -797,8 +810,9 @@ int do_relocation_section(Elf *elf, unsigned int rel_type,
   section->rels.assign(new_rels.begin(), new_rels.end());
   section->shrink(new_rels.size() * section->getEntSize());
 
-  ElfRelHackCode_Section *relhackcode = new ElfRelHackCode_Section(
-      relhackcode_section, *elf, *relhack, original_init, mprotect_cb);
+  ElfRelHackCode_Section *relhackcode =
+      new ElfRelHackCode_Section(relhackcode_section, *elf, *relhack,
+                                 original_init, mprotect_cb, sysconf_cb);
   // Find the first executable section, and insert the relhack code before
   // that. The relhack data is inserted between .rel.dyn and .rel.plt.
   ElfSection *first_executable = nullptr;
diff --git a/build/unix/elfhack/inject.c b/build/unix/elfhack/inject.c
index 862e9d9..7a1f65f 100644
--- a/build/unix/elfhack/inject.c
+++ b/build/unix/elfhack/inject.c
@@ -4,6 +4,7 @@
 
 #include <stdint.h>
 #include <stdlib.h>
+#include <unistd.h>
 #include <sys/mman.h>
 #include <elf.h>
 
@@ -29,6 +30,7 @@ extern __attribute__((visibility("hidden"))) Elf_Ehdr elf_header;
 extern __attribute__((visibility("hidden"))) int (*mprotect_cb)(void *addr,
                                                                 size_t len,
                                                                 int prot);
+extern __attribute__((visibility("hidden"))) long (*sysconf_cb)(int name);
 extern __attribute__((visibility("hidden"))) char relro_start[];
 extern __attribute__((visibility("hidden"))) char relro_end[];
 
@@ -58,34 +60,37 @@ __attribute__((section(".text._init"))) int init(int argc, char **argv,
   return 0;
 }
 
-static inline __attribute__((always_inline)) void relro_pre() {
+static inline __attribute__((always_inline)) void do_relocations_with_relro(
+    void) {
+  long page_size = sysconf_cb(_SC_PAGESIZE);
+  uintptr_t aligned_relro_start = ((uintptr_t)relro_start) & ~(page_size - 1);
+  uintptr_t aligned_relro_end = ((uintptr_t)relro_end) & ~(page_size - 1);
   // By the time the injected code runs, the relro segment is read-only. But
   // we want to apply relocations in it, so we set it r/w first. We'll restore
   // it to read-only in relro_post.
-  mprotect_cb(relro_start, relro_end - relro_start, PROT_READ | PROT_WRITE);
-}
+  mprotect_cb((void *)aligned_relro_start,
+              aligned_relro_end - aligned_relro_start, PROT_READ | PROT_WRITE);
+
+  do_relocations();
 
-static inline __attribute__((always_inline)) void relro_post() {
-  mprotect_cb(relro_start, relro_end - relro_start, PROT_READ);
-  // mprotect_cb is a pointer allocated in .bss, so we need to restore it to
-  // a NULL value.
+  mprotect_cb((void *)aligned_relro_start,
+              aligned_relro_end - aligned_relro_start, PROT_READ);
+  // mprotect_cb and sysconf_cb are allocated in .bss, so we need to restore
+  // them to a NULL value.
   mprotect_cb = NULL;
+  sysconf_cb = NULL;
 }
 
 __attribute__((section(".text._init_noinit_relro"))) int init_noinit_relro(
     int argc, char **argv, char **env) {
-  relro_pre();
-  do_relocations();
-  relro_post();
+  do_relocations_with_relro();
   return 0;
 }
 
 __attribute__((section(".text._init_relro"))) int init_relro(int argc,
                                                              char **argv,
                                                              char **env) {
-  relro_pre();
-  do_relocations();
-  relro_post();
+  do_relocations_with_relro();
   original_init(argc, argv, env);
   return 0;
 }
diff --git a/build/unix/elfhack/test.c b/build/unix/elfhack/test.c
index 3ca40b1..ab994a4 100644
--- a/build/unix/elfhack/test.c
+++ b/build/unix/elfhack/test.c
@@ -127,6 +127,10 @@ int print_status() {
 __thread int foo;
 __thread long long int bar[512];
 
+/* We need a .bss that can hold at least 2 pointers. The static in
+ * end_test() plus this variable should do. */
+size_t dummy;
+
 void end_test() {
   static size_t count = 0;
   /* Only exit when both constructors have been called */