From ddb842ac9342438aae65f8aaf8ce66fb41ca65a1 Mon Sep 17 00:00:00 2001
From: Mark Salyzyn <salyzyn@google.com>
Date: Tue, 10 Nov 2015 15:56:41 -0800
Subject: [PATCH] logd: klogd: parse error

(cherry pick from commit eb298086153a9502bb0b02a1a26919a482b52535)

Resolve three areas missing taglen checking. Add some additional
limit-checking paranoia. Problem started when p was allowed to
go beyond the size of the incoming buffer in some blind
p = cp + 1 fragments, placed the check for that after them all
before harm could be done, rather than in each location.

Bug: 25620123
Change-Id: Ib5687fd30ef0cd3ba3bc0df310b436ad675ccabc
---
 logd/LogKlog.cpp | 38 +++++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/logd/LogKlog.cpp b/logd/LogKlog.cpp
index da5e78d66..2a3f52f99 100644
--- a/logd/LogKlog.cpp
+++ b/logd/LogKlog.cpp
@@ -582,7 +582,7 @@ int LogKlog::log(const char *buf, size_t len) {
     // Some may view the following as an ugly heuristic, the desire is to
     // beautify the kernel logs into an Android Logging format; the goal is
     // admirable but costly.
-    while ((isspace(*p) || !*p) && (p < &buf[len])) {
+    while ((p < &buf[len]) && (isspace(*p) || !*p)) {
         ++p;
     }
     if (p >= &buf[len]) { // timestamp, no content
@@ -596,7 +596,7 @@ int LogKlog::log(const char *buf, size_t len) {
         const char *bt, *et, *cp;
 
         bt = p;
-        if (!fast<strncmp>(p, "[INFO]", 6)) {
+        if ((taglen >= 6) && !fast<strncmp>(p, "[INFO]", 6)) {
             // <PRI>[<TIME>] "[INFO]"<tag> ":" message
             bt = p + 6;
             taglen -= 6;
@@ -620,7 +620,9 @@ int LogKlog::log(const char *buf, size_t len) {
             p = cp + 1;
         } else if (taglen) {
             size = et - bt;
-            if ((*bt == *cp) && fast<strncmp>(bt + 1, cp + 1, size - 1)) {
+            if ((taglen > size) &&   // enough space for match plus trailing :
+                    (*bt == *cp) &&  // ubber fast<strncmp> pair
+                    fast<strncmp>(bt + 1, cp + 1, size - 1)) {
                 // <PRI>[<TIME>] <tag>_host '<tag>.<num>' : message
                 if (!fast<strncmp>(bt + size - 5, "_host", 5)
                         && !fast<strncmp>(bt + 1, cp + 1, size - 6)) {
@@ -694,7 +696,7 @@ int LogKlog::log(const char *buf, size_t len) {
                     p = cp + 1;
                 }
             }
-        }
+        } /* else no tag */
         size = etag - tag;
         if ((size <= 1)
             // register names like x9
@@ -721,8 +723,12 @@ int LogKlog::log(const char *buf, size_t len) {
             taglen = mp - tag;
         }
     }
+    // Deal with sloppy and simplistic harmless p = cp + 1 etc above.
+    if (len < (size_t)(p - buf)) {
+        p = &buf[len];
+    }
     // skip leading space
-    while ((isspace(*p) || !*p) && (p < &buf[len])) {
+    while ((p < &buf[len]) && (isspace(*p) || !*p)) {
         ++p;
     }
     // truncate trailing space or nuls
@@ -735,16 +741,26 @@ int LogKlog::log(const char *buf, size_t len) {
         p = " ";
         b = 1;
     }
+    // paranoid sanity check, can not happen ...
     if (b > LOGGER_ENTRY_MAX_PAYLOAD) {
         b = LOGGER_ENTRY_MAX_PAYLOAD;
     }
+    if (taglen > LOGGER_ENTRY_MAX_PAYLOAD) {
+        taglen = LOGGER_ENTRY_MAX_PAYLOAD;
+    }
+    // calculate buffer copy requirements
     size_t n = 1 + taglen + 1 + b + 1;
-    int rc = n;
-    if ((taglen > n) || (b > n)) { // Can not happen ...
-        rc = -EINVAL;
-        return rc;
+    // paranoid sanity check, first two just can not happen ...
+    if ((taglen > n) || (b > n) || (n > USHRT_MAX)) {
+        return -EINVAL;
     }
 
+    // Careful.
+    // We are using the stack to house the log buffer for speed reasons.
+    // If we malloc'd this buffer, we could get away without n's USHRT_MAX
+    // test above, but we would then required a max(n, USHRT_MAX) as
+    // truncating length argument to logbuf->log() below. Gain is protection
+    // of stack sanity and speedup, loss is truncated long-line content.
     char newstr[n];
     char *np = newstr;
 
@@ -763,8 +779,8 @@ int LogKlog::log(const char *buf, size_t len) {
     np[b] = '\0';
 
     // Log message
-    rc = logbuf->log(LOG_ID_KERNEL, now, uid, pid, tid, newstr,
-                     (n <= USHRT_MAX) ? (unsigned short) n : USHRT_MAX);
+    int rc = logbuf->log(LOG_ID_KERNEL, now, uid, pid, tid, newstr,
+                         (unsigned short) n);
 
     // notify readers
     if (!rc) {