[Lldb-commits] [lldb] 843a0f9 - Enhance debugserver's err reporting on attach fails
Jason Molenda via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 4 19:47:03 PST 2020
Author: Jason Molenda
Date: 2020-02-04T19:46:55-08:00
New Revision: 843a0f97717a006fd21cd89fd229b064506e5d05
URL: https://github.com/llvm/llvm-project/commit/843a0f97717a006fd21cd89fd229b064506e5d05
DIFF: https://github.com/llvm/llvm-project/commit/843a0f97717a006fd21cd89fd229b064506e5d05.diff
LOG: Enhance debugserver's err reporting on attach fails
Explicitly check for a request to attach to a pid that doesn't
exist, to attach to a pid that is already being debugged, unify the
SIP process check, and an attempt at checking if developer mode is
enabled on the system (which isn't working in debugserver, for some
reason; I can't get the authorization record which should be an
unprivileged operation and works in a standalone program I wrote).
I'll debug the developer mode check later, but I wanted to land it
along with everything else; right now it will claim that developer
mode is always enabled so it's harmless to include as-is.
Added:
Modified:
lldb/tools/debugserver/source/DNB.cpp
lldb/tools/debugserver/source/DNB.h
lldb/tools/debugserver/source/RNBRemote.cpp
lldb/tools/debugserver/source/RNBServices.cpp
Removed:
################################################################################
diff --git a/lldb/tools/debugserver/source/DNB.cpp b/lldb/tools/debugserver/source/DNB.cpp
index 8d9c691f9d33..d0dd8fd31841 100644
--- a/lldb/tools/debugserver/source/DNB.cpp
+++ b/lldb/tools/debugserver/source/DNB.cpp
@@ -57,7 +57,6 @@ typedef std::map<nub_process_t, MachProcessSP> ProcessMap;
typedef ProcessMap::iterator ProcessMapIter;
typedef ProcessMap::const_iterator ProcessMapConstIter;
-size_t GetAllInfos(std::vector<struct kinfo_proc> &proc_infos);
static size_t
GetAllInfosMatchingName(const char *process_name,
std::vector<struct kinfo_proc> &matching_proc_infos);
@@ -520,7 +519,7 @@ nub_process_t DNBProcessAttach(nub_process_t attach_pid,
return INVALID_NUB_PROCESS;
}
-size_t GetAllInfos(std::vector<struct kinfo_proc> &proc_infos) {
+size_t DNBGetAllInfos(std::vector<struct kinfo_proc> &proc_infos) {
size_t size = 0;
int name[] = {CTL_KERN, KERN_PROC, KERN_PROC_ALL};
u_int namelen = sizeof(name) / sizeof(int);
@@ -573,7 +572,7 @@ GetAllInfosMatchingName(const char *full_process_name,
const size_t process_name_len = strlen(process_name);
std::vector<struct kinfo_proc> proc_infos;
- const size_t num_proc_infos = GetAllInfos(proc_infos);
+ const size_t num_proc_infos = DNBGetAllInfos(proc_infos);
if (num_proc_infos > 0) {
uint32_t i;
for (i = 0; i < num_proc_infos; i++) {
diff --git a/lldb/tools/debugserver/source/DNB.h b/lldb/tools/debugserver/source/DNB.h
index e29fa0fa6365..ec04a1a11d76 100644
--- a/lldb/tools/debugserver/source/DNB.h
+++ b/lldb/tools/debugserver/source/DNB.h
@@ -150,6 +150,7 @@ nub_size_t DNBProcessGetAvailableProfileData(nub_process_t pid, char *buf,
nub_size_t buf_size) DNB_EXPORT;
nub_size_t DNBProcessGetStopCount(nub_process_t pid) DNB_EXPORT;
uint32_t DNBProcessGetCPUType(nub_process_t pid) DNB_EXPORT;
+size_t DNBGetAllInfos(std::vector<struct kinfo_proc> &proc_infos);
// Process executable and arguments
const char *DNBProcessGetExecutablePath(nub_process_t pid);
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index 6f2c7353b723..9d6ca995905c 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -13,8 +13,10 @@
#include "RNBRemote.h"
#include <errno.h>
+#include <libproc.h>
#include <mach-o/loader.h>
#include <mach/exception_types.h>
+#include <mach/task_info.h>
#include <signal.h>
#include <sys/stat.h>
#include <sys/sysctl.h>
@@ -49,6 +51,9 @@
#include <sstream>
#include <unordered_set>
+#include <CoreFoundation/CoreFoundation.h>
+#include <Security/Security.h>
+
// constants
static const std::string OS_LOG_EVENTS_KEY_NAME("events");
@@ -3644,6 +3649,132 @@ rnb_err_t RNBRemote::HandlePacket_qSupported(const char *p) {
return SendPacket(buf);
}
+static bool process_does_not_exist (nub_process_t pid) {
+ std::vector<struct kinfo_proc> proc_infos;
+ DNBGetAllInfos (proc_infos);
+ const size_t infos_size = proc_infos.size();
+ for (size_t i = 0; i < infos_size; i++)
+ if (proc_infos[i].kp_proc.p_pid == pid)
+ return false;
+
+ return true; // process does not exist
+}
+
+static bool attach_failed_due_to_sip (nub_process_t pid) {
+ bool retval = false;
+#if defined(__APPLE__) && \
+ (__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 101000)
+
+ // csr_check(CSR_ALLOW_TASK_FOR_PID) will be nonzero if System Integrity
+ // Protection is in effect.
+ if (csr_check(CSR_ALLOW_TASK_FOR_PID) == 0)
+ return false;
+
+ if (rootless_allows_task_for_pid(pid) == 0)
+ retval = true;
+
+ int csops_flags = 0;
+ int csops_ret = ::csops(pid, CS_OPS_STATUS, &csops_flags,
+ sizeof(csops_flags));
+ if (csops_ret != -1 && (csops_flags & CS_RESTRICT)) {
+ retval = true;
+ }
+#endif
+
+ return retval;
+}
+
+static bool process_is_already_being_debugged (nub_process_t pid) {
+ struct kinfo_proc kinfo;
+ int mib[] = {CTL_KERN, KERN_PROC, KERN_PROC_PID, pid};
+ size_t len = sizeof(struct kinfo_proc);
+ if (sysctl(mib, sizeof(mib) / sizeof(mib[0]), &kinfo, &len, NULL, 0) != 0) {
+ return false; // pid doesn't exist? well, it's not being debugged...
+ }
+ if (kinfo.kp_proc.p_flag & P_TRACED)
+ return true; // is being debugged already
+ else
+ return false;
+}
+
+// Checking for
+//
+// {
+// 'class' : 'rule',
+// 'comment' : 'For use by Apple. WARNING: administrators are advised
+// not to modify this right.',
+// 'k-of-n' : '1',
+// 'rule' : [
+// 'is-admin',
+// 'is-developer',
+// 'authenticate-developer'
+// ]
+// }
+//
+// $ security authorizationdb read system.privilege.taskport.debug
+
+static bool developer_mode_enabled () {
+ CFDictionaryRef currentRightDict = NULL;
+ const char *debug_right = "system.privilege.taskport.debug";
+ // caller must free dictionary initialized by the following
+ OSStatus status = AuthorizationRightGet(debug_right, ¤tRightDict);
+ if (status != errAuthorizationSuccess) {
+ // could not check authorization
+ return true;
+ }
+
+ bool devmode_enabled = true;
+
+ if (!CFDictionaryContainsKey(currentRightDict, CFSTR("k-of-n"))) {
+ devmode_enabled = false;
+ } else {
+ CFNumberRef item = (CFNumberRef) CFDictionaryGetValue(currentRightDict, CFSTR("k-of-n"));
+ if (item && CFGetTypeID(item) == CFNumberGetTypeID()) {
+ int64_t num = 0;
+ ::CFNumberGetValue(item, kCFNumberSInt64Type, &num);
+ if (num != 1) {
+ devmode_enabled = false;
+ }
+ } else {
+ devmode_enabled = false;
+ }
+ }
+
+ if (!CFDictionaryContainsKey(currentRightDict, CFSTR("class"))) {
+ devmode_enabled = false;
+ } else {
+ CFStringRef item = (CFStringRef) CFDictionaryGetValue(currentRightDict, CFSTR("class"));
+ if (item && CFGetTypeID(item) == CFStringGetTypeID()) {
+ if (strcmp (CFStringGetCStringPtr (item, ::CFStringGetSystemEncoding()), "rule") != 0) {
+ devmode_enabled = false;
+ }
+ } else {
+ devmode_enabled = false;
+ }
+ }
+
+ if (!CFDictionaryContainsKey(currentRightDict, CFSTR("rule"))) {
+ devmode_enabled = false;
+ } else {
+ CFArrayRef item = (CFArrayRef) CFDictionaryGetValue(currentRightDict, CFSTR("rule"));
+ if (item && CFGetTypeID(item) == CFArrayGetTypeID()) {
+ int count = ::CFArrayGetCount(item);
+ CFRange range = CFRangeMake (0, count);
+ if (!::CFArrayContainsValue (item, range, CFSTR("is-admin")))
+ devmode_enabled = false;
+ if (!::CFArrayContainsValue (item, range, CFSTR("is-developer")))
+ devmode_enabled = false;
+ if (!::CFArrayContainsValue (item, range, CFSTR("authenticate-developer")))
+ devmode_enabled = false;
+ } else {
+ devmode_enabled = false;
+ }
+ }
+ ::CFRelease(currentRightDict);
+
+ return devmode_enabled;
+}
+
/*
vAttach;pid
@@ -3802,48 +3933,46 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
else
m_ctx.LaunchStatus().SetErrorString("attach failed");
-#if defined(__APPLE__) && \
- (__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 101000)
if (pid_attaching_to == INVALID_NUB_PROCESS && !attach_name.empty()) {
pid_attaching_to = DNBProcessGetPIDByName(attach_name.c_str());
}
- if (pid_attaching_to != INVALID_NUB_PROCESS &&
- strcmp(err_str, "No such process") != 0) {
- // csr_check(CSR_ALLOW_TASK_FOR_PID) will be nonzero if System Integrity
- // Protection is in effect.
- if (csr_check(CSR_ALLOW_TASK_FOR_PID) != 0) {
- bool attach_failed_due_to_sip = false;
-
- if (rootless_allows_task_for_pid(pid_attaching_to) == 0) {
- attach_failed_due_to_sip = true;
- }
- if (!attach_failed_due_to_sip) {
- int csops_flags = 0;
- int retval = ::csops(pid_attaching_to, CS_OPS_STATUS, &csops_flags,
- sizeof(csops_flags));
- if (retval != -1 && (csops_flags & CS_RESTRICT)) {
- attach_failed_due_to_sip = true;
- }
- }
- if (attach_failed_due_to_sip) {
- std::string return_message = "E96;";
- return_message += cstring_to_asciihex_string(
- "Process attach denied, possibly because "
- "System Integrity Protection is enabled and "
- "process does not allow attaching.");
-
- SendPacket(return_message.c_str());
- DNBLogError("Attach failed because process does not allow "
- "attaching: \"%s\".",
- err_str);
- return rnb_err;
- }
+ // attach_pid is INVALID_NUB_PROCESS - we did not succeed in attaching
+ // if the original request, pid_attaching_to, is available, see if we
+ // can figure out why we couldn't attach. Return an informative error
+ // string to lldb.
+
+ if (pid_attaching_to != INVALID_NUB_PROCESS) {
+ if (process_does_not_exist (pid_attaching_to)) {
+ DNBLogError("Tried to attach to pid that doesn't exist");
+ std::string return_message = "E96;";
+ return_message += cstring_to_asciihex_string("no such process.");
+ return SendPacket(return_message.c_str());
+ }
+ if (process_is_already_being_debugged (pid_attaching_to)) {
+ DNBLogError("Tried to attach to process already being debugged");
+ std::string return_message = "E96;";
+ return_message += cstring_to_asciihex_string("tried to attach to "
+ "process already being debugged");
+ return SendPacket(return_message.c_str());
+ }
+ if (!developer_mode_enabled()) {
+ DNBLogError("Developer mode is not enabled");
+ std::string return_message = "E96;";
+ return_message += cstring_to_asciihex_string("developer mode is not "
+ "enabled on this machine. "
+ "sudo DevToolsSecurity --enable");
+ return SendPacket(return_message.c_str());
+ }
+ if (attach_failed_due_to_sip (pid_attaching_to)) {
+ DNBLogError("Attach failed because of SIP protection.");
+ std::string return_message = "E96;";
+ return_message += cstring_to_asciihex_string("cannot attach "
+ "to process due to System Integrity Protection");
+ return SendPacket(return_message.c_str());
}
}
-#endif
-
SendPacket("E01"); // E01 is our magic error value for attach failed.
DNBLogError("Attach failed: \"%s\".", err_str);
return rnb_err;
diff --git a/lldb/tools/debugserver/source/RNBServices.cpp b/lldb/tools/debugserver/source/RNBServices.cpp
index 7b2ab7c9b737..6e4b55e3e646 100644
--- a/lldb/tools/debugserver/source/RNBServices.cpp
+++ b/lldb/tools/debugserver/source/RNBServices.cpp
@@ -12,6 +12,7 @@
#include "RNBServices.h"
+#include "DNB.h"
#include "CFString.h"
#include "DNBLog.h"
#include "MacOSX/CFUtils.h"
@@ -28,16 +29,13 @@
#include <SpringBoardServices/SpringBoardServices.h>
#endif
-// From DNB.cpp
-size_t GetAllInfos(std::vector<struct kinfo_proc> &proc_infos);
-
int GetProcesses(CFMutableArrayRef plistMutableArray, bool all_users) {
if (plistMutableArray == NULL)
return -1;
// Running as root, get all processes
std::vector<struct kinfo_proc> proc_infos;
- const size_t num_proc_infos = GetAllInfos(proc_infos);
+ const size_t num_proc_infos = DNBGetAllInfos(proc_infos);
if (num_proc_infos > 0) {
const pid_t our_pid = getpid();
const uid_t our_uid = getuid();
More information about the lldb-commits
mailing list