From 01bdb04be63bce0c81d1d07a77699631e913ceb5 Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Wed, 12 Apr 2017 15:11:24 -0700 Subject: [PATCH] liblog: allow event tags to include some punctuations event_log_tag parser complains about a period (.) in the name, we would consider such an enhancement to the tag names possible. I expect we would want to be able to support alphanumerics, underscore (_), period (.), minus (-), at (@) and comma (,) for starters as they are present in the other text log buffer tags. We introduce a local endOfTag function that is used during parsing and during android_lookupEventTagNum for submitting new tags. This function caused us to enforce const char more closely. By filtering in both places we resolve an issue that could have plagued us if garbage requests were made. Test: gTest liblog-unit-tests, logd-unit-tests & logcat-unit-tests Bug: 31456426 Change-Id: I596b8706e843719ddac07ec40e1cd2875c214bed --- liblog/event_tag_map.cpp | 53 +++++++++++++++++++++--------------- logcat/tests/logcat_test.cpp | 2 +- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/liblog/event_tag_map.cpp b/liblog/event_tag_map.cpp index 0b977c21e..73ed16f33 100644 --- a/liblog/event_tag_map.cpp +++ b/liblog/event_tag_map.cpp @@ -229,9 +229,16 @@ int EventTagMap::find(MapString&& tag) const { return it->second; } +// The position after the end of a valid section of the tag string, +// caller makes sure delimited appropriately. +static const char* endOfTag(const char* cp) { + while (*cp && (isalnum(*cp) || strchr("_.-@,", *cp))) ++cp; + return cp; +} + // Scan one tag line. // -// "*pData" should be pointing to the first digit in the tag number. On +// "pData" should be pointing to the first digit in the tag number. On // successful return, it will be pointing to the last character in the // tag line (i.e. the character before the start of the next line). // @@ -241,10 +248,11 @@ int EventTagMap::find(MapString&& tag) const { // data and it will outlive the call. // // Returns 0 on success, nonzero on failure. -static int scanTagLine(EventTagMap* map, char** pData, int lineNum) { - char* cp; - unsigned long val = strtoul(*pData, &cp, 10); - if (cp == *pData) { +static int scanTagLine(EventTagMap* map, const char*& pData, int lineNum) { + char* ep; + unsigned long val = strtoul(pData, &ep, 10); + const char* cp = ep; + if (cp == pData) { if (lineNum) { fprintf(stderr, OUT_TAG ": malformed tag number on line %d\n", lineNum); } @@ -273,14 +281,13 @@ static int scanTagLine(EventTagMap* map, char** pData, int lineNum) { } const char* tag = cp; - // Determine whether "c" is a valid tag char. - while (isalnum(*++cp) || (*cp == '_')) { - } + cp = endOfTag(cp); size_t tagLen = cp - tag; if (!isspace(*cp)) { if (lineNum) { - fprintf(stderr, OUT_TAG ": invalid tag chars on line %d\n", lineNum); + fprintf(stderr, OUT_TAG ": invalid tag char %c on line %d\n", *cp, + lineNum); } errno = EINVAL; return -1; @@ -311,9 +318,9 @@ static int scanTagLine(EventTagMap* map, char** pData, int lineNum) { while (*cp != '\n') ++cp; #ifdef DEBUG - fprintf(stderr, "%d: %p: %.*s\n", lineNum, tag, (int)(cp - *pData), *pData); + fprintf(stderr, "%d: %p: %.*s\n", lineNum, tag, (int)(cp - pData), pData); #endif - *pData = cp; + pData = cp; if (lineNum) { if (map->emplaceUnique(tagIndex, @@ -341,9 +348,9 @@ static const char* eventTagFiles[NUM_MAPS] = { // Parse the tags out of the file. static int parseMapLines(EventTagMap* map, size_t which) { - char* cp = static_cast(map->mapAddr[which]); + const char* cp = static_cast(map->mapAddr[which]); size_t len = map->mapLen[which]; - char* endp = cp + len; + const char* endp = cp + len; // insist on EOL at EOF; simplifies parsing and null-termination if (!len || (*(endp - 1) != '\n')) { @@ -370,7 +377,7 @@ static int parseMapLines(EventTagMap* map, size_t which) { lineStart = false; } else if (isdigit(*cp)) { // looks like a tag; scan it out - if (scanTagLine(map, &cp, lineNum) != 0) { + if (scanTagLine(map, cp, lineNum) != 0) { if (!which || (errno != EMLINK)) { return -1; } @@ -495,14 +502,13 @@ static const TagFmt* __getEventTag(EventTagMap* map, unsigned int tag) { int ret = asprintf(&buf, command_template, tag); if (ret > 0) { // Add some buffer margin for an estimate of the full return content. - char* cp; size_t size = ret - strlen(command_template) + strlen("65535\n4294967295\t?\t\t\t?\t# uid=32767\n\n\f?success?"); if (size > (size_t)ret) { - cp = static_cast(realloc(buf, size)); - if (cp) { - buf = cp; + char* np = static_cast(realloc(buf, size)); + if (np) { + buf = np; } else { size = ret; } @@ -512,10 +518,12 @@ static const TagFmt* __getEventTag(EventTagMap* map, unsigned int tag) { // Ask event log tag service for an existing entry if (__send_log_msg(buf, size) >= 0) { buf[size - 1] = '\0'; - unsigned long val = strtoul(buf, &cp, 10); // return size + char* ep; + unsigned long val = strtoul(buf, &ep, 10); // return size + const char* cp = ep; if ((buf != cp) && (val > 0) && (*cp == '\n')) { // truncation OK ++cp; - if (!scanTagLine(map, &cp, 0)) { + if (!scanTagLine(map, cp, 0)) { free(buf); return map->find(tag); } @@ -573,8 +581,9 @@ LIBLOG_ABI_PUBLIC const char* android_lookupEventTag(const EventTagMap* map, LIBLOG_ABI_PUBLIC int android_lookupEventTagNum(EventTagMap* map, const char* tagname, const char* format, int prio) { - size_t len = strlen(tagname); - if (!len) { + const char* ep = endOfTag(tagname); + size_t len = ep - tagname; + if (!len || *ep) { errno = EINVAL; return -1; } diff --git a/logcat/tests/logcat_test.cpp b/logcat/tests/logcat_test.cpp index 33355e4f5..e487a9720 100644 --- a/logcat/tests/logcat_test.cpp +++ b/logcat/tests/logcat_test.cpp @@ -1677,7 +1677,7 @@ TEST(logcat, descriptive) { // Invent new entries because existing can not serve EventTagMap* map = android_openEventTagMap(nullptr); ASSERT_TRUE(nullptr != map); - static const char name[] = ___STRING(logcat) "_descriptive_monotonic"; + static const char name[] = ___STRING(logcat) ".descriptive-monotonic"; int myTag = android_lookupEventTagNum(map, name, "(new|1|s)", ANDROID_LOG_UNKNOWN); android_closeEventTagMap(map);