fortify: replace bzero/bcmp defines
__builtin_*_chk will emit warnings when things are trivially broken.
Emitting errors instead is probably better (and we can be a bit smarter
about how we emit code for trivially safe cases.)
Bug: 131861088
Test: checkbuild + bionic-unit-tests on blueline
Change-Id: I33957ad419922d0760304758ecb9bc8ad33e0b64
diff --git a/libc/bionic/ndk_cruft.cpp b/libc/bionic/ndk_cruft.cpp
index 2c3299f..f0a7026 100644
--- a/libc/bionic/ndk_cruft.cpp
+++ b/libc/bionic/ndk_cruft.cpp
@@ -243,15 +243,23 @@
return signal(signum, handler);
}
+// bcopy/bzero were previously `#define`d, so we only have `static` wrappers in
+// Bionic headers. Since we have header definitions, we need some way to
+// overload these implementations; __never_call will ensure that any calls to
+// bcopy/bzero call the in-header implementation. Since the implementations
+// should end up functionally identical, it doesn't matter which we actually
+// call.
+#define __never_call __attribute__((enable_if(false, "never selected")))
+
// This was removed from POSIX 2008.
-#undef bcopy
-void bcopy(const void* src, void* dst, size_t n) {
+void bcopy(const void* src, void* dst, size_t n) __never_call __RENAME(bcopy);
+void bcopy(const void* src, void* dst, size_t n) __never_call {
memmove(dst, src, n);
}
// This was removed from POSIX 2008.
-#undef bzero
-void bzero(void* dst, size_t n) {
+void bzero(void* dst, size_t n) __never_call __RENAME(bzero);
+void bzero(void* dst, size_t n) __never_call {
memset(dst, 0, n);
}
diff --git a/libc/include/bits/fortify/strings.h b/libc/include/bits/fortify/strings.h
new file mode 100644
index 0000000..6bda295
--- /dev/null
+++ b/libc/include/bits/fortify/strings.h
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2019 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.
+ */
+
+#if defined(__BIONIC_FORTIFY)
+
+#if __ANDROID_API__ >= __ANDROID_API_J_MR1__
+__BIONIC_FORTIFY_INLINE
+void bcopy(const void *src, void* const dst __pass_object_size0, size_t len)
+ __overloadable
+ __clang_error_if(__bos_unevaluated_lt(__bos0(dst), len),
+ "'bcopy' called with size bigger than buffer") {
+ size_t bos = __bos0(dst);
+ if (__bos_trivially_not_lt(bos, len)) {
+ __builtin_memmove(dst, src, len);
+ } else {
+ __builtin___memmove_chk(dst, src, len, bos);
+ }
+}
+
+__BIONIC_FORTIFY_INLINE
+void bzero(void* const b __pass_object_size0, size_t len)
+ __overloadable
+ __clang_error_if(__bos_unevaluated_lt(__bos0(b), len),
+ "'bzero' called with size bigger than buffer") {
+ size_t bos = __bos0(b);
+ if (__bos_trivially_not_lt(bos, len)) {
+ __builtin_memset(b, 0, len);
+ } else {
+ __builtin___memset_chk(b, 0, len, bos);
+ }
+}
+#endif /* __ANDROID_API__ >= __ANDROID_API_J_MR1__ */
+
+#endif /* defined(__BIONIC_FORTIFY) */
diff --git a/libc/include/strings.h b/libc/include/strings.h
index ccdac04..a054aed 100644
--- a/libc/include/strings.h
+++ b/libc/include/strings.h
@@ -51,17 +51,15 @@
__BEGIN_DECLS
-#if defined(__BIONIC_FORTIFY)
/** Deprecated. Use memmove() instead. */
-#define bcopy(b1, b2, len) (void)(__builtin___memmove_chk((b2), (b1), (len), __bos0(b2)))
+static __inline__ __always_inline void bcopy(const void* b1, void* b2, size_t len) {
+ __builtin_memmove(b2, b1, len);
+}
+
/** Deprecated. Use memset() instead. */
-#define bzero(b, len) (void)(__builtin___memset_chk((b), '\0', (len), __bos0(b)))
-#else
-/** Deprecated. Use memmove() instead. */
-#define bcopy(b1, b2, len) (void)(__builtin_memmove((b2), (b1), (len)))
-/** Deprecated. Use memset() instead. */
-#define bzero(b, len) (void)(__builtin_memset((b), '\0', (len)))
-#endif
+static __inline__ __always_inline void bzero(void* b, size_t len) {
+ __builtin_memset(b, 0, len);
+}
#if !defined(__i386__) || __ANDROID_API__ >= __ANDROID_API_J_MR2__
/**
@@ -72,6 +70,10 @@
int ffs(int __i) __INTRODUCED_IN_X86(18);
#endif
+#if defined(__BIONIC_INCLUDE_FORTIFY_HEADERS)
+#include <bits/fortify/strings.h>
+#endif
+
__END_DECLS
#include <android/legacy_strings_inlines.h>
diff --git a/tests/clang_fortify_tests.cpp b/tests/clang_fortify_tests.cpp
index 291fc5a..1b6b898 100644
--- a/tests/clang_fortify_tests.cpp
+++ b/tests/clang_fortify_tests.cpp
@@ -165,10 +165,9 @@
EXPECT_FORTIFY_DEATH(memset(small_buffer, 0, sizeof(large_buffer)));
// expected-warning@+1{{arguments got flipped?}}
EXPECT_NO_DEATH(memset(small_buffer, sizeof(small_buffer), 0));
- // FIXME: Should these be warnings?
- // expected-warning@+1{{will always overflow}}
+ // expected-error@+1{{size bigger than buffer}}
EXPECT_FORTIFY_DEATH(bcopy(large_buffer, small_buffer, sizeof(large_buffer)));
- // expected-warning@+1{{will always overflow}}
+ // expected-error@+1{{size bigger than buffer}}
EXPECT_FORTIFY_DEATH(bzero(small_buffer, sizeof(large_buffer)));
}