From d0631c289a37ce4d8c91220ea95087890991f449 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Mon, 11 Jan 2016 13:35:44 -0800 Subject: Fix bug where a failure becomes success on x86. In libunwind, the unw_step function on x86, an error can get changed into a success. This meant illegal frames were added that didn't really exist. Even worse, this could cause crashes because these completely garbage ip addresses were used to try and get another stack frame. This lead to attempts to access illegal data, and if you get unlucky and the map for the bad data was valid at one point, and invalid at the point of unwind, you get a crash. There is a small similar possibility of a stop unwind becoming a continue on aarch64. Bug: 26248094 (cherry picked from commit c60f22c30ebf16ee4ab990994535f972f349e00e) Change-Id: I9ef0364d0494791c4866fa82af4ca8930c452bd0 --- src/aarch64/Gstep.c | 2 +- src/x86/Gstep.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/aarch64/Gstep.c b/src/aarch64/Gstep.c index 3f5e8adc..5833ce2a 100644 --- a/src/aarch64/Gstep.c +++ b/src/aarch64/Gstep.c @@ -164,7 +164,7 @@ unw_step (unw_cursor_t *cursor) if (unlikely (ret == -UNW_ESTOPUNWIND)) return ret; - if (unlikely (ret < 0)) + if (unlikely (ret <= 0)) return 0; return (c->dwarf.ip == 0) ? 0 : 1; diff --git a/src/x86/Gstep.c b/src/x86/Gstep.c index a1368eb1..308f220b 100644 --- a/src/x86/Gstep.c +++ b/src/x86/Gstep.c @@ -130,7 +130,8 @@ unw_step (unw_cursor_t *cursor) c->dwarf.frame++; } /* End of ANDROID update. */ - ret = (c->dwarf.ip == 0) ? 0 : 1; - Debug (2, "returning %d\n", ret); - return ret; + if (unlikely (ret <= 0)) + return 0; + + return (c->dwarf.ip == 0) ? 0 : 1; } -- cgit v1.2.3 From 0e8a5fd11c73c790ab4d46b8841b54b1f33d42ba Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Wed, 24 Feb 2016 00:44:24 -0800 Subject: Mark executable maps with no name as unreadable. On some devices, executable maps without names are created and deleted. Doing unwinds can sometimes attempt to access these maps and crash because the map has disappeared since that last read of the map. Mark these maps as unreadable to avoid this problem. Bug: 27152097 Change-Id: I17277823c1793a9d695d53cc6de3df53242c80a2 --- src/os-linux.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/os-linux.c b/src/os-linux.c index 0a584703..7a54faa6 100644 --- a/src/os-linux.c +++ b/src/os-linux.c @@ -75,8 +75,8 @@ map_create_list (int map_create_type, pid_t pid) && strncmp ("ashmem/", cur_map->path + 5, 7) != 0) cur_map->flags |= MAP_FLAGS_DEVICE_MEM; - /* Remove readable bit from any map ending in (deleted) that is - not executable and is not a shared library. The shared library + /* Remove readable bit from any empty map or a map ending in (deleted) + that is not executable and is not a shared library. The shared library restriction is so that if a shared library has been deleted, the unwind from memory can still access the other possible shared memory readable segments. */ @@ -91,6 +91,8 @@ map_create_list (int map_create_type, pid_t pid) SHARED_LIB, sizeof(SHARED_LIB) - 1) != 0) { cur_map->flags &= ~PROT_READ; } + } else if (path_len == 0 && (cur_map->flags & PROT_EXEC)) { + cur_map->flags &= ~PROT_READ; } /* If this is a readable executable map, and not a stack map or an -- cgit v1.2.3