From a0140047525b3c12e9e64903fb7b85fea0d5a0f9 Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Fri, 13 Mar 2015 06:50:32 -0700 Subject: [PATCH 1/2] liblog: add __android_log_is_loggable checking to writer Add __android_log_is_loggable() checking for all buffers except LOG_ID_SECURITY. Return -EPERM if blocked. Since we are sniffing the log tag, check validity and return -EINVAL. NB: Try not to call __android_log_is_loggable() in native code within a signal handler. Both here, and in the system properties, there are locking paths that are not guaranteed to play well in that environment. This has also been the case for the log writer path even before this change. All attempts have been made to use trylock, and to use a more expensive code path when contention occurs rather than lead to deadlock. Bug: 19544788 Bug: 26178938 Change-Id: I98738c662f6328189a6703251eb8721a05e956f9 --- liblog/logd_write.c | 91 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 90 insertions(+), 1 deletion(-) diff --git a/liblog/logd_write.c b/liblog/logd_write.c index c2b0ec2ea..5406c50e1 100644 --- a/liblog/logd_write.c +++ b/liblog/logd_write.c @@ -39,6 +39,7 @@ #include #endif +#include #include #include #include @@ -71,6 +72,11 @@ static void lock() pthread_mutex_lock(&log_init_lock); } +static int trylock() +{ + return pthread_mutex_trylock(&log_init_lock); +} + static void unlock() { pthread_mutex_unlock(&log_init_lock); @@ -79,6 +85,7 @@ static void unlock() #else /* !defined(_WIN32) */ #define lock() ((void)0) +#define trylock() (0) /* success */ #define unlock() ((void)0) #endif /* !defined(_WIN32) */ @@ -194,6 +201,9 @@ static int __write_to_log_daemon(log_id_t log_id, struct iovec *vec, size_t nr) last_pid = getpid(); } if (log_id == LOG_ID_SECURITY) { + if (vec[0].iov_len < 4) { + return -EINVAL; + } if ((last_uid != AID_SYSTEM) && (last_uid != AID_ROOT)) { uid_t uid = geteuid(); if ((uid != AID_SYSTEM) && (uid != AID_ROOT)) { @@ -207,9 +217,86 @@ static int __write_to_log_daemon(log_id_t log_id, struct iovec *vec, size_t nr) } } if (!__android_log_security()) { + atomic_store(&dropped_security, 0); + return -EPERM; + } + } else if (log_id == LOG_ID_EVENTS) { + static atomic_uintptr_t map; + int ret; + const char *tag; + EventTagMap *m, *f; + + if (vec[0].iov_len < 4) { + return -EINVAL; + } + + tag = NULL; + f = NULL; + m = (EventTagMap *)atomic_load(&map); + + if (!m) { + ret = trylock(); + m = (EventTagMap *)atomic_load(&map); /* trylock flush cache */ + if (!m) { + m = android_openEventTagMap(EVENT_TAG_MAP_FILE); + if (ret) { /* trylock failed, use local copy, mark for close */ + f = m; + } else { + if (!m) { /* One chance to open map file */ + m = (EventTagMap *)(uintptr_t)-1LL; + } + atomic_store(&map, (uintptr_t)m); + } + } + if (!ret) { /* trylock succeeded, unlock */ + unlock(); + } + } + if (m && (m != (EventTagMap *)(uintptr_t)-1LL)) { + tag = android_lookupEventTag( + m, + htole32(((uint32_t *)vec[0].iov_base)[0])); + } + ret = __android_log_is_loggable(ANDROID_LOG_INFO, + tag, + ANDROID_LOG_VERBOSE); + if (f) { /* local copy marked for close */ + android_closeEventTagMap(f); + } + if (!ret) { + return -EPERM; + } + } else { + /* Validate the incoming tag, tag content can not split across iovec */ + char prio = ANDROID_LOG_VERBOSE; + const char *tag = vec[0].iov_base; + size_t len = vec[0].iov_len; + if (!tag) { + len = 0; + } + if (len > 0) { + prio = *tag; + if (len > 1) { + --len; + ++tag; + } else { + len = vec[1].iov_len; + tag = ((const char *)vec[1].iov_base); + if (!tag) { + len = 0; + } + } + } + /* tag must be nul terminated */ + if (strnlen(tag, len) >= len) { + tag = NULL; + } + + if (!__android_log_is_loggable(prio, tag, ANDROID_LOG_VERBOSE)) { return -EPERM; } } + /* * struct { * // what we provide to pstore @@ -267,7 +354,9 @@ static int __write_to_log_daemon(log_id_t log_id, struct iovec *vec, size_t nr) } } snapshot = atomic_exchange_explicit(&dropped, 0, memory_order_relaxed); - if (snapshot) { + if (snapshot && __android_log_is_loggable(ANDROID_LOG_INFO, + "liblog", + ANDROID_LOG_VERBOSE)) { android_log_event_int_t buffer; header.id = LOG_ID_EVENTS; From 6aa21b225dd1600473388bd640443653d649420a Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Tue, 22 Dec 2015 15:50:14 -0800 Subject: [PATCH 2/2] logd: liblog: whitelist "snet_event_log" Dangerous bridge to cross to whitelist, who is special, who is not? Rationalized as these events are used to catch exploits on platform. As it stands no one should be allowed to block any messages in the security context, not even for development purposes. Bug: 26178938 Change-Id: Ibdc76bc0fe29ba05be168b623af1c9f41d7edbd2 --- liblog/Android.mk | 2 +- liblog/logd_write.c | 66 ++++++++++++++++++++++----------------------- logd/Android.mk | 4 ++- logd/LogBuffer.cpp | 12 ++++++--- 4 files changed, 44 insertions(+), 40 deletions(-) diff --git a/liblog/Android.mk b/liblog/Android.mk index a183db83e..4ab5006e2 100644 --- a/liblog/Android.mk +++ b/liblog/Android.mk @@ -22,7 +22,7 @@ include $(CLEAR_VARS) # 's/^\([0-9]*\)[ \t]*liblog[ \t].*/-DLIBLOG_LOG_TAG=\1/p' \ # $(LOCAL_PATH)/event.logtags) # so make sure we do not regret hard-coding it as follows: -liblog_cflags := -DLIBLOG_LOG_TAG=1005 +liblog_cflags := -DLIBLOG_LOG_TAG=1005 -DSNET_EVENT_LOG_TAG=1397638484 liblog_host_sources := logd_write.c log_event_write.c fake_log_device.c event.logtags liblog_target_sources := logd_write.c log_event_write.c event_tag_map.c log_time.cpp log_is_loggable.c diff --git a/liblog/logd_write.c b/liblog/logd_write.c index 5406c50e1..ec86e6ba2 100644 --- a/liblog/logd_write.c +++ b/liblog/logd_write.c @@ -221,50 +221,48 @@ static int __write_to_log_daemon(log_id_t log_id, struct iovec *vec, size_t nr) return -EPERM; } } else if (log_id == LOG_ID_EVENTS) { - static atomic_uintptr_t map; - int ret; - const char *tag; - EventTagMap *m, *f; - if (vec[0].iov_len < 4) { return -EINVAL; } + if (((uint32_t *)vec[0].iov_base)[0] != htole32(SNET_EVENT_LOG_TAG)) { + static atomic_uintptr_t map; + int ret; + const char *tag = NULL; + EventTagMap *m, *f = NULL; - tag = NULL; - f = NULL; - m = (EventTagMap *)atomic_load(&map); - - if (!m) { - ret = trylock(); - m = (EventTagMap *)atomic_load(&map); /* trylock flush cache */ + m = (EventTagMap *)atomic_load(&map); if (!m) { - m = android_openEventTagMap(EVENT_TAG_MAP_FILE); - if (ret) { /* trylock failed, use local copy, mark for close */ - f = m; - } else { - if (!m) { /* One chance to open map file */ - m = (EventTagMap *)(uintptr_t)-1LL; + ret = trylock(); + m = (EventTagMap *)atomic_load(&map); /* trylock flush cache */ + if (!m) { + m = android_openEventTagMap(EVENT_TAG_MAP_FILE); + if (ret) { /* trylock failed, local copy, mark for close */ + f = m; + } else { + if (!m) { /* One chance to open map file */ + m = (EventTagMap *)(uintptr_t)-1LL; + } + atomic_store(&map, (uintptr_t)m); } - atomic_store(&map, (uintptr_t)m); + } + if (!ret) { /* trylock succeeded, unlock */ + unlock(); } } - if (!ret) { /* trylock succeeded, unlock */ - unlock(); - } - } - if (m && (m != (EventTagMap *)(uintptr_t)-1LL)) { - tag = android_lookupEventTag( + if (m && (m != (EventTagMap *)(uintptr_t)-1LL)) { + tag = android_lookupEventTag( m, htole32(((uint32_t *)vec[0].iov_base)[0])); - } - ret = __android_log_is_loggable(ANDROID_LOG_INFO, - tag, - ANDROID_LOG_VERBOSE); - if (f) { /* local copy marked for close */ - android_closeEventTagMap(f); - } - if (!ret) { - return -EPERM; + } + ret = __android_log_is_loggable(ANDROID_LOG_INFO, + tag, + ANDROID_LOG_VERBOSE); + if (f) { /* local copy marked for close */ + android_closeEventTagMap(f); + } + if (!ret) { + return -EPERM; + } } } else { /* Validate the incoming tag, tag content can not split across iovec */ diff --git a/logd/Android.mk b/logd/Android.mk index feca8d555..d19c2552b 100644 --- a/logd/Android.mk +++ b/logd/Android.mk @@ -38,7 +38,9 @@ LOCAL_SHARED_LIBRARIES := \ # event_flag := $(call event_logtags,auditd) # event_flag += $(call event_logtags,logd) # so make sure we do not regret hard-coding it as follows: -event_flag := -DAUDITD_LOG_TAG=1003 -DLOGD_LOG_TAG=1004 +event_flag := -DAUDITD_LOG_TAG=1003 \ + -DLOGD_LOG_TAG=1004 \ + -DSNET_EVENT_LOG_TAG=1397638484 LOCAL_CFLAGS := -Werror $(event_flag) diff --git a/logd/LogBuffer.cpp b/logd/LogBuffer.cpp index ec323934b..1b829c602 100644 --- a/logd/LogBuffer.cpp +++ b/logd/LogBuffer.cpp @@ -205,16 +205,20 @@ int LogBuffer::log(log_id_t log_id, log_time realtime, LogBufferElement *elem = new LogBufferElement(log_id, realtime, uid, pid, tid, msg, len); - if (log_id != LOG_ID_SECURITY) { + if (log_id != LOG_ID_SECURITY) { // whitelist LOG_ID_SECURITY int prio = ANDROID_LOG_INFO; - const char *tag = NULL; + const char *tag = (const char *)-1; if (log_id == LOG_ID_EVENTS) { - tag = android::tagToName(elem->getTag()); + // whitelist "snet_event_log" + if (elem->getTag() != SNET_EVENT_LOG_TAG) { + tag = android::tagToName(elem->getTag()); + } } else { prio = *msg; tag = msg + 1; } - if (!__android_log_is_loggable(prio, tag, ANDROID_LOG_VERBOSE)) { + if ((tag != (const char *)-1) && + !__android_log_is_loggable(prio, tag, ANDROID_LOG_VERBOSE)) { // Log traffic received to total pthread_mutex_lock(&mLogElementsLock); stats.add(elem);