From 613f8145087a763e128d58e638bc85799fb06989 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Mon, 20 Jul 2015 22:34:27 +0000 Subject: [PATCH] Revert "Revert "make vdso function pointers read-only at runtime"" This reverts commit 1946856b1f18a27c51ba30bb9e304f25b722ee05. This goes back to the original scheme of PROT_NONEing a page within libc. Allocating a new page didn't fail safe for cases where these functions are called from the dynamic linker. Bug: http://b/22568628 Change-Id: I3e7241c8b54c27ea4a898bc952375c1e9ae38c80 --- libc/bionic/vdso.cpp | 50 +++++++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/libc/bionic/vdso.cpp b/libc/bionic/vdso.cpp index a2406634d..b55c57ff5 100644 --- a/libc/bionic/vdso.cpp +++ b/libc/bionic/vdso.cpp @@ -30,8 +30,14 @@ #define VDSO_GETTIMEOFDAY_SYMBOL "__vdso_gettimeofday" #endif +#include +#include +#include #include +#include "private/bionic_prctl.h" +#include "private/libc_logging.h" + extern "C" int __clock_gettime(int, timespec*); extern "C" int __gettimeofday(timeval*, struct timezone*); @@ -46,28 +52,31 @@ enum { VDSO_END }; -static vdso_entry vdso_entries[] = { +static union { + vdso_entry entries[VDSO_END]; + char padding[PAGE_SIZE]; +} vdso __attribute__((aligned(PAGE_SIZE))) = {{ [VDSO_CLOCK_GETTIME] = { VDSO_CLOCK_GETTIME_SYMBOL, reinterpret_cast(__clock_gettime) }, [VDSO_GETTIMEOFDAY] = { VDSO_GETTIMEOFDAY_SYMBOL, reinterpret_cast(__gettimeofday) }, -}; +}}; int clock_gettime(int clock_id, timespec* tp) { - static int (*vdso_clock_gettime)(int, timespec*) = - reinterpret_cast(vdso_entries[VDSO_CLOCK_GETTIME].fn); + int (*vdso_clock_gettime)(int, timespec*) = + reinterpret_cast(vdso.entries[VDSO_CLOCK_GETTIME].fn); return vdso_clock_gettime(clock_id, tp); } int gettimeofday(timeval* tv, struct timezone* tz) { - static int (*vdso_gettimeofday)(timeval*, struct timezone*) = - reinterpret_cast(vdso_entries[VDSO_GETTIMEOFDAY].fn); + int (*vdso_gettimeofday)(timeval*, struct timezone*) = + reinterpret_cast(vdso.entries[VDSO_GETTIMEOFDAY].fn); return vdso_gettimeofday(tv, tz); } -void __libc_init_vdso() { +static void __libc_init_vdso_entries() { // Do we have a vdso? uintptr_t vdso_ehdr_addr = getauxval(AT_SYSINFO_EHDR); ElfW(Ehdr)* vdso_ehdr = reinterpret_cast(vdso_ehdr_addr); - if (vdso_ehdr == NULL) { + if (vdso_ehdr == nullptr) { return; } @@ -85,7 +94,7 @@ void __libc_init_vdso() { // Where's the dynamic table? ElfW(Addr) vdso_addr = 0; - ElfW(Dyn)* vdso_dyn = NULL; + ElfW(Dyn)* vdso_dyn = nullptr; ElfW(Phdr)* vdso_phdr = reinterpret_cast(vdso_ehdr_addr + vdso_ehdr->e_phoff); for (size_t i = 0; i < vdso_ehdr->e_phnum; ++i) { if (vdso_phdr[i].p_type == PT_DYNAMIC) { @@ -94,13 +103,13 @@ void __libc_init_vdso() { vdso_addr = vdso_ehdr_addr + vdso_phdr[i].p_offset - vdso_phdr[i].p_vaddr; } } - if (vdso_addr == 0 || vdso_dyn == NULL) { + if (vdso_addr == 0 || vdso_dyn == nullptr) { return; } // Where are the string and symbol tables? - const char* strtab = NULL; - ElfW(Sym)* symtab = NULL; + const char* strtab = nullptr; + ElfW(Sym)* symtab = nullptr; for (ElfW(Dyn)* d = vdso_dyn; d->d_tag != DT_NULL; ++d) { if (d->d_tag == DT_STRTAB) { strtab = reinterpret_cast(vdso_addr + d->d_un.d_ptr); @@ -108,20 +117,31 @@ void __libc_init_vdso() { symtab = reinterpret_cast(vdso_addr + d->d_un.d_ptr); } } - if (strtab == NULL || symtab == NULL) { + if (strtab == nullptr || symtab == nullptr) { return; } // Are there any symbols we want? for (size_t i = 0; i < symbol_count; ++i) { for (size_t j = 0; j < VDSO_END; ++j) { - if (strcmp(vdso_entries[j].name, strtab + symtab[i].st_name) == 0) { - vdso_entries[j].fn = reinterpret_cast(vdso_addr + symtab[i].st_value); + if (strcmp(vdso.entries[j].name, strtab + symtab[i].st_name) == 0) { + vdso.entries[j].fn = reinterpret_cast(vdso_addr + symtab[i].st_value); } } } } +void __libc_init_vdso() { + __libc_init_vdso_entries(); + + // We can't use PR_SET_VMA because this isn't an anonymous region. + // Long-term we should be able to replace all of this with ifuncs. + static_assert(PAGE_SIZE == sizeof(vdso), "sizeof(vdso) too large"); + if (mprotect(vdso.entries, sizeof(vdso), PROT_READ) == -1) { + __libc_fatal("failed to mprotect PROT_READ vdso function pointer table: %s", strerror(errno)); + } +} + #else void __libc_init_vdso() {