wenchao-hao
/
lmkd
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

lmkd: Fix UAF caused by calling pid_remove() from the watchdog thread 

pid_remove() is not a thread-safe function and can be called only from
the main thread. Calling it from the watchdog_callback() executed in the
context of the watchdog thread can cause a use-after-free failure if the
same record is being used by the main thread. Fix this issue by marking
the process record as invalid instead of destroying it. Later on invalid
records will be cleared in kill_one_process() called from the context of
the main thread.

Fixes: f8727745f9 ("lmkd: Remove process record after it is killed by lmkd watchdog")
Bug: 248448498
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: I0c7776aea1518c17f0a29904a44b7fe8f33980ca
This commit is contained in:
Suren Baghdasaryan 2022-09-27 14:30:34 -07:00
parent 26c9de4e72
commit a3802f16cf
1 changed files with 17 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
lmkd.cpp
View File

@ -511,6 +511,7 @@ struct proc {
uid_t uid;
int oomadj;
pid_t reg_pid; /* PID of the process that registered this record */
bool valid;
struct proc *pidhash_next;
};
@ -942,6 +943,7 @@ static void proc_insert(struct proc *procp) {
proc_slot(procp);
}
// Can be called only from the main thread.
static int pid_remove(int pid) {
int hval = pid_hashfn(pid);
struct proc *procp;
@ -971,6 +973,15 @@ static int pid_remove(int pid) {
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.
* Returns false if the file does not exist.
@ -1221,6 +1232,7 @@ static void cmd_procprio(LMKD_CTRL_PACKET packet, int field_count, struct ucred
procp->uid = params.uid;
procp->reg_pid = cred->pid;
procp->oomadj = params.oomadj;
procp->valid = true;
proc_insert(procp);
} else {
if (!claim_record(procp, cred->pid)) {
@ -2088,7 +2100,7 @@ static struct proc *proc_adj_prev(int oomadj, int pid) {
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) {
struct adjslot_list *head = &procadjslot_list[ADJTOSLOT(oomadj)];
struct adjslot_list *curr = head->next;
@ -2150,11 +2162,11 @@ static void watchdog_callback() {
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);
killinfo_log(&target, 0, 0, 0, NULL, NULL, NULL, NULL, NULL);
// WARNING: do not use target after pid_remove()
pid_remove(target.pid);
// Can't call pid_remove() from non-main thread, therefore just invalidate the record
pid_invalidate(target.pid);
break;
}
prev_pid = target.pid;
@ -2292,7 +2304,7 @@ static int kill_one_process(struct proc* procp, int min_oom_score, struct kill_i
char buf[PAGE_SIZE];
char desc[LINE_MAX];
if (!read_proc_status(pid, buf, sizeof(buf))) {
if (!procp->valid || !read_proc_status(pid, buf, sizeof(buf))) {
goto out;
}
if (!parse_status_tag(buf, PROC_STATUS_TGID_FIELD, &tgid)) {