diff --git a/llkd/README.md b/llkd/README.md index 43bb94ae0..191f98819 100644 --- a/llkd/README.md +++ b/llkd/README.md @@ -165,9 +165,14 @@ size of 92. NB: false is a very very very unlikely process to want to blacklist. #### ro.llk.blacklist.parent -default 0,2,adbd (kernel, [kthreadd] and adbd). +default 0,2,adbd&[setsid] (kernel, [kthreadd] and adbd *only for zombie setsid*). Do not watch processes that have this parent. -A parent process can be comm, cmdline or pid reference. +An ampersand (*&*) separator is used to specify that the parent is ignored +only in combination with the target child process. +Ampersand was selected because it is never part of a process name, +however a setprop in the shell requires it to be escaped or quoted; +init rc file where this is normally specified does not have this issue. +A parent or target processes can be specified as comm, cmdline or pid reference. #### ro.llk.blacklist.uid default *empty* or false, comma separated list of uid numbers or names. diff --git a/llkd/include/llkd.h b/llkd/include/llkd.h index 7b7dbf90f..3586ca1b1 100644 --- a/llkd/include/llkd.h +++ b/llkd/include/llkd.h @@ -56,11 +56,7 @@ unsigned llkCheckMilliseconds(void); #define LLK_BLACKLIST_PROCESS_DEFAULT \ "0,1,2,init,[kthreadd],[khungtaskd],lmkd,llkd,watchdogd,[watchdogd],[watchdogd/0]" #define LLK_BLACKLIST_PARENT_PROPERTY "ro.llk.blacklist.parent" -#ifdef __PTRACE_ENABLED__ // defined if userdebug build -#define LLK_BLACKLIST_PARENT_DEFAULT "0,2,[kthreadd],adbd" -#else -#define LLK_BLACKLIST_PARENT_DEFAULT "0,2,[kthreadd]" -#endif +#define LLK_BLACKLIST_PARENT_DEFAULT "0,2,[kthreadd],adbd&[setsid]" #define LLK_BLACKLIST_UID_PROPERTY "ro.llk.blacklist.uid" #define LLK_BLACKLIST_UID_DEFAULT "" #define LLK_BLACKLIST_STACK_PROPERTY "ro.llk.blacklist.process.stack" diff --git a/llkd/libllkd.cpp b/llkd/libllkd.cpp index 3a593ecc1..3c295b506 100644 --- a/llkd/libllkd.cpp +++ b/llkd/libllkd.cpp @@ -108,6 +108,9 @@ std::unordered_set llkBlacklistProcess; // list of parent pids, comm or cmdline names to skip. default: // kernel pid (0), [kthreadd] (2), or ourselves, enforced and implied std::unordered_set llkBlacklistParent; +// list of parent and target processes to skip. default: +// adbd *and* [setsid] +std::unordered_map> llkBlacklistParentAndChild; // list of uids, and uid names, to skip, default nothing std::unordered_set llkBlacklistUid; #ifdef __PTRACE_ENABLED__ @@ -624,6 +627,19 @@ std::string llkFormat(const std::unordered_set& blacklist) { return ret; } +std::string llkFormat( + const std::unordered_map>& blacklist, + bool leading_comma = false) { + std::string ret; + for (const auto& entry : blacklist) { + for (const auto& target : entry.second) { + if (leading_comma || !ret.empty()) ret += ","; + ret += entry.first + "&" + target; + } + } + return ret; +} + // This function parses the properties as a list, incorporating the supplied // default. A leading comma separator means preserve the defaults and add // entries (with an optional leading + sign), or removes entries with a leading @@ -691,6 +707,27 @@ bool llkSkipProc(proc* procp, return false; } +const std::unordered_set& llkSkipName( + const std::string& name, + const std::unordered_map>& blacklist) { + static const std::unordered_set empty; + if (name.empty() || blacklist.empty()) return empty; + auto found = blacklist.find(name); + if (found == blacklist.end()) return empty; + return found->second; +} + +bool llkSkipPproc(proc* pprocp, proc* procp, + const std::unordered_map>& + blacklist = llkBlacklistParentAndChild) { + if (!pprocp || !procp || blacklist.empty()) return false; + if (llkSkipProc(procp, llkSkipName(std::to_string(pprocp->pid), blacklist))) return true; + if (llkSkipProc(procp, llkSkipName(pprocp->getComm(), blacklist))) return true; + if (llkSkipProc(procp, llkSkipName(pprocp->getCmdline(), blacklist))) return true; + return llkSkipProc(procp, + llkSkipName(android::base::Basename(pprocp->getCmdline()), blacklist)); +} + bool llkSkipPid(pid_t pid) { return llkSkipName(std::to_string(pid), llkBlacklistProcess); } @@ -875,7 +912,8 @@ void llkLogConfig(void) { << LLK_BLACKLIST_STACK_PROPERTY "=" << llkFormat(llkBlacklistStack) << "\n" #endif << LLK_BLACKLIST_PROCESS_PROPERTY "=" << llkFormat(llkBlacklistProcess) << "\n" - << LLK_BLACKLIST_PARENT_PROPERTY "=" << llkFormat(llkBlacklistParent) << "\n" + << LLK_BLACKLIST_PARENT_PROPERTY "=" << llkFormat(llkBlacklistParent) + << llkFormat(llkBlacklistParentAndChild, true) << "\n" << LLK_BLACKLIST_UID_PROPERTY "=" << llkFormat(llkBlacklistUid); } @@ -1050,7 +1088,8 @@ milliseconds llkCheck(bool checkRunning) { break; } - if (llkSkipName(procp->getComm())) { + auto process_comm = procp->getComm(); + if (llkSkipName(process_comm)) { continue; } if (llkSkipName(procp->getCmdline())) { @@ -1065,6 +1104,7 @@ milliseconds llkCheck(bool checkRunning) { pprocp = llkTidAlloc(ppid, ppid, 0, "", 0, '?'); } if (pprocp) { + if (llkSkipPproc(pprocp, procp)) break; if (llkSkipProc(pprocp, llkBlacklistParent)) break; } else { if (llkSkipName(std::to_string(ppid), llkBlacklistParent)) break; @@ -1084,7 +1124,7 @@ milliseconds llkCheck(bool checkRunning) { stuck = true; } else if (procp->count != 0ms) { LOG(VERBOSE) << state << ' ' << llkFormat(procp->count) << ' ' << ppid << "->" - << pid << "->" << tid << ' ' << procp->getComm(); + << pid << "->" << tid << ' ' << process_comm; } } if (!stuck) continue; @@ -1092,7 +1132,7 @@ milliseconds llkCheck(bool checkRunning) { if (procp->count >= llkStateTimeoutMs[(state == 'Z') ? llkStateZ : llkStateD]) { if (procp->count != 0ms) { LOG(VERBOSE) << state << ' ' << llkFormat(procp->count) << ' ' << ppid << "->" - << pid << "->" << tid << ' ' << procp->getComm(); + << pid << "->" << tid << ' ' << process_comm; } continue; } @@ -1120,7 +1160,7 @@ milliseconds llkCheck(bool checkRunning) { break; } LOG(WARNING) << "Z " << llkFormat(procp->count) << ' ' << ppid << "->" - << pid << "->" << tid << ' ' << procp->getComm() << " [kill]"; + << pid << "->" << tid << ' ' << process_comm << " [kill]"; if ((llkKillOneProcess(pprocp, procp) >= 0) || (llkKillOneProcess(ppid, procp) >= 0)) { continue; @@ -1137,7 +1177,7 @@ milliseconds llkCheck(bool checkRunning) { // kernel (worse). default: LOG(WARNING) << state << ' ' << llkFormat(procp->count) << ' ' << pid - << "->" << tid << ' ' << procp->getComm() << " [kill]"; + << "->" << tid << ' ' << process_comm << " [kill]"; if ((llkKillOneProcess(llkTidLookup(pid), procp) >= 0) || (llkKillOneProcess(pid, state, tid) >= 0) || (llkKillOneProcess(procp, procp) >= 0) || @@ -1150,7 +1190,7 @@ milliseconds llkCheck(bool checkRunning) { // We are here because we have confirmed kernel live-lock const auto message = state + " "s + llkFormat(procp->count) + " " + std::to_string(ppid) + "->" + std::to_string(pid) + "->" + - std::to_string(tid) + " " + procp->getComm() + " [panic]"; + std::to_string(tid) + " " + process_comm + " [panic]"; llkPanicKernel(dump, tid, (state == 'Z') ? "zombie" : (state == 'D') ? "driver" : "sleeping", message); @@ -1274,6 +1314,26 @@ bool llkInit(const char* threadname) { llkBlacklistParent = llkSplit(LLK_BLACKLIST_PARENT_PROPERTY, std::to_string(kernelPid) + "," + std::to_string(kthreaddPid) + "," LLK_BLACKLIST_PARENT_DEFAULT); + // derive llkBlacklistParentAndChild by moving entries with '&' from above + for (auto it = llkBlacklistParent.begin(); it != llkBlacklistParent.end();) { + auto pos = it->find('&'); + if (pos == std::string::npos) { + ++it; + continue; + } + auto parent = it->substr(0, pos); + auto child = it->substr(pos + 1); + it = llkBlacklistParent.erase(it); + + auto found = llkBlacklistParentAndChild.find(parent); + if (found == llkBlacklistParentAndChild.end()) { + llkBlacklistParentAndChild.emplace(std::make_pair( + std::move(parent), std::unordered_set({std::move(child)}))); + } else { + found->second.emplace(std::move(child)); + } + } + llkBlacklistUid = llkSplit(LLK_BLACKLIST_UID_PROPERTY, LLK_BLACKLIST_UID_DEFAULT); // internal watchdog diff --git a/llkd/tests/llkd_test.cpp b/llkd/tests/llkd_test.cpp index d73893583..96079cc69 100644 --- a/llkd/tests/llkd_test.cpp +++ b/llkd/tests/llkd_test.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -333,3 +334,37 @@ TEST(llkd, sleep) { unlink(stack_pipe_file); } + +// b/120983740 +TEST(llkd, adbd_and_setsid) { + if (checkKill("kernel_panic,sysrq,livelock,zombie")) { + return; + } + const auto period = llkdSleepPeriod('S'); + + // expect llkd.zombie to trigger, but not for adbd&[setsid] + // Create a Persistent Zombie setsid Process + pid_t child_pid = fork(); + ASSERT_LE(0, child_pid); + if (!child_pid) { + prctl(PR_SET_NAME, "adbd"); + auto zombie_pid = fork(); + ASSERT_LE(0, zombie_pid); + if (!zombie_pid) { + prctl(PR_SET_NAME, "setsid"); + sleep(1); + exit(0); + } + sleep(period.count()); + exit(42); + } + + // Reverse of waitForPid, do _not_ expect kill + int wstatus; + ASSERT_LE(0, waitpid(child_pid, &wstatus, 0)); + EXPECT_TRUE(WIFEXITED(wstatus)); + if (WIFEXITED(wstatus)) { + EXPECT_EQ(42, WEXITSTATUS(wstatus)); + } + ASSERT_FALSE(WIFSIGNALED(wstatus)) << "[ INFO ] signo=" << WTERMSIG(wstatus); +}