Make fork equivalent to vfork when HWASan or MTE stack tagging is enabled.
Bug: 274056091
Change-Id: Iac029ca6b0e26f57f20c0a54822b75e3cae67344
diff --git a/libc/arch-arm64/bionic/vfork.S b/libc/arch-arm64/bionic/vfork.S
index 9eb82d8..9b19232 100644
--- a/libc/arch-arm64/bionic/vfork.S
+++ b/libc/arch-arm64/bionic/vfork.S
@@ -28,6 +28,7 @@
#include <platform/bionic/tls_defines.h>
#include <private/bionic_asm.h>
+#include <private/bionic_asm_offsets.h>
#include <asm/signal.h>
#include <linux/sched.h>
@@ -42,10 +43,29 @@
ldr w10, [x9, #20]
str w0, [x9, #20]
- // Clear vfork_child_stack_bottom_.
- str xzr, [x9, #776]
+ mov x0, #SIGCHLD
- mov x0, #(CLONE_VM | CLONE_VFORK | SIGCHLD)
+ // If either HWASan or stack MTE is enabled, set up the clone() flags to
+ // make vfork() act like fork(). We don't call the atfork handlers, so we
+ // may deadlock if the child allocates, but we have seen badly written
+ // atfork handlers themselves cause deadlocks [1]. ndk_translation already
+ // implements vfork() as fork() without calling handlers, so we have some
+ // evidence that it isn't necessary to call them.
+ //
+ // POSIX.1 defines vfork() to have the same effect as fork() except that
+ // most behavior, including heap allocation, becomes undefined in the child,
+ // so we aren't violating POSIX by doing this.
+ //
+ // [1] https://cs.android.com/android/platform/superproject/+/master:system/extras/simpleperf/app_api/cpp/simpleperf.cpp;drc=788fa4183441f4977ddbd5a055e42a7fe7691d21;l=308
+#if !__has_feature(hwaddress_sanitizer)
+ // if (!__libc_globals->memtag_stack) x0 |= CLONE_VM | CLONE_VFORK;
+ adrp x1, __libc_globals + OFFSETOF_libc_globals_memtag_stack
+ ldrb w1, [x1, :lo12:__libc_globals + OFFSETOF_libc_globals_memtag_stack]
+ cbnz w1, 1f
+ orr x0, x0, #CLONE_VM
+ orr x0, x0, #CLONE_VFORK
+1:
+#endif
mov x1, xzr
mov x2, xzr
mov x3, xzr
@@ -62,25 +82,6 @@
cneg x0, x0, hi
b.hi __set_errno_internal
- // Clean up stack shadow in the parent process.
- // https://github.com/google/sanitizers/issues/925
- paciasp
- .cfi_negate_ra_state
- stp x0, x30, [sp, #-16]!
- .cfi_adjust_cfa_offset 16
- .cfi_rel_offset x0, 0
- .cfi_rel_offset x30, 8
-
- add x0, sp, #16
- bl memtag_handle_vfork
-
- ldp x0, x30, [sp], #16
- .cfi_adjust_cfa_offset -16
- .cfi_restore x0
- .cfi_restore x30
- autiasp
- .cfi_negate_ra_state
-
.L_exit:
ret
END(vfork)
diff --git a/libc/bionic/exec.cpp b/libc/bionic/exec.cpp
index 40612e7..863aa97 100644
--- a/libc/bionic/exec.cpp
+++ b/libc/bionic/exec.cpp
@@ -186,6 +186,5 @@
__attribute__((no_sanitize("memtag"))) int execve(const char* pathname, char* const* argv,
char* const* envp) {
- __get_thread()->vfork_child_stack_bottom = __builtin_frame_address(0);
return __execve(pathname, argv, envp);
}
diff --git a/libc/bionic/exit.cpp b/libc/bionic/exit.cpp
index 52fd193..04baac2 100644
--- a/libc/bionic/exit.cpp
+++ b/libc/bionic/exit.cpp
@@ -37,7 +37,6 @@
extern "C" __noreturn void __exit_group(int status);
__attribute__((no_sanitize("memtag"))) void _exit(int status) {
- __get_thread()->vfork_child_stack_bottom = __builtin_frame_address(0);
__exit_group(status);
}
diff --git a/libc/bionic/heap_tagging.cpp b/libc/bionic/heap_tagging.cpp
index 78d21b0..be21e0c 100644
--- a/libc/bionic/heap_tagging.cpp
+++ b/libc/bionic/heap_tagging.cpp
@@ -212,29 +212,3 @@
__hwasan_handle_longjmp(sp_dst);
#endif // __has_feature(hwaddress_sanitizer)
}
-
-extern "C" __LIBC_HIDDEN__ __attribute__((no_sanitize("memtag"), no_sanitize("hwaddress"))) void
-memtag_handle_vfork(void* sp __unused) {
-#ifdef __aarch64__
- if (__libc_globals->memtag_stack) {
- void* child_sp = __get_thread()->vfork_child_stack_bottom;
- __get_thread()->vfork_child_stack_bottom = nullptr;
- if (child_sp) {
- size_t distance = reinterpret_cast<uintptr_t>(sp) - reinterpret_cast<uintptr_t>(child_sp);
- if (distance > kUntagLimit) {
- async_safe_fatal(
- "memtag_handle_vfork: stack adjustment too large! %p -> %p, distance %zx > %zx\n",
- child_sp, sp, distance, kUntagLimit);
- } else {
- untag_memory(child_sp, sp);
- }
- } else {
- async_safe_fatal("memtag_handle_vfork: child SP unknown\n");
- }
- }
-#endif // __aarch64__
-
-#if __has_feature(hwaddress_sanitizer)
- __hwasan_handle_vfork(sp);
-#endif // __has_feature(hwaddress_sanitizer)
-}
diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h
index 083c2ed..8f2b9ac 100644
--- a/libc/bionic/pthread_internal.h
+++ b/libc/bionic/pthread_internal.h
@@ -173,13 +173,6 @@
bionic_tls* bionic_tls;
int errno_value;
-
- // The last observed value of SP in a vfork child process.
- // The part of the stack between this address and the value of SP when the vfork parent process
- // regains control may have stale MTE tags and needs cleanup. This field is only meaningful while
- // the parent is waiting for the vfork child to return control by calling either exec*() or
- // exit().
- void* vfork_child_stack_bottom;
};
struct ThreadMapping {
diff --git a/libc/private/bionic_asm_offsets.h b/libc/private/bionic_asm_offsets.h
new file mode 100644
index 0000000..c2f2b56
--- /dev/null
+++ b/libc/private/bionic_asm_offsets.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#pragma once
+
+#ifdef __aarch64__
+#define OFFSETOF_libc_globals_memtag_stack 80
+#endif
diff --git a/libc/private/bionic_globals.h b/libc/private/bionic_globals.h
index c375cc4..510d556 100644
--- a/libc/private/bionic_globals.h
+++ b/libc/private/bionic_globals.h
@@ -38,6 +38,7 @@
#include "private/WriteProtected.h"
#include "private/bionic_allocator.h"
+#include "private/bionic_asm_offsets.h"
#include "private/bionic_elf_tls.h"
#include "private/bionic_fdsan.h"
#include "private/bionic_malloc_dispatch.h"
@@ -65,6 +66,10 @@
MallocDispatch malloc_dispatch_table;
};
+#ifdef __aarch64__
+static_assert(OFFSETOF_libc_globals_memtag_stack == offsetof(libc_globals, memtag_stack));
+#endif
+
__LIBC_HIDDEN__ extern WriteProtected<libc_globals> __libc_globals;
struct abort_msg_t;
diff --git a/tests/struct_layout_test.cpp b/tests/struct_layout_test.cpp
index 10c100a..0123ed9 100644
--- a/tests/struct_layout_test.cpp
+++ b/tests/struct_layout_test.cpp
@@ -30,7 +30,7 @@
#define CHECK_OFFSET(name, field, offset) \
check_offset(#name, #field, offsetof(name, field), offset);
#ifdef __LP64__
- CHECK_SIZE(pthread_internal_t, 784);
+ CHECK_SIZE(pthread_internal_t, 776);
CHECK_OFFSET(pthread_internal_t, next, 0);
CHECK_OFFSET(pthread_internal_t, prev, 8);
CHECK_OFFSET(pthread_internal_t, tid, 16);
@@ -55,7 +55,6 @@
CHECK_OFFSET(pthread_internal_t, dlerror_buffer, 248);
CHECK_OFFSET(pthread_internal_t, bionic_tls, 760);
CHECK_OFFSET(pthread_internal_t, errno_value, 768);
- CHECK_OFFSET(pthread_internal_t, vfork_child_stack_bottom, 776);
CHECK_SIZE(bionic_tls, 12200);
CHECK_OFFSET(bionic_tls, key_data, 0);
CHECK_OFFSET(bionic_tls, locale, 2080);
@@ -73,7 +72,7 @@
CHECK_OFFSET(bionic_tls, bionic_systrace_disabled, 12193);
CHECK_OFFSET(bionic_tls, padding, 12194);
#else
- CHECK_SIZE(pthread_internal_t, 672);
+ CHECK_SIZE(pthread_internal_t, 668);
CHECK_OFFSET(pthread_internal_t, next, 0);
CHECK_OFFSET(pthread_internal_t, prev, 4);
CHECK_OFFSET(pthread_internal_t, tid, 8);
@@ -98,7 +97,6 @@
CHECK_OFFSET(pthread_internal_t, dlerror_buffer, 148);
CHECK_OFFSET(pthread_internal_t, bionic_tls, 660);
CHECK_OFFSET(pthread_internal_t, errno_value, 664);
- CHECK_OFFSET(pthread_internal_t, vfork_child_stack_bottom, 668);
CHECK_SIZE(bionic_tls, 11080);
CHECK_OFFSET(bionic_tls, key_data, 0);
CHECK_OFFSET(bionic_tls, locale, 1040);