Use PROT_NONE on the unused parts of CFI shadow.

This replaces a single 2Gb readable memory region with a bunch of tiny
regions, and leaves the bulk of 2Gb mapped but unaccessible. This makes
it harder to defeat ASLR by probing for the CFI shadow region.

Sample CFI shadow mapping with this change:
7165151000-716541f000 ---p 00000000 00:00 0                              [anon:cfi shadow]
716541f000-7165420000 r--p 00000000 00:00 0                              [anon:cfi shadow]
7165420000-71654db000 ---p 00000000 00:00 0                              [anon:cfi shadow]
71654db000-71654dc000 r--p 00000000 00:00 0                              [anon:cfi shadow]
71654dc000-71654dd000 r--p 00000000 00:00 0                              [anon:cfi shadow]
71654dd000-71654f0000 ---p 00000000 00:00 0                              [anon:cfi shadow]
71654f0000-71654f1000 r--p 00000000 00:00 0                              [anon:cfi shadow]
71654f1000-71e5151000 ---p 00000000 00:00 0                              [anon:cfi shadow]

This change degrades CFI diagnostics for wild jumps and casts (i.e. when
the target of a CFI check is outside of any known library bounds). This
is acceptable, because CFI does not have much to tell about those cases
anyway. Such bugs will show up as SEGV_ACCERR crashes inside
__cfi_slowpath in libdl.so from now on.

Bug: 158113540
Test: bionic-unit-tests/cfi_test.*
Test: adb shell cat /proc/$PID/maps | grep cfi

Change-Id: I57cbd0d3f87eb1610ad99b48d98ffd497ba214b4

2 files changed, 5 insertions, 7 deletions

diff --git a/linker/linker_cfi.cpp b/linker/linker_cfi.cpp
index 5995013b4..87b5d3485 100644
--- a/linker/linker_cfi.cpp
+++ b/linker/linker_cfi.cpp
@@ -56,6 +56,7 @@ class ShadowWrite {
         reinterpret_cast<char*>(mmap(nullptr, aligned_end - aligned_start, PROT_READ | PROT_WRITE,
                                      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0));
     CHECK(tmp_start != MAP_FAILED);
+    mprotect(aligned_start, aligned_end - aligned_start, PROT_READ);
     memcpy(tmp_start, aligned_start, shadow_start - aligned_start);
     memcpy(tmp_start + (shadow_end - aligned_start), shadow_end, aligned_end - shadow_end);
   }
@@ -154,7 +155,7 @@ uintptr_t soinfo_find_cfi_check(soinfo* si) {
 
 uintptr_t CFIShadowWriter::MapShadow() {
   void* p =
-      mmap(nullptr, kShadowSize, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0);
+      mmap(nullptr, kShadowSize, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0);
   CHECK(p != MAP_FAILED);
   return reinterpret_cast<uintptr_t>(p);
 }
diff --git a/tests/libs/cfi_test_lib.cpp b/tests/libs/cfi_test_lib.cpp
index 9f456d39b..6f551c5f8 100644
--- a/tests/libs/cfi_test_lib.cpp
+++ b/tests/libs/cfi_test_lib.cpp
@@ -67,12 +67,9 @@ struct A {
   void check_cfi_self() {
     g_last_type_id = 0;
     assert(&__cfi_slowpath);
-    // CFI check for an invalid address. Normally, this would kill the process by routing the call
-    // back to the calling module's __cfi_check, which does the right thing based on
-    // -fsanitize-recover / -fsanitize-trap. But this module has custom __cfi_check that does not do
-    // any of that, so the result looks like a passing check.
-    int zz;
-    __cfi_slowpath(13, static_cast<void*>(&zz));
+    // CFI check for an address inside this DSO. This goes to the current module's __cfi_check,
+    // which updates g_last_type_id.
+    __cfi_slowpath(13, static_cast<void*>(&g_last_type_id));
     assert(g_last_type_id == 13);
     // CFI check for a libc function. This never goes into this module's __cfi_check, and must pass.
     __cfi_slowpath(14, reinterpret_cast<void*>(&exit));