[Win] return value is not properly handled for undocumented NT* Windows APIs. (#1477)

9 files changed, 133 insertions, 53 deletions

diff --git a/HISTORY.rst b/HISTORY.rst
index 7b0d889c..b5e1e6a0 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -23,6 +23,8 @@
   by Daniel Beer)
 - 1475_: [Windows] OSError.winerror attribute wasn't properly checked resuling
   in WindowsError being raised instead of AccessDenied.
+- 1477_: [Windows] wrong or absent error handling for private NTSTATUS Windows
+  APIs. Different process methods were affected by this.
 
 5.6.1
 =====
diff --git a/psutil/_psutil_common.c b/psutil/_psutil_common.c
index 4b6ab399..c6e37bc2 100644
--- a/psutil/_psutil_common.c
+++ b/psutil/_psutil_common.c
@@ -62,7 +62,7 @@ PyErr_SetFromOSErrnoWithSyscall(const char *syscall) {
     char fullmsg[1024];
 
 #ifdef _WIN32
-    sprintf(fullmsg, "originated from %s", syscall);
+    sprintf(fullmsg, "(originated from %s)", syscall);
     PyErr_SetFromWindowsErrWithFilename(GetLastError(), fullmsg);
 #else
     PyObject *exc;
diff --git a/psutil/_psutil_windows.c b/psutil/_psutil_windows.c
index 4dfae2d5..b1f8d650 100644
--- a/psutil/_psutil_windows.c
+++ b/psutil/_psutil_windows.c
@@ -3,7 +3,15 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  *
- * Windows platform-specific module methods for _psutil_windows
+ * Windows platform-specific module methods for _psutil_windows.
+ *
+ * List of undocumented Windows NT APIs which are used in here and in
+ * other modules:
+ * - NtQuerySystemInformation
+ * - NtQueryInformationProcess
+ * - NtQueryObject
+ * - NtSuspendProcess
+ * - NtResumeProcess
  */
 
 // Fixes clash between winsock2.h and windows.h
@@ -797,8 +805,8 @@ psutil_GetProcWsetInformation(
         }
         else {
             PyErr_Clear();
-            psutil_debug("NtQueryVirtualMemory failed with %i", status);
-            PyErr_SetString(PyExc_RuntimeError, "NtQueryVirtualMemory failed");
+            psutil_SetFromNTStatusErr(
+                status, "NtQueryVirtualMemory(MemoryWorkingSetInformation)");
         }
         HeapFree(GetProcessHeap(), 0, buffer);
         return 1;
@@ -946,8 +954,11 @@ psutil_per_cpu_times(PyObject *self, PyObject *args) {
         sppi,
         ncpus * sizeof(_SYSTEM_PROCESSOR_PERFORMANCE_INFORMATION),
         NULL);
-    if (status != 0) {
-        PyErr_SetFromWindowsErr(0);
+    if (! NT_SUCCESS(status)) {
+        psutil_SetFromNTStatusErr(
+            status,
+            "NtQuerySystemInformation(SystemProcessorPerformanceInformation)"
+        );
         goto error;
     }
 
@@ -1025,7 +1036,7 @@ psutil_proc_cwd(PyObject *self, PyObject *args) {
 static PyObject *
 psutil_proc_suspend_or_resume(PyObject *self, PyObject *args) {
     long pid;
-    int ret;
+    NTSTATUS status;
     HANDLE hProcess;
     PyObject* suspend;
 
@@ -1037,15 +1048,15 @@ psutil_proc_suspend_or_resume(PyObject *self, PyObject *args) {
         return NULL;
 
     if (PyObject_IsTrue(suspend))
-        ret = psutil_NtSuspendProcess(hProcess);
+        status = psutil_NtSuspendProcess(hProcess);
     else
-        ret = psutil_NtResumeProcess(hProcess);
+        status = psutil_NtResumeProcess(hProcess);
 
-    if (ret != 0) {
-        PyErr_SetFromWindowsErr(0);
+    if (! NT_SUCCESS(status)) {
         CloseHandle(hProcess);
-        return NULL;
+        return psutil_SetFromNTStatusErr(status, "NtSuspend|ResumeProcess");
     }
+
     CloseHandle(hProcess);
     Py_RETURN_NONE;
 }
@@ -1339,6 +1350,7 @@ error:
 
 
 // https://msdn.microsoft.com/library/aa365928.aspx
+// TODO properly handle return code
 static DWORD __GetExtendedTcpTable(_GetExtendedTcpTable call,
                                    ULONG address_family,
                                    PVOID * data, DWORD * size)
@@ -1373,6 +1385,7 @@ static DWORD __GetExtendedTcpTable(_GetExtendedTcpTable call,
 
 
 // https://msdn.microsoft.com/library/aa365930.aspx
+// TODO properly check return value
 static DWORD __GetExtendedUdpTable(_GetExtendedUdpTable call,
                                    ULONG address_family,
                                    PVOID * data, DWORD * size)
@@ -1859,20 +1872,26 @@ psutil_proc_io_priority_get(PyObject *self, PyObject *args) {
     long pid;
     HANDLE hProcess;
     DWORD IoPriority;
+    NTSTATUS status;
 
     if (! PyArg_ParseTuple(args, "l", &pid))
         return NULL;
+
     hProcess = psutil_handle_from_pid(pid, PROCESS_QUERY_LIMITED_INFORMATION);
     if (hProcess == NULL)
         return NULL;
-    psutil_NtQueryInformationProcess(
+
+    status = psutil_NtQueryInformationProcess(
         hProcess,
         ProcessIoPriority,
         &IoPriority,
         sizeof(DWORD),
         NULL
     );
+
     CloseHandle(hProcess);
+    if (! NT_SUCCESS(status))
+        return psutil_SetFromNTStatusErr(status, "NtQueryInformationProcess");
     return Py_BuildValue("i", IoPriority);
 }
 
@@ -1885,15 +1904,17 @@ psutil_proc_io_priority_set(PyObject *self, PyObject *args) {
     long pid;
     DWORD prio;
     HANDLE hProcess;
+    NTSTATUS status;
     DWORD access = PROCESS_QUERY_INFORMATION | PROCESS_SET_INFORMATION;
 
     if (! PyArg_ParseTuple(args, "li", &pid, &prio))
         return NULL;
+
     hProcess = psutil_handle_from_pid(pid, access);
     if (hProcess == NULL)
         return NULL;
 
-    psutil_NtSetInformationProcess(
+    status = psutil_NtSetInformationProcess(
         hProcess,
         ProcessIoPriority,
         (PVOID)&prio,
@@ -1901,6 +1922,8 @@ psutil_proc_io_priority_set(PyObject *self, PyObject *args) {
     );
 
     CloseHandle(hProcess);
+    if (! NT_SUCCESS(status))
+        return psutil_SetFromNTStatusErr(status, "NtSetInformationProcess");
     Py_RETURN_NONE;
 }
 #endif
@@ -3217,9 +3240,9 @@ psutil_cpu_stats(PyObject *self, PyObject *args) {
         spi,
         ncpus * sizeof(_SYSTEM_PERFORMANCE_INFORMATION),
         NULL);
-    if (status != 0) {
-        PyErr_SetFromOSErrnoWithSyscall(
-            "NtQuerySystemInformation(SYSTEM_PERFORMANCE_INFORMATION)");
+    if (! NT_SUCCESS(status)) {
+        psutil_SetFromNTStatusErr(
+            status, "NtQuerySystemInformation(SystemPerformanceInformation)");
         goto error;
     }
 
@@ -3236,9 +3259,9 @@ psutil_cpu_stats(PyObject *self, PyObject *args) {
         InterruptInformation,
         ncpus * sizeof(SYSTEM_INTERRUPT_INFORMATION),
         NULL);
-    if (status != 0) {
-        PyErr_SetFromOSErrnoWithSyscall(
-            "NtQuerySystemInformation(SYSTEM_INTERRUPT_INFORMATION)");
+    if (! NT_SUCCESS(status)) {
+        psutil_SetFromNTStatusErr(
+            status, "NtQuerySystemInformation(SystemInterruptInformation)");
         goto error;
     }
     for (i = 0; i < ncpus; i++) {
@@ -3258,9 +3281,10 @@ psutil_cpu_stats(PyObject *self, PyObject *args) {
         sppi,
         ncpus * sizeof(_SYSTEM_PROCESSOR_PERFORMANCE_INFORMATION),
         NULL);
-    if (status != 0) {
-        PyErr_SetFromOSErrnoWithSyscall(
-            "NtQuerySystemInformation(SYSTEM_PROCESSOR_PERFORMANCE_INFORMATION)");
+    if (! NT_SUCCESS(status)) {
+        psutil_SetFromNTStatusErr(
+            status,
+            "NtQuerySystemInformation(SystemProcessorPerformanceInformation)");
         goto error;
     }
 
diff --git a/psutil/arch/windows/global.c b/psutil/arch/windows/global.c
index 9ef92092..4d8526e3 100644
--- a/psutil/arch/windows/global.c
+++ b/psutil/arch/windows/global.c
@@ -18,6 +18,14 @@
 int PSUTIL_WINVER;
 SYSTEM_INFO PSUTIL_SYSTEM_INFO;
 
+#define NT_FACILITY_MASK 0xfff
+#define NT_FACILITY_SHIFT 16
+#define NT_FACILITY(Status) \
+    ((((ULONG)(Status)) >> NT_FACILITY_SHIFT) & NT_FACILITY_MASK)
+#define NT_NTWIN32(status) (NT_FACILITY(Status) == FACILITY_WIN32)
+#define WIN32_FROM_NTSTATUS(Status) (((ULONG)(Status)) & 0xffff)
+
+
 // A wrapper around GetModuleHandle and GetProcAddress.
 PVOID
 psutil_GetProcAddress(LPCSTR libname, LPCSTR procname) {
@@ -60,6 +68,26 @@ psutil_GetProcAddressFromLib(LPCSTR libname, LPCSTR procname) {
 }
 
 
+/*
+ * Convert a NTSTATUS value to a Win32 error code and set the proper
+ * Python exception.
+ */
+PVOID
+psutil_SetFromNTStatusErr(NTSTATUS Status, const char *syscall) {
+    ULONG err;
+    char fullmsg[1024];
+
+    if (NT_NTWIN32(Status))
+        err = WIN32_FROM_NTSTATUS(Status);
+    else
+        err = psutil_RtlNtStatusToDosErrorNoTeb(Status);
+    // if (GetLastError() != 0)
+    //     err = GetLastError();
+    sprintf(fullmsg, "(originated from %s)", syscall);
+    return PyErr_SetFromWindowsErrWithFilename(err, fullmsg);
+}
+
+
 static int
 psutil_loadlibs() {
     /*
@@ -127,6 +155,11 @@ psutil_loadlibs() {
     if (! psutil_NtQueryVirtualMemory)
         return 1;
 
+    psutil_RtlNtStatusToDosErrorNoTeb = psutil_GetProcAddressFromLib(
+        "ntdll", "RtlNtStatusToDosErrorNoTeb");
+    if (! psutil_RtlNtStatusToDosErrorNoTeb)
+        return 1;
+
     /*
      * Optional.
      */
diff --git a/psutil/arch/windows/global.h b/psutil/arch/windows/global.h
index fb24bac9..10ae6405 100644
--- a/psutil/arch/windows/global.h
+++ b/psutil/arch/windows/global.h
@@ -23,6 +23,7 @@ extern SYSTEM_INFO PSUTIL_SYSTEM_INFO;
 int psutil_load_globals();
 PVOID psutil_GetProcAddress(LPCSTR libname, LPCSTR procname);
 PVOID psutil_GetProcAddressFromLib(LPCSTR libname, LPCSTR procname);
+PVOID psutil_SetFromNTStatusErr(NTSTATUS Status, const char *syscall);
 
 _NtQuerySystemInformation \
     psutil_NtQuerySystemInformation;
@@ -71,3 +72,6 @@ _NtResumeProcess \
 
 _NtQueryVirtualMemory \
     psutil_NtQueryVirtualMemory;
+
+_RtlNtStatusToDosErrorNoTeb \
+    psutil_RtlNtStatusToDosErrorNoTeb;
diff --git a/psutil/arch/windows/ntextapi.h b/psutil/arch/windows/ntextapi.h
index 178f9866..b6f23d99 100644
--- a/psutil/arch/windows/ntextapi.h
+++ b/psutil/arch/windows/ntextapi.h
@@ -500,4 +500,8 @@ typedef NTSTATUS (NTAPI *_NtQueryVirtualMemory) (
     PSIZE_T ReturnLength
 );
 
+typedef ULONG (WINAPI *_RtlNtStatusToDosErrorNoTeb) (
+    NTSTATUS status
+);
+
 #endif // __NTEXTAPI_H__
diff --git a/psutil/arch/windows/process_handles.c b/psutil/arch/windows/process_handles.c
index 8b899972..5966669e 100644
--- a/psutil/arch/windows/process_handles.c
+++ b/psutil/arch/windows/process_handles.c
@@ -52,6 +52,7 @@ psutil_wait_thread(LPVOID lpvParam) {
     while (TRUE) {
         WaitForSingleObject(g_hEvtStart, INFINITE);
 
+        // TODO: return code not checked
         g_status = psutil_NtQueryObject(
             g_hFile,
             ObjectNameInformation,
@@ -159,8 +160,9 @@ psutil_get_open_files_ntqueryobject(long dwPid, HANDLE hProcess) {
                             &dwRet)) == STATUS_INFO_LENGTH_MISMATCH);
 
     // NtQuerySystemInformation stopped giving us STATUS_INFO_LENGTH_MISMATCH
-    if (!NT_SUCCESS(status)) {
-        PyErr_SetFromWindowsErr(HRESULT_FROM_NT(status));
+    if (! NT_SUCCESS(status)) {
+        psutil_SetFromNTStatusErr(
+            status, "NtQuerySystemInformation(SystemExtendedHandleInformation)");
         error = TRUE;
         goto cleanup;
     }
@@ -355,8 +357,9 @@ psutil_get_open_files_getmappedfilename(long dwPid, HANDLE hProcess) {
                             &dwRet)) == STATUS_INFO_LENGTH_MISMATCH);
 
     // NtQuerySystemInformation stopped giving us STATUS_INFO_LENGTH_MISMATCH
-    if (!NT_SUCCESS(status)) {
-        PyErr_SetFromWindowsErr(HRESULT_FROM_NT(status));
+    if (! NT_SUCCESS(status)) {
+        psutil_SetFromNTStatusErr(
+            status, "NtQuerySystemInformation(SystemExtendedHandleInformation)");
         error = TRUE;
         goto cleanup;
     }
diff --git a/psutil/arch/windows/process_info.c b/psutil/arch/windows/process_info.c
index 946a01cb..3b3c677e 100644
--- a/psutil/arch/windows/process_info.c
+++ b/psutil/arch/windows/process_info.c
@@ -483,6 +483,7 @@ psutil_get_process_data(long pid,
     BOOL theyAreWow64;
 #endif
     DWORD access = PROCESS_QUERY_INFORMATION | PROCESS_VM_READ;
+    NTSTATUS status;
 
     hProcess = psutil_handle_from_pid(pid, access);
     if (hProcess == NULL)
@@ -491,15 +492,16 @@ psutil_get_process_data(long pid,
 #ifdef _WIN64
     /* 64 bit case.  Check if the target is a 32 bit process running in WoW64
      * mode. */
-    if (! NT_SUCCESS(psutil_NtQueryInformationProcess(
-            hProcess,
-            ProcessWow64Information,
-            &ppeb32,
-            sizeof(LPVOID),
-            NULL)))
-    {
-        PyErr_SetFromOSErrnoWithSyscall(
-            "NtQueryInformationProcess(ProcessWow64Information)");
+    status = psutil_NtQueryInformationProcess(
+        hProcess,
+        ProcessWow64Information,
+        &ppeb32,
+        sizeof(LPVOID),
+        NULL);
+
+    if (!NT_SUCCESS(status)) {
+        psutil_SetFromNTStatusErr(
+            status, "NtQueryInformationProcess(ProcessWow64Information)");
         goto error;
     }
 
@@ -633,18 +635,20 @@ psutil_get_process_data(long pid,
         PEB_ peb;
         RTL_USER_PROCESS_PARAMETERS_ procParameters;
 
-        if (! NT_SUCCESS(psutil_NtQueryInformationProcess(
-                hProcess,
-                ProcessBasicInformation,
-                &pbi,
-                sizeof(pbi),
-                NULL)))
-        {
-            PyErr_SetFromOSErrnoWithSyscall(
-                "NtQueryInformationProcess(ProcessBasicInformation)");
+        status = psutil_NtQueryInformationProcess(
+            hProcess,
+            ProcessBasicInformation,
+            &pbi,
+            sizeof(pbi),
+            NULL);
+
+        if (!NT_SUCCESS(status)) {
+            psutil_SetFromNTStatusErr(
+                status, "NtQueryInformationProcess(ProcessBasicInformation)");
             goto error;
         }
 
+
         // read peb
         if (!ReadProcessMemory(hProcess,
                                pbi.PebBaseAddress,
@@ -767,10 +771,12 @@ psutil_cmdline_query_proc(long pid, WCHAR **pdata, SIZE_T *psize) {
         NULL,
         0,
         &bufLen);
+
     if (status != STATUS_BUFFER_OVERFLOW && \
             status != STATUS_BUFFER_TOO_SMALL && \
             status != STATUS_INFO_LENGTH_MISMATCH) {
-        PyErr_SetFromOSErrnoWithSyscall("NtQueryInformationProcess(0)");
+        psutil_SetFromNTStatusErr(
+            status, "NtQueryInformationProcess(ProcessBasicInformation)");
         goto error;
     }
 
@@ -789,8 +795,9 @@ psutil_cmdline_query_proc(long pid, WCHAR **pdata, SIZE_T *psize) {
         bufLen,
         &bufLen
     );
-    if (! NT_SUCCESS(status)) {
-        PyErr_SetFromOSErrnoWithSyscall("NtQueryInformationProcess(withlen)");
+    if (!NT_SUCCESS(status)) {
+        psutil_SetFromNTStatusErr(
+            status, "NtQueryInformationProcess(ProcessCommandLineInformation)");
         goto error;
     }
 
@@ -971,9 +978,9 @@ psutil_get_proc_info(DWORD pid, PSYSTEM_PROCESS_INFORMATION *retProcess,
         }
     }
 
-    if (status != 0) {
-        PyErr_Format(
-            PyExc_RuntimeError, "NtQuerySystemInformation() syscall failed");
+    if (! NT_SUCCESS(status)) {
+        psutil_SetFromNTStatusErr(
+            status, "NtQuerySystemInformation(SystemProcessInformation)");
         goto error;
     }
 
diff --git a/psutil/tests/test_contracts.py b/psutil/tests/test_contracts.py
index 08e9e9b8..adf7b680 100755
--- a/psutil/tests/test_contracts.py
+++ b/psutil/tests/test_contracts.py
@@ -614,8 +614,11 @@ class TestFetchAllProcesses(unittest.TestCase):
                         # commented as on Linux we might get
                         # '/foo/bar (deleted)'
                         # assert os.path.exists(nt.path), nt.path
-                elif fname in ('addr', 'perms'):
-                    assert value
+                elif fname == 'addr':
+                    assert value, repr(value)
+                elif fname == 'perms':
+                    if not WINDOWS:
+                        assert value, repr(value)
                 else:
                     self.assertIsInstance(value, (int, long))
                     self.assertGreaterEqual(value, 0)