Skip to content
Commit a9940c2a authored by Nick Desaulniers's avatar Nick Desaulniers
Browse files

zygote: fix mprotect range for non-page-aligned segments



As of LLVM r369344
commit f66b767abe5e ("[ELF][AArch64] Allow PT_LOAD to have overlapping p_offset ranges")
it's no longer guaranteed that binaries linked with LLD have segments
that are loaded at virtual addresses that are multiples of the page
size. This saves significant space in the binary.

This implies the subexpression from `man 3 dl_iterate_phdr`:

addr == info->dlpi_addr + info->dlpi_phdr[x].p_vaddr;

is not precise (as noted in bionic/linker/linker.cpp as well).

This change results in failures to change page protections via calls to
mprotect from execute only pages back to read+execute for apps targeting
an Android SDK version < 28 (Q), since mprotect requires a page aligned
address. When Zygote forks the app, the child may segfault (SIGSEGV) due
to invalid permissions (SEGV_ACCERR).

Previously, `info->dlpi_phdr[x].p_vaddr` was page aligned. Now that it's
not, we must round it down to the closet multiple of the page size, and
extend the range to account for this difference.

Previously:

+----+  (page)
+---+|  (segment)
|   ||
+---|+
+---+
^ page boundary, segment p_vaddr (aligned)

Now:

+----+  (page)
| +---+ (segment)
| |  ||
+-|--+|
  +---*
  ^ segment p_vaddr (unaligned)
^ page boundary

Account for the alignment of the segment's virtual address not
necessarily being a multiple of the page size by rounding down to the
nearest multiple of the page size for the address passed to mprotect,
then add the remainder back so the correct number of pages get remapped
properly. (It would not be correct to subtract the remainder, otherwise
a segment could span two pages, and we might not remap both).

Example:

p_vaddr == PAGE_START(p_vaddr) + PAGE_OFFSET(p_vaddr)
0x378c  == 0x3000              + 0x78c
|          |                     |
^ Can't be passed to mprotect, not page aligned.
           ^ Added to dlpi_addr (which is already page aligned), then
             passed to mprotect. |
                                 ^ Added to size.

Finally, check the return code of mprotect, and fail early in zygote,
rather than segfault down the line in the child.

Test: atest \
  CtsSelinuxTargetSdk27TestCases:android.security.SELinuxTargetSdkTest#testNoExecuteOnly
Test: launch Facebook or Instagram
Bug: 145825270
Change-Id: I6c609e73b59b86c2fd493a8ccf91ccf6c4dc75bf
Signed-off-by: default avatarNick Desaulniers <ndesaulniers@google.com>
parent 20ff4372
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment