commit 010632748657423d17f9fd4c2908e5bb92aa0dbb
author Suren Baghdasaryan <surenb@google.com> Fri Oct 12 11:28:33 2018 -0700
committer Suren Baghdasaryan <surenb@google.com> Fri Oct 12 11:28:33 2018 -0700
tree c6b6a9024126b6f501fbc7612314ce57de8928e8
parent c53a0e9158bbfb2a4ffa1a15f15bb7450d3fb8d9

lmkd: Fix an invalid access to a pointer after it's freed

pid_remove() frees a structure representing registered process and the
pointer can't be used anymore. This change fixes an instance when pointer
was used after it was freed. pid_remove() is moved to the end of the
function and comments are added to prevent similar situation in the future.

Bug: 117625315

Change-Id: I6a922952a31232497b3f9caf87d5a21bd402db94
Signed-off-by: Suren Baghdasaryan <surenb@google.com>

lmkd/lmkd.c

diff --git a/lmkd/lmkd.c b/lmkd/lmkd.c
index 8b68dca..cf0bef8 100644
--- a/lmkd/lmkd.c
+++ b/lmkd/lmkd.c
@@ -612,6 +612,10 @@
     }
 
     lmkd_pack_get_procremove(packet, &params);
+    /*
+     * WARNING: After pid_remove() procp is freed and can't be used!
+     * Therefore placed at the end of the function.
+     */
     pid_remove(params.pid);
 }
 
@@ -1135,6 +1139,7 @@
     char *taskname;
     int tasksize;
     int r;
+    int result = -1;
 
 #ifdef LMKD_LOG_STATS
     struct memory_stat mem_st = {};
@@ -1143,14 +1148,12 @@
 
     taskname = proc_get_name(pid);
     if (!taskname) {
-        pid_remove(pid);
-        return -1;
+        goto out;
     }
 
     tasksize = proc_get_size(pid);
     if (tasksize <= 0) {
-        pid_remove(pid);
-        return -1;
+        goto out;
     }
 
 #ifdef LMKD_LOG_STATS
@@ -1169,13 +1172,12 @@
     r = kill(pid, SIGKILL);
     ALOGI("Kill '%s' (%d), uid %d, oom_adj %d to free %ldkB",
         taskname, pid, uid, procp->oomadj, tasksize * page_k);
-    pid_remove(pid);
 
     TRACE_KILL_END();
 
     if (r) {
         ALOGE("kill(%d): errno=%d", pid, errno);
-        return -1;
+        goto out;
     } else {
 #ifdef LMKD_LOG_STATS
         if (memory_stat_parse_result == 0) {
@@ -1187,10 +1189,16 @@
                                           -1, -1, tasksize * BYTES_IN_KILOBYTE, -1, -1);
         }
 #endif
-        return tasksize;
+        result = tasksize;
     }
 
-    return tasksize;
+out:
+    /*
+     * WARNING: After pid_remove() procp is freed and can't be used!
+     * Therefore placed at the end of the function.
+     */
+    pid_remove(pid);
+    return result;
 }
 
 /*