From b6699b41e2c4873deac532e925561686bdb827d5 Mon Sep 17 00:00:00 2001 From: Giampaolo Rodola Date: Mon, 14 Dec 2020 20:27:47 +0100 Subject: [Windows] #1877: turn OpenProcess -> ERROR_SUCCESS into AD or NSP (#1887) Signed-off-by: Giampaolo Rodola --- HISTORY.rst | 2 ++ psutil/_psutil_bsd.c | 6 +++--- psutil/_psutil_common.c | 4 ++-- psutil/_psutil_osx.c | 3 ++- psutil/_psutil_windows.c | 31 ++++++++++++------------------- psutil/arch/freebsd/specific.c | 2 +- psutil/arch/netbsd/specific.c | 6 +++--- psutil/arch/osx/process_info.c | 2 +- psutil/arch/windows/process_info.c | 7 ++----- psutil/arch/windows/process_utils.c | 28 +++++++++++++++++++++++----- psutil/arch/windows/process_utils.h | 1 + psutil/tests/test_contracts.py | 8 ++++++-- scripts/procsmem.py | 7 ++++--- 13 files changed, 62 insertions(+), 45 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index ea283782..d55a4ee3 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -20,6 +20,8 @@ XXXX-XX-XX - 1866_: [Windows] process exe(), cmdline(), environ() may raise "invalid access to memory location" on Python 3.9. - 1874_: [Solaris] wrong swap output given when encrypted column is present +- 1877_: [Windows] OpenProcess may fail with ERROR_SUCCESS. Turn it into + AccessDenied or NoSuchProcess depending on whether the PID is alive. - 1886_: [macOS] EIO error may be raised on cmdline() and environment(). Now it gets translated into AccessDenied. diff --git a/psutil/_psutil_bsd.c b/psutil/_psutil_bsd.c index c1b811c6..1956ff27 100644 --- a/psutil/_psutil_bsd.c +++ b/psutil/_psutil_bsd.c @@ -477,10 +477,10 @@ psutil_proc_environ(PyObject *self, PyObject *args) { kvm_close(kd); return py_retdict; case EPERM: - AccessDenied("kvm_getenvv"); + AccessDenied("kvm_getenvv -> EPERM"); break; case ESRCH: - NoSuchProcess("kvm_getenvv"); + NoSuchProcess("kvm_getenvv -> ESRCH"); break; #if defined(PSUTIL_FREEBSD) case ENOMEM: @@ -489,7 +489,7 @@ psutil_proc_environ(PyObject *self, PyObject *args) { // "sudo procstat -e ".) // Map the error condition to 'AccessDenied'. sprintf(errbuf, - "kvm_getenvv(pid=%ld, ki_uid=%d): errno=ENOMEM", + "kvm_getenvv(pid=%ld, ki_uid=%d) -> ENOMEM", pid, p->ki_uid); AccessDenied(errbuf); break; diff --git a/psutil/_psutil_common.c b/psutil/_psutil_common.c index e72904de..fae8d970 100644 --- a/psutil/_psutil_common.c +++ b/psutil/_psutil_common.c @@ -105,7 +105,7 @@ NoSuchProcess(const char *syscall) { PyObject *exc; char msg[1024]; - sprintf(msg, "No such process (originated from %s)", syscall); + sprintf(msg, "assume no such process (originated from %s)", syscall); exc = PyObject_CallFunction(PyExc_OSError, "(is)", ESRCH, msg); PyErr_SetObject(PyExc_OSError, exc); Py_XDECREF(exc); @@ -122,7 +122,7 @@ AccessDenied(const char *syscall) { PyObject *exc; char msg[1024]; - sprintf(msg, "Access denied (originated from %s)", syscall); + sprintf(msg, "assume access denied (originated from %s)", syscall); exc = PyObject_CallFunction(PyExc_OSError, "(is)", EACCES, msg); PyErr_SetObject(PyExc_OSError, exc); Py_XDECREF(exc); diff --git a/psutil/_psutil_osx.c b/psutil/_psutil_osx.c index ee8d1c83..33d46635 100644 --- a/psutil/_psutil_osx.c +++ b/psutil/_psutil_osx.c @@ -99,7 +99,8 @@ psutil_task_for_pid(pid_t pid, mach_port_t *task) if (psutil_pid_exists(pid) == 0) NoSuchProcess("task_for_pid"); else if (psutil_is_zombie(pid) == 1) - PyErr_SetString(ZombieProcessError, "task_for_pid() failed"); + PyErr_SetString(ZombieProcessError, + "task_for_pid -> psutil_is_zombie -> 1"); else { psutil_debug( "task_for_pid() failed (pid=%ld, err=%i, errno=%i, msg='%s'); " diff --git a/psutil/_psutil_windows.c b/psutil/_psutil_windows.c index b836d708..711c1a79 100644 --- a/psutil/_psutil_windows.c +++ b/psutil/_psutil_windows.c @@ -159,22 +159,15 @@ psutil_proc_kill(PyObject *self, PyObject *args) { return AccessDenied("automatically set for PID 0"); hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, pid); + hProcess = psutil_check_phandle(hProcess, pid, 0); if (hProcess == NULL) { - if (GetLastError() == ERROR_INVALID_PARAMETER) { - // see https://github.com/giampaolo/psutil/issues/24 - psutil_debug("OpenProcess -> ERROR_INVALID_PARAMETER turned " - "into NoSuchProcess"); - NoSuchProcess("OpenProcess"); - } - else { - PyErr_SetFromWindowsErr(0); - } return NULL; } if (! TerminateProcess(hProcess, SIGTERM)) { // ERROR_ACCESS_DENIED may happen if the process already died. See: // https://github.com/giampaolo/psutil/issues/1099 + // http://bugs.python.org/issue14252 if (GetLastError() != ERROR_ACCESS_DENIED) { PyErr_SetFromOSErrnoWithSyscall("TerminateProcess"); return NULL; @@ -280,7 +273,7 @@ psutil_proc_times(PyObject *self, PyObject *args) { if (GetLastError() == ERROR_ACCESS_DENIED) { // usually means the process has died so we throw a NoSuchProcess // here - NoSuchProcess("GetProcessTimes"); + NoSuchProcess("GetProcessTimes -> ERROR_ACCESS_DENIED"); } else { PyErr_SetFromWindowsErr(0); @@ -332,7 +325,7 @@ psutil_proc_cmdline(PyObject *self, PyObject *args, PyObject *kwdict) { pid_return = psutil_pid_is_running(pid); if (pid_return == 0) - return NoSuchProcess("psutil_pid_is_running"); + return NoSuchProcess("psutil_pid_is_running -> 0"); if (pid_return == -1) return NULL; @@ -356,7 +349,7 @@ psutil_proc_environ(PyObject *self, PyObject *args) { pid_return = psutil_pid_is_running(pid); if (pid_return == 0) - return NoSuchProcess("psutil_pid_is_running"); + return NoSuchProcess("psutil_pid_is_running -> 0"); if (pid_return == -1) return NULL; @@ -383,7 +376,7 @@ psutil_proc_exe(PyObject *self, PyObject *args) { return NULL; if (pid == 0) - return AccessDenied("forced for PID 0"); + return AccessDenied("automatically set for PID 0"); buffer = MALLOC_ZERO(bufferSize); if (! buffer) @@ -417,7 +410,7 @@ psutil_proc_exe(PyObject *self, PyObject *args) { if (! NT_SUCCESS(status)) { FREE(buffer); if (psutil_pid_is_running(pid) == 0) - NoSuchProcess("NtQuerySystemInformation"); + NoSuchProcess("psutil_pid_is_running -> 0"); else psutil_SetFromNTStatusErr(status, "NtQuerySystemInformation"); return NULL; @@ -536,10 +529,10 @@ psutil_GetProcWsetInformation( if (!NT_SUCCESS(status)) { if (status == STATUS_ACCESS_DENIED) { - AccessDenied("NtQueryVirtualMemory"); + AccessDenied("NtQueryVirtualMemory -> STATUS_ACCESS_DENIED"); } else if (psutil_pid_is_running(pid) == 0) { - NoSuchProcess("psutil_pid_is_running"); + NoSuchProcess("psutil_pid_is_running -> 0"); } else { PyErr_Clear(); @@ -644,7 +637,7 @@ psutil_proc_cwd(PyObject *self, PyObject *args) { pid_return = psutil_pid_is_running(pid); if (pid_return == 0) - return NoSuchProcess("psutil_pid_is_running"); + return NoSuchProcess("psutil_pid_is_running -> 0"); if (pid_return == -1) return NULL; @@ -703,13 +696,13 @@ psutil_proc_threads(PyObject *self, PyObject *args) { if (pid == 0) { // raise AD instead of returning 0 as procexp is able to // retrieve useful information somehow - AccessDenied("automatically set for PID 0"); + AccessDenied("forced for PID 0"); goto error; } pid_return = psutil_pid_is_running(pid); if (pid_return == 0) { - NoSuchProcess("psutil_pid_is_running"); + NoSuchProcess("psutil_pid_is_running -> 0"); goto error; } if (pid_return == -1) diff --git a/psutil/arch/freebsd/specific.c b/psutil/arch/freebsd/specific.c index e7517a40..e498a7b3 100644 --- a/psutil/arch/freebsd/specific.c +++ b/psutil/arch/freebsd/specific.c @@ -262,7 +262,7 @@ psutil_proc_exe(PyObject *self, PyObject *args) { if (ret == -1) return NULL; else if (ret == 0) - return NoSuchProcess("psutil_pid_exists"); + return NoSuchProcess("psutil_pid_exists -> 0"); else strcpy(pathname, ""); } diff --git a/psutil/arch/netbsd/specific.c b/psutil/arch/netbsd/specific.c index 9dab3618..2fbc2418 100644 --- a/psutil/arch/netbsd/specific.c +++ b/psutil/arch/netbsd/specific.c @@ -126,7 +126,7 @@ psutil_proc_cwd(PyObject *self, PyObject *args) { int name[] = { CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_CWD}; if (sysctl(name, 4, path, &pathlen, NULL, 0) != 0) { if (errno == ENOENT) - NoSuchProcess(""); + NoSuchProcess("sysctl -> ENOENT"); else PyErr_SetFromErrno(PyExc_OSError); return NULL; @@ -142,7 +142,7 @@ psutil_proc_cwd(PyObject *self, PyObject *args) { free(buf); if (len == -1) { if (errno == ENOENT) - NoSuchProcess("readlink (ENOENT)"); + NoSuchProcess("readlink -> ENOENT"); else PyErr_SetFromErrno(PyExc_OSError); return NULL; @@ -198,7 +198,7 @@ psutil_proc_exe(PyObject *self, PyObject *args) { if (ret == -1) return NULL; else if (ret == 0) - return NoSuchProcess("psutil_pid_exists"); + return NoSuchProcess("psutil_pid_exists -> 0"); else strcpy(pathname, ""); } diff --git a/psutil/arch/osx/process_info.c b/psutil/arch/osx/process_info.c index 3732f91d..fb9f24ff 100644 --- a/psutil/arch/osx/process_info.c +++ b/psutil/arch/osx/process_info.c @@ -125,7 +125,7 @@ psutil_sysctl_procargs(pid_t pid, char *procargs, size_t argmax) { if (sysctl(mib, 3, procargs, &argmax, NULL, 0) < 0) { if (psutil_pid_exists(pid) == 0) { - NoSuchProcess(""); + NoSuchProcess("psutil_pid_exists -> 0"); return 1; } // In case of zombie process we'll get EINVAL. We translate it diff --git a/psutil/arch/windows/process_info.c b/psutil/arch/windows/process_info.c index 36283add..0ce6297a 100644 --- a/psutil/arch/windows/process_info.c +++ b/psutil/arch/windows/process_info.c @@ -55,10 +55,7 @@ psutil_convert_winerr(ULONG err, char* syscall) { char fullmsg[8192]; if (err == ERROR_NOACCESS) { - sprintf( - fullmsg, - "(originated from %s -> ERROR_NOACCESS; converted to AccessDenied)", - syscall); + sprintf(fullmsg, "%s -> ERROR_NOACCESS", syscall); psutil_debug(fullmsg); AccessDenied(fullmsg); } @@ -434,7 +431,7 @@ psutil_cmdline_query_proc(DWORD pid, WCHAR **pdata, SIZE_T *psize) { // https://github.com/giampaolo/psutil/issues/1501 if (status == STATUS_NOT_FOUND) { AccessDenied("NtQueryInformationProcess(ProcessBasicInformation) -> " - "STATUS_NOT_FOUND translated into PermissionError"); + "STATUS_NOT_FOUND"); goto error; } diff --git a/psutil/arch/windows/process_utils.c b/psutil/arch/windows/process_utils.c index f9d2f2f9..acbda301 100644 --- a/psutil/arch/windows/process_utils.c +++ b/psutil/arch/windows/process_utils.c @@ -61,8 +61,10 @@ psutil_pid_in_pids(DWORD pid) { DWORD i; proclist = psutil_get_pids(&numberOfReturnedPIDs); - if (proclist == NULL) + if (proclist == NULL) { + psutil_debug("psutil_get_pids() failed"); return -1; + } for (i = 0; i < numberOfReturnedPIDs; i++) { if (proclist[i] == pid) { free(proclist); @@ -78,20 +80,36 @@ psutil_pid_in_pids(DWORD pid) { // does return the handle, else return NULL with Python exception set. // This is needed because OpenProcess API sucks. HANDLE -psutil_check_phandle(HANDLE hProcess, DWORD pid) { +psutil_check_phandle(HANDLE hProcess, DWORD pid, int check_exit_code) { DWORD exitCode; if (hProcess == NULL) { if (GetLastError() == ERROR_INVALID_PARAMETER) { // Yeah, this is the actual error code in case of // "no such process". - NoSuchProcess("OpenProcess"); + NoSuchProcess("OpenProcess -> ERROR_INVALID_PARAMETER"); + return NULL; + } + if (GetLastError() == ERROR_SUCCESS) { + // Yeah, it's this bad. + // https://github.com/giampaolo/psutil/issues/1877 + if (psutil_pid_in_pids(pid) == 1) { + psutil_debug("OpenProcess -> ERROR_SUCCESS turned into AD"); + AccessDenied("OpenProcess -> ERROR_SUCCESS"); + } + else { + psutil_debug("OpenProcess -> ERROR_SUCCESS turned into NSP"); + NoSuchProcess("OpenProcess -> ERROR_SUCCESS"); + } return NULL; } PyErr_SetFromOSErrnoWithSyscall("OpenProcess"); return NULL; } + if (check_exit_code == 0) + return hProcess; + if (GetExitCodeProcess(hProcess, &exitCode)) { // XXX - maybe STILL_ACTIVE is not fully reliable as per: // http://stackoverflow.com/questions/1591342/#comment47830782_1591379 @@ -137,7 +155,7 @@ psutil_handle_from_pid(DWORD pid, DWORD access) { return NULL; } - hProcess = psutil_check_phandle(hProcess, pid); + hProcess = psutil_check_phandle(hProcess, pid, 1); return hProcess; } @@ -159,7 +177,7 @@ psutil_pid_is_running(DWORD pid) { if ((hProcess == NULL) && (GetLastError() == ERROR_ACCESS_DENIED)) return 1; - hProcess = psutil_check_phandle(hProcess, pid); + hProcess = psutil_check_phandle(hProcess, pid, 1); if (hProcess != NULL) { CloseHandle(hProcess); return 1; diff --git a/psutil/arch/windows/process_utils.h b/psutil/arch/windows/process_utils.h index a7171c5c..dca7c991 100644 --- a/psutil/arch/windows/process_utils.h +++ b/psutil/arch/windows/process_utils.h @@ -6,6 +6,7 @@ DWORD* psutil_get_pids(DWORD *numberOfReturnedPIDs); HANDLE psutil_handle_from_pid(DWORD pid, DWORD dwDesiredAccess); +HANDLE psutil_check_phandle(HANDLE hProcess, DWORD pid, int check_exit_code); int psutil_pid_is_running(DWORD pid); int psutil_assert_pid_exists(DWORD pid, char *err); int psutil_assert_pid_not_exists(DWORD pid, char *err); diff --git a/psutil/tests/test_contracts.py b/psutil/tests/test_contracts.py index 43eb9cbc..dd6002e6 100755 --- a/psutil/tests/test_contracts.py +++ b/psutil/tests/test_contracts.py @@ -449,8 +449,12 @@ class TestFetchAllProcesses(PsutilTestCase): # http://stackoverflow.com/questions/3112546/os-path-exists-lies if POSIX and os.path.isfile(ret): if hasattr(os, 'access') and hasattr(os, "X_OK"): - # XXX may fail on MACOS - assert os.access(ret, os.X_OK) + # XXX: may fail on MACOS + try: + assert os.access(ret, os.X_OK) + except AssertionError: + if os.path.exists(ret): + raise def pid(self, ret, info): self.assertIsInstance(ret, int) diff --git a/scripts/procsmem.py b/scripts/procsmem.py index 259d79d4..1074c4c2 100755 --- a/scripts/procsmem.py +++ b/scripts/procsmem.py @@ -80,18 +80,19 @@ def main(): procs.append(p) procs.sort(key=lambda p: p._uss) - templ = "%-7s %-7s %-30s %7s %7s %7s %7s" - print(templ % ("PID", "User", "Cmdline", "USS", "PSS", "Swap", "RSS")) + templ = "%-7s %-7s %7s %7s %7s %7s %7s" + print(templ % ("PID", "User", "USS", "PSS", "Swap", "RSS", "Cmdline")) print("=" * 78) for p in procs[:86]: + cmd = " ".join(p._info["cmdline"])[:50] if p._info["cmdline"] else "" line = templ % ( p.pid, p._info["username"][:7] if p._info["username"] else "", - " ".join(p._info["cmdline"])[:30], convert_bytes(p._uss), convert_bytes(p._pss) if p._pss != "" else "", convert_bytes(p._swap) if p._swap != "" else "", convert_bytes(p._rss), + cmd, ) print(line) if ad_pids: -- cgit v1.2.1