From 7708a89c60e7b024d31c48c8034932c5e9f0aceb Mon Sep 17 00:00:00 2001 From: David 'Digit' Turner Date: Thu, 30 Jun 2011 18:32:03 +0200 Subject: [PATCH] libc: Add logcat error message for memory corruption Our dlmalloc implementation currently calls abort() when it detects that the heap is corrupted, or that an invalid pointer is passed to one of its functions. The only way to detect this is because abort() will force-fully crash the current program with a magic fault address of '0xdeadbaad'. However, this is not really well documented, and a frequent topic on the android-ndk forum (among others). This change makes our dlmalloc code dump a simple message to the log just before the abort() call (and hence before the stack trace) to better help identify the problem. Change-Id: Iebf7eb7fe26463ecadfaca8f247d237edb441e3c --- libc/bionic/dlmalloc.c | 52 ++++++++++++++++++++++++++++++++++++++++++------ libc/bionic/logd_write.c | 47 +++++++++++++++++++++++++++++-------------- libc/private/logd.h | 1 + 3 files changed, 79 insertions(+), 21 deletions(-) diff --git a/libc/bionic/dlmalloc.c b/libc/bionic/dlmalloc.c index 19fbb7521..035dbac59 100644 --- a/libc/bionic/dlmalloc.c +++ b/libc/bionic/dlmalloc.c @@ -2265,13 +2265,53 @@ static void reset_on_error(mstate m); #else /* PROCEED_ON_ERROR */ -#ifndef CORRUPTION_ERROR_ACTION -#define CORRUPTION_ERROR_ACTION(m) ABORT -#endif /* CORRUPTION_ERROR_ACTION */ +/* The following Android-specific code is used to print an informative + * fatal error message to the log when we detect that a heap corruption + * was detected. We need to be careful about not using a log function + * that may require an allocation here! + */ +#ifdef __ANDROID__ + +# include + +static void __bionic_heap_error(const char* msg, const char* function) +{ + /* We format the buffer explicitely, i.e. without using snprintf() + * which may use malloc() internally. Not something we can trust + * if we just detected a corrupted heap. + */ + char buffer[256]; + strlcpy(buffer, "@@@ ABORTING: ", sizeof(buffer)); + strlcat(buffer, msg, sizeof(buffer)); + if (function != NULL) { + strlcat(buffer, " IN ", sizeof(buffer)); + strlcat(buffer, function, sizeof(buffer)); + } + __libc_android_log_write(ANDROID_LOG_FATAL,"libc","%s", buffer); + abort(); +} + +# ifndef CORRUPTION_ERROR_ACTION +# define CORRUPTION_ERROR_ACTION(m) \ + __bionic_heap_error("HEAP MEMORY CORRUPTION", __FUNCTION__) +# endif +# ifndef USAGE_ERROR_ACTION +# define USAGE_ERROR_ACTION(m,p) \ + __bionic_heap_error("INVALID HEAP ADDRESS", __FUNCTION__) +# endif + +#else /* !__ANDROID__ */ + +# ifndef CORRUPTION_ERROR_ACTION +# define CORRUPTION_ERROR_ACTION(m) ABORT +# endif /* CORRUPTION_ERROR_ACTION */ + +# ifndef USAGE_ERROR_ACTION +# define USAGE_ERROR_ACTION(m,p) ABORT +# endif /* USAGE_ERROR_ACTION */ + +#endif /* !__ANDROID__ */ -#ifndef USAGE_ERROR_ACTION -#define USAGE_ERROR_ACTION(m,p) ABORT -#endif /* USAGE_ERROR_ACTION */ #endif /* PROCEED_ON_ERROR */ diff --git a/libc/bionic/logd_write.c b/libc/bionic/logd_write.c index 63dfd596b..2bc39fa4f 100644 --- a/libc/bionic/logd_write.c +++ b/libc/bionic/logd_write.c @@ -48,6 +48,16 @@ #include +/* IMPORTANT IMPORTANT IMPORTANT: TECHNICAL NOTE + * + * Some of the functions below can be called when our malloc() implementation + * has detected that the heap is corrupted, or even from a signal handler. + * + * These functions should *not* use a function that allocates heap memory + * or is not signal-safe. Using direct system calls is acceptable, and we + * also assume that pthread_mutex_lock/unlock can be used too. + */ + #define LOG_BUF_SIZE 1024 typedef enum { @@ -77,9 +87,10 @@ static log_channel_t log_channels[LOG_ID_MAX] = { { __write_to_log_init, -1, "/dev/"LOGGER_LOG_RADIO } }; +/* Important: see technical note at start of source file */ static int __write_to_log_null(log_id_t log_id, struct iovec *vec) { - /* + /* * ALTERED behaviour from previous version * always returns successful result */ @@ -97,23 +108,21 @@ static int __write_to_log_null(log_id_t log_id, struct iovec *vec) * it's supposed, that log_id contains valid id always. * this check must be performed in higher level functions */ +/* Important: see technical note at start of source file */ static int __write_to_log_kernel(log_id_t log_id, struct iovec *vec) { - ssize_t ret; - - do { - ret = writev(log_channels[log_id].fd, vec, 3); - } while ((ret < 0) && (errno == EINTR)); - - return ret; + return TEMP_FAILURE_RETRY( writev(log_channels[log_id].fd, vec, 3) ); } +/* Important: see technical note at start of source file */ static int __write_to_log_init(log_id_t log_id, struct iovec *vec) { if ((LOG_ID_NONE < log_id) && (log_id < LOG_ID_MAX)) { + int fd; + pthread_mutex_lock(&log_init_lock); - int fd = open(log_channels[log_id].path, O_WRONLY); + fd = TEMP_FAILURE_RETRY(open(log_channels[log_id].path, O_WRONLY)); log_channels[log_id].logger = (fd < 0) ? __write_to_log_null : __write_to_log_kernel; @@ -130,7 +139,9 @@ static int __write_to_log_init(log_id_t log_id, struct iovec *vec) return -1; } -static int __android_log_write(int prio, const char *tag, const char *msg) +/* Important: see technical note at start of source file */ +__LIBC_HIDDEN__ +int __libc_android_log_write(int prio, const char *tag, const char *msg) { struct iovec vec[3]; log_id_t log_id = LOG_ID_MAIN; @@ -151,7 +162,11 @@ static int __android_log_write(int prio, const char *tag, const char *msg) return log_channels[log_id].logger(log_id, vec); } - +/* The functions below are not designed to be called from a heap panic + * function or from a signal handler. As such, they are free to use complex + * C library functions like vsnprintf() + */ +__LIBC_HIDDEN__ int __libc_android_log_vprint(int prio, const char *tag, const char *fmt, va_list ap) { @@ -159,9 +174,10 @@ int __libc_android_log_vprint(int prio, const char *tag, const char *fmt, vsnprintf(buf, LOG_BUF_SIZE, fmt, ap); - return __android_log_write(prio, tag, buf); + return __libc_android_log_write(prio, tag, buf); } +__LIBC_HIDDEN__ int __libc_android_log_print(int prio, const char *tag, const char *fmt, ...) { va_list ap; @@ -171,20 +187,21 @@ int __libc_android_log_print(int prio, const char *tag, const char *fmt, ...) vsnprintf(buf, LOG_BUF_SIZE, fmt, ap); va_end(ap); - return __android_log_write(prio, tag, buf); + return __libc_android_log_write(prio, tag, buf); } +__LIBC_HIDDEN__ int __libc_android_log_assert(const char *cond, const char *tag, const char *fmt, ...) { va_list ap; - char buf[LOG_BUF_SIZE]; + char buf[LOG_BUF_SIZE]; va_start(ap, fmt); vsnprintf(buf, LOG_BUF_SIZE, fmt, ap); va_end(ap); - __android_log_write(ANDROID_LOG_FATAL, tag, buf); + __libc_android_log_write(ANDROID_LOG_FATAL, tag, buf); exit(1); diff --git a/libc/private/logd.h b/libc/private/logd.h index 43fa74235..4a9b62e83 100644 --- a/libc/private/logd.h +++ b/libc/private/logd.h @@ -44,6 +44,7 @@ enum { ANDROID_LOG_SILENT, /* only for SetMinPriority(); must be last */ }; +int __libc_android_log_write(int prio, const char* tag, const char* buffer); int __libc_android_log_print(int prio, const char *tag, const char *fmt, ...); int __libc_android_log_vprint(int prio, const char *tag, const char *fmt, va_list ap); -- 2.11.0