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>
This commit is contained in:
Suren Baghdasaryan 2018-10-12 11:28:33 -07:00
parent 2534d2e54d
commit 8b00c6d79a
1 changed files with 16 additions and 8 deletions

24
lmkd.c
View File

@ -612,6 +612,10 @@ static void cmd_procremove(LMKD_CTRL_PACKET packet) {
} }
lmkd_pack_get_procremove(packet, &params); 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); pid_remove(params.pid);
} }
@ -1135,6 +1139,7 @@ static int kill_one_process(struct proc* procp) {
char *taskname; char *taskname;
int tasksize; int tasksize;
int r; int r;
int result = -1;
#ifdef LMKD_LOG_STATS #ifdef LMKD_LOG_STATS
struct memory_stat mem_st = {}; struct memory_stat mem_st = {};
@ -1143,14 +1148,12 @@ static int kill_one_process(struct proc* procp) {
taskname = proc_get_name(pid); taskname = proc_get_name(pid);
if (!taskname) { if (!taskname) {
pid_remove(pid); goto out;
return -1;
} }
tasksize = proc_get_size(pid); tasksize = proc_get_size(pid);
if (tasksize <= 0) { if (tasksize <= 0) {
pid_remove(pid); goto out;
return -1;
} }
#ifdef LMKD_LOG_STATS #ifdef LMKD_LOG_STATS
@ -1169,13 +1172,12 @@ static int kill_one_process(struct proc* procp) {
r = kill(pid, SIGKILL); r = kill(pid, SIGKILL);
ALOGI("Kill '%s' (%d), uid %d, oom_adj %d to free %ldkB", ALOGI("Kill '%s' (%d), uid %d, oom_adj %d to free %ldkB",
taskname, pid, uid, procp->oomadj, tasksize * page_k); taskname, pid, uid, procp->oomadj, tasksize * page_k);
pid_remove(pid);
TRACE_KILL_END(); TRACE_KILL_END();
if (r) { if (r) {
ALOGE("kill(%d): errno=%d", pid, errno); ALOGE("kill(%d): errno=%d", pid, errno);
return -1; goto out;
} else { } else {
#ifdef LMKD_LOG_STATS #ifdef LMKD_LOG_STATS
if (memory_stat_parse_result == 0) { if (memory_stat_parse_result == 0) {
@ -1187,10 +1189,16 @@ static int kill_one_process(struct proc* procp) {
-1, -1, tasksize * BYTES_IN_KILOBYTE, -1, -1); -1, -1, tasksize * BYTES_IN_KILOBYTE, -1, -1);
} }
#endif #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;
} }
/* /*