commit 9ce18f72203d523e7378ddb3f0432871973df561
author Ray Essick <essick@google.com> Mon May 07 15:54:30 2018 -0700
committer Ray Essick <essick@google.com> Wed May 09 15:45:24 2018 +0000
tree 0b1ab53f1f35f531ac31abdf3c39e6c3b0488152
parent d9ed870e6402de3ec1253c5c2a43ed1d10078518

Fix memory leak in media metrics primitives

code was clearing a pointer without having checked & reclaimed the
referenced memory.

Bug: 79352137
Test: adb shell dumpsys meminfo --unreachable system
Change-Id: I2b8faa806ac99d8e6390880d67b5bc090ef7c19b

media/libmediametrics/MediaAnalyticsItem.cpp

diff --git a/media/libmediametrics/MediaAnalyticsItem.cpp b/media/libmediametrics/MediaAnalyticsItem.cpp
index dc2bec8..135c9b6 100644
--- a/media/libmediametrics/MediaAnalyticsItem.cpp
+++ b/media/libmediametrics/MediaAnalyticsItem.cpp
@@ -248,12 +248,17 @@
 }
 
 void MediaAnalyticsItem::Prop::setName(const char *name, size_t len) {
-    mNameLen = len;
+    free((void *)mName);
     mName = (const char *) malloc(len+1);
+    LOG_ALWAYS_FATAL_IF(mName == NULL,
+                        "failed malloc() for property '%s' (len %zu)",
+                        name, len);
     memcpy ((void *)mName, name, len+1);
+    mNameLen = len;
 }
 
-// used only as part of a storing operation
+// consider this "find-or-allocate".
+// caller validates type and uses clearPropValue() accordingly
 MediaAnalyticsItem::Prop *MediaAnalyticsItem::allocateProp(const char *name) {
     size_t len = strlen(name);
     size_t i = findPropIndex(name, len);
@@ -271,7 +276,6 @@
         i = mPropCount++;
         prop = &mProps[i];
         prop->setName(name, len);
-        prop->mType = kTypeNone;        // make caller set type info
     }
 
     return prop;
@@ -299,6 +303,7 @@
 void MediaAnalyticsItem::setInt32(MediaAnalyticsItem::Attr name, int32_t value) {
     Prop *prop = allocateProp(name);
     if (prop != NULL) {
+        clearPropValue(prop);
         prop->mType = kTypeInt32;
         prop->u.int32Value = value;
     }
@@ -307,6 +312,7 @@
 void MediaAnalyticsItem::setInt64(MediaAnalyticsItem::Attr name, int64_t value) {
     Prop *prop = allocateProp(name);
     if (prop != NULL) {
+        clearPropValue(prop);
         prop->mType = kTypeInt64;
         prop->u.int64Value = value;
     }
@@ -315,6 +321,7 @@
 void MediaAnalyticsItem::setDouble(MediaAnalyticsItem::Attr name, double value) {
     Prop *prop = allocateProp(name);
     if (prop != NULL) {
+        clearPropValue(prop);
         prop->mType = kTypeDouble;
         prop->u.doubleValue = value;
     }
@@ -325,6 +332,7 @@
     Prop *prop = allocateProp(name);
     // any old value will be gone
     if (prop != NULL) {
+        clearPropValue(prop);
         prop->mType = kTypeCString;
         prop->u.CStringValue = strdup(value);
     }
@@ -333,6 +341,7 @@
 void MediaAnalyticsItem::setRate(MediaAnalyticsItem::Attr name, int64_t count, int64_t duration) {
     Prop *prop = allocateProp(name);
     if (prop != NULL) {
+        clearPropValue(prop);
         prop->mType = kTypeRate;
         prop->u.rate.count = count;
         prop->u.rate.duration = duration;
@@ -585,6 +594,9 @@
     // fix any pointers that we blindly copied, so we have our own copies
     if (dst->mName) {
         void *p =  malloc(dst->mNameLen + 1);
+        LOG_ALWAYS_FATAL_IF(p == NULL,
+                            "failed malloc() duping property '%s' (len %zu)",
+                            dst->mName, dst->mNameLen);
         memcpy (p, src->mName, dst->mNameLen + 1);
         dst->mName = (const char *) p;
     }