lmkd: Fix UAF caused by calling pid_remove() from the watchdog thread am: ba9ea6e3d6 am: d5dbec8d23 am: 02af677a49

Original change: https://android-review.googlesource.com/c/platform/system/memory/lmkd/+/2235075

Change-Id: I166f3e4f03e886d5a6d9c684267ac46ec1f9d698
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Suren Baghdasaryan 2022-09-29 19:43:01 +00:00 committed by Automerger Merge Worker
commit 6057c60d5b
1 changed files with 17 additions and 5 deletions

View File

@ -510,6 +510,7 @@ struct proc {
uid_t uid; uid_t uid;
int oomadj; int oomadj;
pid_t reg_pid; /* PID of the process that registered this record */ pid_t reg_pid; /* PID of the process that registered this record */
bool valid;
struct proc *pidhash_next; struct proc *pidhash_next;
}; };
@ -941,6 +942,7 @@ static void proc_insert(struct proc *procp) {
proc_slot(procp); proc_slot(procp);
} }
// Can be called only from the main thread.
static int pid_remove(int pid) { static int pid_remove(int pid) {
int hval = pid_hashfn(pid); int hval = pid_hashfn(pid);
struct proc *procp; struct proc *procp;
@ -970,6 +972,15 @@ static int pid_remove(int pid) {
return 0; return 0;
} }
static void pid_invalidate(int pid) {
std::shared_lock lock(adjslot_list_lock);
struct proc *procp = pid_lookup(pid);
if (procp) {
procp->valid = false;
}
}
/* /*
* Write a string to a file. * Write a string to a file.
* Returns false if the file does not exist. * Returns false if the file does not exist.
@ -1220,6 +1231,7 @@ static void cmd_procprio(LMKD_CTRL_PACKET packet, int field_count, struct ucred
procp->uid = params.uid; procp->uid = params.uid;
procp->reg_pid = cred->pid; procp->reg_pid = cred->pid;
procp->oomadj = params.oomadj; procp->oomadj = params.oomadj;
procp->valid = true;
proc_insert(procp); proc_insert(procp);
} else { } else {
if (!claim_record(procp, cred->pid)) { if (!claim_record(procp, cred->pid)) {
@ -2092,7 +2104,7 @@ static struct proc *proc_adj_prev(int oomadj, int pid) {
return NULL; return NULL;
} }
// When called from a non-main thread, adjslot_list_lock read lock should be taken. // Can be called only from the main thread.
static struct proc *proc_get_heaviest(int oomadj) { static struct proc *proc_get_heaviest(int oomadj) {
struct adjslot_list *head = &procadjslot_list[ADJTOSLOT(oomadj)]; struct adjslot_list *head = &procadjslot_list[ADJTOSLOT(oomadj)];
struct adjslot_list *curr = head->next; struct adjslot_list *curr = head->next;
@ -2154,11 +2166,11 @@ static void watchdog_callback() {
continue; continue;
} }
if (reaper.kill({ target.pidfd, target.pid, target.uid }, true) == 0) { if (target.valid && reaper.kill({ target.pidfd, target.pid, target.uid }, true) == 0) {
ALOGW("lmkd watchdog killed process %d, oom_score_adj %d", target.pid, oom_score); ALOGW("lmkd watchdog killed process %d, oom_score_adj %d", target.pid, oom_score);
killinfo_log(&target, 0, 0, 0, NULL, NULL, NULL, NULL, NULL); killinfo_log(&target, 0, 0, 0, NULL, NULL, NULL, NULL, NULL);
// WARNING: do not use target after pid_remove() // Can't call pid_remove() from non-main thread, therefore just invalidate the record
pid_remove(target.pid); pid_invalidate(target.pid);
break; break;
} }
prev_pid = target.pid; prev_pid = target.pid;
@ -2296,7 +2308,7 @@ static int kill_one_process(struct proc* procp, int min_oom_score, struct kill_i
char buf[PAGE_SIZE]; char buf[PAGE_SIZE];
char desc[LINE_MAX]; char desc[LINE_MAX];
if (!read_proc_status(pid, buf, sizeof(buf))) { if (!procp->valid || !read_proc_status(pid, buf, sizeof(buf))) {
goto out; goto out;
} }
if (!parse_status_tag(buf, PROC_STATUS_TGID_FIELD, &tgid)) { if (!parse_status_tag(buf, PROC_STATUS_TGID_FIELD, &tgid)) {