ARM: Remove any calls to the kuser_helpers magic page. |
||||||
Issue descriptionOn 32-bit Linux/ARM and Android/ARM, the kernel maps a special page at address 0xffff0000 that contains optimized implementations of certain low-level operations (e.g. atomic operations) tailored for the specific kernel and hardware configuration being used. Two functions provided by this page are typically used to perform atomic compare-and-swap and data-memory-barrier. This magic page will be removed in future versions of the Android kernel, to prevent ASLR bypass However, because Chrome now targets 32-bit devices that are all multi-core and based on the ARMv7-A architecture, it is possible to ignore these functions, and implementing these operations directly through the appropriate machine code instructions (LDREX/STREX and DMB, respecitvely). The point of this entry is to remove any explicit use of the kuser_helpers page from Chrome at runtime. There are potentially several sources to consider: - Explicit use of the kernel helpers in third-party libraries that implement their own atomics operations. - base/atomic_ops.h used to do this, but now relies on std::atomic operations. We need to check that the std::atomic runtime does not contain calls to the kernel page, since it is linked into the final Chrome binaries (i.e. it's not provided by the system on Android). - Similarly, the GCC toolchain used to implement its atomic intrinsics (e.g. __sync_fetch_add) using the kernel page as well, so we need to check that it has been configured to not do this anymore.
,
Jan 27 2017
Looking for 0xffff0xfc0 directly: third_party/android_protobuf/src/src/google/protobuf/stubs/atomicops_internals_arm_gcc.h third_party/protobuf/src/google/protobuf/stubs/atomicops_internals_arm_gcc.h third_party/tcmalloc/vendor/src/base/atomicops-internals-arm-generic.h third_party/tcmalloc/chromium/src/base/atomicops-internals-arm-generic.h Looking for 0xffff0fa0, we find the headers above, and also: third_party/re2/src/util/atomicops.h third_party/WebKit/Source/wtf/Atomics.h third_party/leveldatabase/src/port/atomic_pointer.h That does not mean that they are all used by Chrome on Android, just the presence in the sources of the Clank tree.
,
Jan 27 2017
Looking at the <atomic> header used for building the 32-bit Android/ARM binaries, it comes from: third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libc++/include/atomic The content of this file shows that the implementation of std::atomic<> methods uses the compiler builtins like __atomic_fetch_add, __atomic_thread_fence, etc. So we only need to look at the toolchain being used and the way it was configured.
,
Jan 27 2017
The toolchain being used is gcc-4.9. We have prebuilt binaries built from android.googlesource.com/toolchain/gcc/gcc-4.9 with commit 1641488ea2548cf9c5e61ef5a2f914aa496f9870 Looking at the sources reveals that: The old/legacy __sync_xxxx atomic builtins are provided by libgcc.a which implements them using the kernel helper unconditionally on ARM [1] The new __atomic_xxxx builtins on the other hand are expanded directly by the compiler as LDRES/STREX and DMB instruction when using libatomic/config/linux/arm/host-config.h (which we do) The libatomic sources use the kernel helper functions, but only if the macros HAVE_DMB and/or HAVE_STREX are not defined [2], but these are actually always defined for ARMv7, as controlled by the header [3] As a conclusion, we don't need to care about libatomic and __atomic_xxx primitives here. On the other hand, we need to look for use of __sync_xxx builtins in the sources as well because they will translate to calls into libgcc functions that depend on the kernel helpers. Another potential fix is to remove these kernel helper calls from libgcc though. [1] https://android.googlesource.com/toolchain/gcc/+/1641488ea2548cf9c5e61ef5a2f914aa496f9870/gcc-4.9/libgcc/config/arm/linux-atomic.c [2] https://android.googlesource.com/toolchain/gcc/+/1641488ea2548cf9c5e61ef5a2f914aa496f9870/gcc-4.9/libatomic/config/linux/arm/host-config.h [3] https://android.googlesource.com/toolchain/gcc/+/1641488ea2548cf9c5e61ef5a2f914aa496f9870/gcc-4.9/libatomic/config/arm/arm-config.h
,
Jan 27 2017
There are *plenty* of uses of __sync_synchronize() and other __sync_xxxx() builtins in our source tree, be it in third-party libraries and even the NDK's own <stdatomic.h> and <sys/atomics.h> headers. At this point, it seems easier to wait for an updated NDK toolchain that will change the libgcc implementation of said primitives to use __atomic_xxxx ones instead.
,
Jan 30 2017
,
Jan 30 2017
Slight correction: while libgcc.a does indeed provide a version of __sync_synchronize that uses the kernel helper, the toolchain will always translate a __sync_synchronize() call into a DMB instruction if -march=armeabi-v7a is used. This means there is no need for an updated toolchain, and we can keep __sync_synchronize() calls in all sources! So we just need to get rid of the explicit kernel helper calls in Blink and third-party libraries. Relevant Blink patch at https://codereview.chromium.org/2651203006/
,
Jan 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/70847b53c7d1d5d8c65d526f966718a9e2c8adef commit 70847b53c7d1d5d8c65d526f966718a9e2c8adef Author: digit <digit@chromium.org> Date: Mon Jan 30 17:41:07 2017 wtf/Atomics.h: Remove Android/ARM kernel helpers. Future Android kernels are going to remove the special page that is mapped in every process at a fixed address, and which provides optimized low-level operations. Instead of calling the memory helper related to memory barriers, use __sync_synchonize which will generate a DMB instruction directly, just like all other platforms (e.g. Linux/ARM). The only benefit of using the kernel helper was to avoid the DMB instruction on single-core devices, which are now totally obsolete. This change only impacts Android/ARM builds. BUG= 686001 Review-Url: https://codereview.chromium.org/2651203006 Cr-Commit-Position: refs/heads/master@{#447008} [modify] https://crrev.com/70847b53c7d1d5d8c65d526f966718a9e2c8adef/third_party/WebKit/Source/wtf/Atomics.h
,
Jul 13 2017
Looking at the Chromium sources today, the only occurrences of 0xffff0fc0 and 0xffff0fa0 happen in the following headers: third_party/protobuf/src/google/protobuf/stubs/atomicops_internals_arm_gcc.h third_party/tcmalloc/chromium/src/base/atomicops-internals-arm-generic.h third_party/tcmalloc/vendor/src/base/atomicops-internals-arm-generic.h Looking at the atomicsops.h header in the same directories indicates when/how these headers are included: - For third_party/protobuf/, the header above is included for 32-bit ARM builds on both Linux and Android. - For third_party/tcmalloc/chromium and third_party/tcmalloc/vendor/ the atomicops.h will include atomicops-internal-arm-v6plus.h for builds that target ARMv6 or above, which is the case for all Android and Linux 32-bit builds. This means that we only need to fix the protobuf header usage to close this bug. This requires adding a new patch under third_party/protobuf/patches and applying it properly to the source tree.
,
Jan 22 2018
Reviving this bug by looking at the current set of sources for today: For reference, the list of kernel user helper addresses is at: https://www.kernel.org/doc/Documentation/arm/kernel_user_helpers.txt Looking for 0xffff0ffc (kuser_helper_version): - No match. Looking for 0xffff0fe0 (kuser_get_tls): - One Crashpad test uses this here [1] Proposal: Add a #ifdef __ANDROID__ block that uses machine instruction to get the value from the right coprocessor register. See [2]. Then propose an upstream fix. Looking for 0xffff0fc0 (kuser_cmpxchg): - Found in //third_party/tcmalloc/chromium/ [3], this header is not included when building for ARMv7-A devices (see [4]) so never part of the Chromium build! Also: TCMalloc is never used on Android (that could have been a concern for ChromeOS/ARM though). Proposal: NOTHING TO DO. - Found in //third_party/tcmalloc/vendor/ and NOTHING TO DO for the exact same reasons (besides the fact that these are never used, since they are overriden by the Chromium version). - Found in //third_party/protobuf/ [5], normally, this header is not used when using GCC 4.7 or higher (see [6]). *HOWEVER*, we're now using clang++ to build Clank which advertises itself as GCC 4.2.1 As a consequence this header IS INCLUDED, and the corresponding machine code CONTAINS references to the kuser helper (verified by looking at the 32-bit ARM disassembly for protobuf/src/google/protobuf/stubs/once.cc!) Proposal: Either find a way for Clank to advertise itself as GCC 4.9 or modify this header with a Chromium-specific patch to include a different header when __clang__ is defined! Then provide an upstream fix. - Found in //third_party/android_protobuf/, has exactly the same issues that the version in //third_party/protobuf/, except that the header is only used to build the *host* android_protoc binary (which is used at build time to generate Java protobuf sources). Verified by looking at the content of the build output directory, i.e.: out/Release/clang_x64/obj/third_party/android_protobuf/android_protoc/once.o Proposal: IGNORE for the moment, though it would be nice to get a more up-to-date upstream version that doesn't have this issue. Looking for 0xffff0fa0: (kuser_memory_barrier) For the record, this helper is implemented as follows: * ARMv7+: Use a single "dmb" hardware instruction then return. * ARMv6: Use a co-processor mov instruction that is equivalent to "dmb" * ARMv5: Nothing, since these devices are not multi-core. - Found under //third_party/leveldatabase/ [7], this one is really always compiled into a call into the kernel user page :-( Proposal: patch to use the "dmb" assembly instruction when building for ARMv7-A+ (for the record, that is *exactly* what the kernel helper implements). then provide an upstream fix. - Found in //third_party/tcmalloc [8], and NOTHING TO DO here as well since this is never used on Android, nor even compiled for ARMv6+ devices. - Found in //third_party/protobuf/ and needs the same Clang-specific fix as kuser_cmpxchg - Found in //third_party/android_protobuf/ and can be IGNORED here as well, just as for kuser_cmpxchg. Looking for 0xffff0f60 (kuser_cmpxchg64) - Nothing! :-) [1] https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/test/linux/get_tls.cc?type=cs&q=0xffff0fe0&l=26 [2] https://android.googlesource.com/platform/bionic/+/master/libc/private/__get_tls.h [3] https://cs.chromium.org/chromium/src/third_party/tcmalloc/chromium/src/base/atomicops-internals-arm-generic.h?q=0xffff0fc0&l=61&dr=C [4] https://cs.chromium.org/chromium/src/third_party/tcmalloc/chromium/src/base/atomicops.h?dr=C&l=94 [5] https://cs.chromium.org/chromium/src/third_party/protobuf/src/google/protobuf/stubs/atomicops_internals_arm_gcc.h?q=0xffff0fc0&l=51&dr=C [6] https://cs.chromium.org/chromium/src/third_party/protobuf/src/google/protobuf/stubs/atomicops.h?dr=C&l=202 [7] https://cs.chromium.org/chromium/src/third_party/leveldatabase/src/port/atomic_pointer.h?q=0xffff0fa0&l=95&dr=C [8] https://cs.chromium.org/chromium/src/third_party/tcmalloc/vendor/src/base/atomicops-internals-arm-generic.h?q=0xffff0fa0&l=65&dr=C
,
Jan 22 2018
,
Jan 29 2018
,
Aug 23
,
Nov 19
There are no more calls to the magic page in the current Chrome builds. Switching to Fixed now! |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by digit@chromium.org
, Jan 27 2017Just started to look into the Chromium source tree, and found third_party/android_protobuf/src/src/google/protobuf/subs/atomicops_internals_arm_gcc.h which contains things like: // 0xffff0fc0 is the hard coded address of a function provided by // the kernel which implements an atomic compare-exchange. On older // ARM architecture revisions (pre-v6) this may be implemented using // a syscall. This address is stable, and in active use (hard coded) // by at least glibc-2.7 and the Android C library. typedef Atomic32 (*LinuxKernelCmpxchgFunc)(Atomic32 old_value, Atomic32 new_value, volatile Atomic32* ptr); LinuxKernelCmpxchgFunc pLinuxKernelCmpxchg __attribute__((weak)) = (LinuxKernelCmpxchgFunc) 0xffff0fc0; typedef void (*LinuxKernelMemoryBarrierFunc)(void); LinuxKernelMemoryBarrierFunc pLinuxKernelMemoryBarrier __attribute__((weak)) = (LinuxKernelMemoryBarrierFunc) 0xffff0fa0;