From 58ba58a97c8ec56b2c2a32d6cda19a3a57e3cccf Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Tue, 7 Apr 2015 01:25:43 -0700 Subject: logd: Don't embed a flexible array member within another struct C (but not C++) has a concept of a flexible array member, which is documented at https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html . Using a flexible array member indicates that the structure is really a header for a variable length object. In logd's case, the variable length structure android_event_string_t was embedded within another structure called android_log_event_string_t. This makes gcc's __builtin_object_size() function really confused. When compiling with C++, __builtin_object_size(android_log_event_string_t.payload.data, 1) would return 0, whereas if you compiled the code with C, the same call would (properly) return -1. Code which does automatic bounds checking, such as the proposed patch at https://android-review.googlesource.com/145411 , will cause problems for logd if this syntax is used. Don't try to embed a variable length structure within another structure. This doesn't appear to be valid C nor C++, and while it's worked, it seems problematic. Instead, inline the structure so it's one big happy structure. Change-Id: I8ac02b7142a4f6560f5f80df2effcf720f9896fc --- include/private/android_logger.h | 16 ++++++++++++++-- logd/LogAudit.cpp | 6 +++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/include/private/android_logger.h b/include/private/android_logger.h index 724ca51..04238a6 100644 --- a/include/private/android_logger.h +++ b/include/private/android_logger.h @@ -70,7 +70,17 @@ typedef struct __attribute__((__packed__)) { android_event_long_t payload; } android_log_event_long_t; -/* Event payload EVENT_TYPE_STRING */ +/* + * Event payload EVENT_TYPE_STRING + * + * Danger: do not embed this structure into another structure. + * This structure uses a flexible array member, and when + * compiled using g++, __builtin_object_size(data, 1) returns + * a bad value. This is possibly a g++ bug, or a bug due to + * the fact that flexible array members are not supported + * in C++. + * http://stackoverflow.com/questions/4412749/are-flexible-array-members-valid-in-c + */ typedef struct __attribute__((__packed__)) { int8_t type; // EVENT_TYPE_STRING; int32_t length; // Little Endian Order @@ -80,7 +90,9 @@ typedef struct __attribute__((__packed__)) { /* Event with single EVENT_TYPE_STRING */ typedef struct __attribute__((__packed__)) { android_event_header_t header; - android_event_string_t payload; + int8_t type; // EVENT_TYPE_STRING; + int32_t length; // Little Endian Order + char data[]; } android_log_event_string_t; #endif diff --git a/logd/LogAudit.cpp b/logd/LogAudit.cpp index 6b3e637..bdb2915 100644 --- a/logd/LogAudit.cpp +++ b/logd/LogAudit.cpp @@ -150,9 +150,9 @@ int LogAudit::logPrint(const char *fmt, ...) { rc = -ENOMEM; } else { event->header.tag = htole32(AUDITD_LOG_TAG); - event->payload.type = EVENT_TYPE_STRING; - event->payload.length = htole32(l); - memcpy(event->payload.data, str, l); + event->type = EVENT_TYPE_STRING; + event->length = htole32(l); + memcpy(event->data, str, l); logbuf->log(LOG_ID_EVENTS, now, uid, pid, tid, reinterpret_cast(event), -- cgit v1.1