From 326ea5413d18ea019cd1bda415ce428f7bdcafd2 Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Tue, 4 Dec 2012 13:55:19 -0800 Subject: [PATCH] clean up FORTIFY_SOURCE handling. Avoid duplicating huge chunks of code. Change-Id: Id6145cdfce781c5ffba2abaaa79681d25a7ab28f --- libc/bionic/__fgets_chk.cpp | 8 ++------ libc/bionic/__memcpy_chk.cpp | 6 ++---- libc/bionic/__memmove_chk.cpp | 6 ++---- libc/bionic/__memset_chk.cpp | 6 ++---- libc/bionic/__strcat_chk.cpp | 12 ++++-------- libc/bionic/__strcpy_chk.cpp | 6 ++---- libc/bionic/__strlcat_chk.cpp | 4 +--- libc/bionic/__strlcpy_chk.cpp | 4 +--- libc/bionic/__strlen_chk.cpp | 4 +--- libc/bionic/__strncat_chk.cpp | 12 ++++-------- libc/bionic/__strncpy_chk.cpp | 6 ++---- libc/bionic/__umask_chk.cpp | 4 +--- libc/bionic/__vsnprintf_chk.cpp | 4 +--- libc/bionic/__vsprintf_chk.cpp | 4 +--- libc/bionic/logd_write.c | 11 +++++++++++ libc/private/logd.h | 2 ++ libc/string/strchr.c | 7 ++----- libc/string/strrchr.c | 7 ++----- 18 files changed, 43 insertions(+), 70 deletions(-) diff --git a/libc/bionic/__fgets_chk.cpp b/libc/bionic/__fgets_chk.cpp index 780cf1635..6ae97ccc0 100644 --- a/libc/bionic/__fgets_chk.cpp +++ b/libc/bionic/__fgets_chk.cpp @@ -45,15 +45,11 @@ extern "C" char *__fgets_chk(char *dest, int supplied_size, FILE *stream, size_t dest_len_from_compiler) { if (supplied_size < 0) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** fgets buffer size less than 0 ***\n"); - abort(); + __fortify_chk_fail("fgets buffer size less than 0", 0); } if (((size_t) supplied_size) > dest_len_from_compiler) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** fgets buffer overflow detected ***\n"); - abort(); + __fortify_chk_fail("fgets buffer overflow", 0); } return fgets(dest, supplied_size, stream); diff --git a/libc/bionic/__memcpy_chk.cpp b/libc/bionic/__memcpy_chk.cpp index 991ff02a4..7a98cb716 100644 --- a/libc/bionic/__memcpy_chk.cpp +++ b/libc/bionic/__memcpy_chk.cpp @@ -46,10 +46,8 @@ extern "C" void *__memcpy_chk(void *dest, const void *src, size_t copy_amount, size_t dest_len) { if (__builtin_expect(copy_amount > dest_len, 0)) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** memcpy buffer overflow detected ***\n"); - __libc_android_log_event_uid(BIONIC_EVENT_MEMCPY_BUFFER_OVERFLOW); - abort(); + __fortify_chk_fail("memcpy buffer overflow", + BIONIC_EVENT_MEMCPY_BUFFER_OVERFLOW); } return memcpy(dest, src, copy_amount); diff --git a/libc/bionic/__memmove_chk.cpp b/libc/bionic/__memmove_chk.cpp index 1867d716a..51f2e1cca 100644 --- a/libc/bionic/__memmove_chk.cpp +++ b/libc/bionic/__memmove_chk.cpp @@ -45,10 +45,8 @@ extern "C" void *__memmove_chk (void *dest, const void *src, size_t len, size_t dest_len) { if (len > dest_len) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** memmove buffer overflow detected ***\n"); - __libc_android_log_event_uid(BIONIC_EVENT_MEMMOVE_BUFFER_OVERFLOW); - abort(); + __fortify_chk_fail("memmove buffer overflow", + BIONIC_EVENT_MEMMOVE_BUFFER_OVERFLOW); } return memmove(dest, src, len); diff --git a/libc/bionic/__memset_chk.cpp b/libc/bionic/__memset_chk.cpp index 97c5c384a..99a12ad09 100644 --- a/libc/bionic/__memset_chk.cpp +++ b/libc/bionic/__memset_chk.cpp @@ -43,10 +43,8 @@ */ extern "C" void *__memset_chk (void *dest, int c, size_t n, size_t dest_len) { if (n > dest_len) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** memset buffer overflow detected ***\n"); - __libc_android_log_event_uid(BIONIC_EVENT_MEMSET_BUFFER_OVERFLOW); - abort(); + __fortify_chk_fail("memset buffer overflow", + BIONIC_EVENT_MEMSET_BUFFER_OVERFLOW); } return memset(dest, c, n); diff --git a/libc/bionic/__strcat_chk.cpp b/libc/bionic/__strcat_chk.cpp index ec194fc18..2450da6cc 100644 --- a/libc/bionic/__strcat_chk.cpp +++ b/libc/bionic/__strcat_chk.cpp @@ -50,17 +50,13 @@ extern "C" char *__strcat_chk (char *dest, const char *src, size_t dest_buf_size // sum = src_len + dest_len + 1 (with overflow protection) if (!safe_add3(&sum, src_len, dest_len, 1U)) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** strcat integer overflow detected ***\n"); - __libc_android_log_event_uid(BIONIC_EVENT_STRCAT_INTEGER_OVERFLOW); - abort(); + __fortify_chk_fail("strcat integer overflow", + BIONIC_EVENT_STRCAT_INTEGER_OVERFLOW); } if (sum > dest_buf_size) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** strcat buffer overflow detected ***\n"); - __libc_android_log_event_uid(BIONIC_EVENT_STRNCAT_BUFFER_OVERFLOW); - abort(); + __fortify_chk_fail("strcat buffer overflow", + BIONIC_EVENT_STRCAT_BUFFER_OVERFLOW); } return strcat(dest, src); diff --git a/libc/bionic/__strcpy_chk.cpp b/libc/bionic/__strcpy_chk.cpp index 1d45ea27b..74ceda1bc 100644 --- a/libc/bionic/__strcpy_chk.cpp +++ b/libc/bionic/__strcpy_chk.cpp @@ -45,10 +45,8 @@ extern "C" char *__strcpy_chk (char *dest, const char *src, size_t dest_len) { // TODO: optimize so we don't scan src twice. size_t src_len = strlen(src) + 1; if (src_len > dest_len) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** strcpy buffer overflow detected ***\n"); - __libc_android_log_event_uid(BIONIC_EVENT_STRCPY_BUFFER_OVERFLOW); - abort(); + __fortify_chk_fail("strcpy buffer overflow", + BIONIC_EVENT_STRCPY_BUFFER_OVERFLOW); } return strcpy(dest, src); diff --git a/libc/bionic/__strlcat_chk.cpp b/libc/bionic/__strlcat_chk.cpp index 05b7d7d50..12676f4f0 100644 --- a/libc/bionic/__strlcat_chk.cpp +++ b/libc/bionic/__strlcat_chk.cpp @@ -46,9 +46,7 @@ extern "C" size_t __strlcat_chk(char *dest, const char *src, size_t supplied_size, size_t dest_len_from_compiler) { if (supplied_size > dest_len_from_compiler) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** strlcat buffer overflow detected ***\n"); - abort(); + __fortify_chk_fail("strlcat buffer overflow", 0); } return strlcat(dest, src, supplied_size); diff --git a/libc/bionic/__strlcpy_chk.cpp b/libc/bionic/__strlcpy_chk.cpp index bf9803781..62fa14bc5 100644 --- a/libc/bionic/__strlcpy_chk.cpp +++ b/libc/bionic/__strlcpy_chk.cpp @@ -46,9 +46,7 @@ extern "C" size_t __strlcpy_chk(char *dest, const char *src, size_t supplied_size, size_t dest_len_from_compiler) { if (supplied_size > dest_len_from_compiler) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** strlcpy buffer overflow detected ***\n"); - abort(); + __fortify_chk_fail("strlcpy buffer overflow", 0); } return strlcpy(dest, src, supplied_size); diff --git a/libc/bionic/__strlen_chk.cpp b/libc/bionic/__strlen_chk.cpp index 67410d42c..5cc052e56 100644 --- a/libc/bionic/__strlen_chk.cpp +++ b/libc/bionic/__strlen_chk.cpp @@ -57,9 +57,7 @@ extern "C" size_t __strlen_chk(const char *s, size_t s_len) { size_t ret = strlen(s); if (__builtin_expect(ret >= s_len, 0)) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** strlen read overflow detected ***\n"); - abort(); + __fortify_chk_fail("strlen read overflow", 0); } return ret; diff --git a/libc/bionic/__strncat_chk.cpp b/libc/bionic/__strncat_chk.cpp index 2ba855034..32a3962ff 100644 --- a/libc/bionic/__strncat_chk.cpp +++ b/libc/bionic/__strncat_chk.cpp @@ -55,17 +55,13 @@ extern "C" char *__strncat_chk (char *dest, const char *src, size_t sum; // sum = src_len + dest_len + 1 (with overflow protection) if (!safe_add3(&sum, src_len, dest_len, 1U)) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** strncat integer overflow detected ***\n"); - __libc_android_log_event_uid(BIONIC_EVENT_STRNCAT_INTEGER_OVERFLOW); - abort(); + __fortify_chk_fail("strncat integer overflow", + BIONIC_EVENT_STRNCAT_INTEGER_OVERFLOW); } if (sum > dest_buf_size) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** strncat buffer overflow detected ***\n"); - __libc_android_log_event_uid(BIONIC_EVENT_STRNCAT_BUFFER_OVERFLOW); - abort(); + __fortify_chk_fail("strncat buffer overflow", + BIONIC_EVENT_STRNCAT_BUFFER_OVERFLOW); } return strncat(dest, src, len); diff --git a/libc/bionic/__strncpy_chk.cpp b/libc/bionic/__strncpy_chk.cpp index 875d092f0..c9676ed85 100644 --- a/libc/bionic/__strncpy_chk.cpp +++ b/libc/bionic/__strncpy_chk.cpp @@ -45,10 +45,8 @@ extern "C" char *__strncpy_chk (char *dest, const char *src, size_t len, size_t dest_len) { if (len > dest_len) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** strncpy buffer overflow detected ***\n"); - __libc_android_log_event_uid(BIONIC_EVENT_STRNCPY_BUFFER_OVERFLOW); - abort(); + __fortify_chk_fail("strncpy buffer overflow", + BIONIC_EVENT_STRNCPY_BUFFER_OVERFLOW); } return strncpy(dest, src, len); diff --git a/libc/bionic/__umask_chk.cpp b/libc/bionic/__umask_chk.cpp index df066b213..e1bc96d28 100644 --- a/libc/bionic/__umask_chk.cpp +++ b/libc/bionic/__umask_chk.cpp @@ -43,9 +43,7 @@ */ extern "C" mode_t __umask_chk(mode_t mode) { if ((mode & 0777) != mode) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** FORTIFY_SOURCE: umask called with invalid mask ***\n"); - abort(); + __fortify_chk_fail("umask called with invalid mask", 0); } return umask(mode); diff --git a/libc/bionic/__vsnprintf_chk.cpp b/libc/bionic/__vsnprintf_chk.cpp index b4f534bc3..95d491569 100644 --- a/libc/bionic/__vsnprintf_chk.cpp +++ b/libc/bionic/__vsnprintf_chk.cpp @@ -51,9 +51,7 @@ extern "C" int __vsnprintf_chk( va_list va) { if (supplied_size > dest_len_from_compiler) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** vsnprintf buffer overflow detected ***\n"); - abort(); + __fortify_chk_fail("vsnprintf buffer overflow", 0); } return vsnprintf(dest, supplied_size, format, va); diff --git a/libc/bionic/__vsprintf_chk.cpp b/libc/bionic/__vsprintf_chk.cpp index 00010cfca..e1d10f55a 100644 --- a/libc/bionic/__vsprintf_chk.cpp +++ b/libc/bionic/__vsprintf_chk.cpp @@ -52,9 +52,7 @@ extern "C" int __vsprintf_chk( int ret = vsnprintf(dest, dest_len_from_compiler, format, va); if ((size_t) ret >= dest_len_from_compiler) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** vsprintf buffer overflow detected ***\n"); - abort(); + __fortify_chk_fail("vsprintf buffer overflow", 0); } return ret; diff --git a/libc/bionic/logd_write.c b/libc/bionic/logd_write.c index ac71689cc..71a6f8e80 100644 --- a/libc/bionic/logd_write.c +++ b/libc/bionic/logd_write.c @@ -247,3 +247,14 @@ void __libc_android_log_event_uid(int32_t tag) { __libc_android_log_event_int(tag, getuid()); } + +__LIBC_HIDDEN__ +void __fortify_chk_fail(const char *msg, uint32_t tag) { + __libc_android_log_print(ANDROID_LOG_FATAL, "libc", + "FORTIFY_SOURCE: %s. Calling abort().\n", + msg); + if (tag != 0) { + __libc_android_log_event_uid(tag); + } + abort(); +} diff --git a/libc/private/logd.h b/libc/private/logd.h index 26878ba61..a2828ec09 100644 --- a/libc/private/logd.h +++ b/libc/private/logd.h @@ -71,6 +71,8 @@ int __libc_android_log_vprint(int prio, const char *tag, const char *fmt, va_lis void __libc_android_log_event_int(int32_t tag, int value); void __libc_android_log_event_uid(int32_t tag); +__noreturn extern void __fortify_chk_fail(const char *, uint32_t); + #ifdef __cplusplus }; #endif diff --git a/libc/string/strchr.c b/libc/string/strchr.c index 44516efd1..564ea8051 100644 --- a/libc/string/strchr.c +++ b/libc/string/strchr.c @@ -35,11 +35,8 @@ char * __strchr_chk(const char *p, int ch, size_t s_len) { for (;; ++p, s_len--) { - if (s_len == 0) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** FORTIFY_SOURCE strchr read beyond buffer ***\n"); - abort(); - } + if (s_len == 0) + __fortify_chk_fail("strchr read beyond buffer", 0); if (*p == (char) ch) return((char *)p); if (!*p) diff --git a/libc/string/strrchr.c b/libc/string/strrchr.c index fc3dc4ed7..5d0415e13 100644 --- a/libc/string/strrchr.c +++ b/libc/string/strrchr.c @@ -37,11 +37,8 @@ __strrchr_chk(const char *p, int ch, size_t s_len) char *save; for (save = NULL;; ++p, s_len--) { - if (s_len == 0) { - __libc_android_log_print(ANDROID_LOG_FATAL, "libc", - "*** FORTIFY_SOURCE strrchr read beyond buffer ***\n"); - abort(); - } + if (s_len == 0) + __fortify_chk_fail("strrchr read beyond buffer", 0); if (*p == (char) ch) save = (char *)p; if (!*p)