Bug 1455662 - Guard against mprotect() failure when manipulating link map r=jchen
MozReview-Commit-ID: 7orhBmf4j5j
--- a/mozglue/linker/ElfLoader.cpp
+++ b/mozglue/linker/ElfLoader.cpp
@@ -899,26 +899,38 @@ public:
length = lastPageEnd - firstPage;
uintptr_t start = reinterpret_cast<uintptr_t>(firstPage);
uintptr_t end;
prot = getProt(start, &end);
if (prot == -1 || (start + length) > end)
MOZ_CRASH();
- if (prot & PROT_WRITE)
+ if (prot & PROT_WRITE) {
+ success = true;
return;
+ }
page = firstPage;
- mprotect(page, length, prot | PROT_WRITE);
+ int ret = mprotect(page, length, prot | PROT_WRITE);
+ success = ret == 0;
+ if (!success) {
+ ERROR("mprotect(%p, %zu, %d) = %d (errno=%d; %s)",
+ page, length, prot | PROT_WRITE, ret,
+ errno, strerror(errno));
+ }
+ }
+
+ bool IsWritable() const {
+ return success;
}
~EnsureWritable()
{
- if (page != MAP_FAILED) {
+ if (success && page != MAP_FAILED) {
mprotect(page, length, prot);
}
}
private:
int getProt(uintptr_t addr, uintptr_t *end)
{
/* The interesting part of the /proc/self/maps format looks like:
@@ -948,16 +960,17 @@ private:
return result;
}
return -1;
}
int prot;
void *page;
size_t length;
+ bool success;
};
/**
* The system linker maintains a doubly linked list of library it loads
* for use by the debugger. Unfortunately, it also uses the list pointers
* in a lot of operations and adding our data in the list is likely to
* trigger crashes when the linker tries to use data we don't provide or
* that fall off the amount data we allocated. Fortunately, the linker only
@@ -971,54 +984,73 @@ private:
* program executable, so the system linker should not be changing
* r_debug::r_map.
*/
void
ElfLoader::DebuggerHelper::Add(ElfLoader::link_map *map)
{
if (!dbg->r_brk)
return;
+
dbg->r_state = r_debug::RT_ADD;
dbg->r_brk();
- map->l_prev = nullptr;
- map->l_next = dbg->r_map;
+
if (!firstAdded) {
- firstAdded = map;
/* When adding a library for the first time, r_map points to data
* handled by the system linker, and that data may be read-only */
EnsureWritable w(&dbg->r_map->l_prev);
+ if (!w.IsWritable()) {
+ dbg->r_state = r_debug::RT_CONSISTENT;
+ dbg->r_brk();
+ return;
+ }
+
+ firstAdded = map;
dbg->r_map->l_prev = map;
} else
dbg->r_map->l_prev = map;
+
+ map->l_prev = nullptr;
+ map->l_next = dbg->r_map;
+
dbg->r_map = map;
dbg->r_state = r_debug::RT_CONSISTENT;
dbg->r_brk();
}
void
ElfLoader::DebuggerHelper::Remove(ElfLoader::link_map *map)
{
if (!dbg->r_brk)
return;
+
dbg->r_state = r_debug::RT_DELETE;
dbg->r_brk();
+
+ if (map == firstAdded) {
+ /* When removing the first added library, its l_next is going to be
+ * data handled by the system linker, and that data may be read-only */
+ EnsureWritable w(&map->l_next->l_prev);
+ if (!w.IsWritable()) {
+ dbg->r_state = r_debug::RT_CONSISTENT;
+ dbg->r_brk();
+ return;
+ }
+
+ firstAdded = map->l_prev;
+ map->l_next->l_prev = map->l_prev;
+ } else if (map->l_next) {
+ map->l_next->l_prev = map->l_prev;
+ }
+
if (dbg->r_map == map)
dbg->r_map = map->l_next;
else if (map->l_prev) {
map->l_prev->l_next = map->l_next;
}
- if (map == firstAdded) {
- firstAdded = map->l_prev;
- /* When removing the first added library, its l_next is going to be
- * data handled by the system linker, and that data may be read-only */
- EnsureWritable w(&map->l_next->l_prev);
- map->l_next->l_prev = map->l_prev;
- } else if (map->l_next) {
- map->l_next->l_prev = map->l_prev;
- }
dbg->r_state = r_debug::RT_CONSISTENT;
dbg->r_brk();
}
#if defined(ANDROID) && defined(__NR_sigaction)
/* As some system libraries may be calling signal() or sigaction() to
* set a SIGSEGV handler, effectively breaking MappableSeekableZStream,
* or worse, restore our SIGSEGV handler with wrong flags (which using