Skip to contentSkip to conversation

Send stdout and stderr of OpenVPN started by interactive service to NUL

PATCH In review Selva Nair 2 participants 2 messages Feb 14, 2016 – Feb 25, 2016

lore
Share & tools
lore

Selva Nair

author
#0 lore raw

PATCH 1/1 Send stdout and stderr of OpenVPN started by interactive service to NUL

Currently the service directs stdout/stderr of openvpn process to a pipe. The service never reads from it unless the process exits with an error. This causes the process to hang when large amount of log is written to stdout.

- Direct stdout/stderr to NUL
- Write the exit code (if nonzero) to the event log

Signed-off-by Selva Nair <[email protected]...>
---
 src/openvpnserv/interactive.c |   25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)
src/openvpnserv/interactive.c +18 -7
index 0f3d1d4..839a5e8 100644
@@ -732,7 +732,7 @@ RunOpenvpn (LPVOID p)
   PTOKEN_USER svc_user, ovpn_user;
   HANDLE svc_token = NULL, imp_token = NULL, pri_token = NULL;
   HANDLE stdin_read = NULL, stdin_write = NULL;
-  HANDLE stdout_read = NULL, stdout_write = NULL;
+  HANDLE stdout_write = NULL;
   DWORD pipe_mode, len, exit_code = 0;
   STARTUP_DATA sud = { 0, 0, 0 };
   STARTUPINFOW startup_info;

@@ -874,10 +874,17 @@ RunOpenvpn (LPVOID p)
       goto out;
     }
 
+  /* use /dev/null for stdout of openvpn (client should use --log for output) */
+  stdout_write = CreateFile(_T("NUL"), GENERIC_WRITE, FILE_SHARE_WRITE,
+                            &inheritable, OPEN_EXISTING, 0, NULL);
+  if (stdout_write == INVALID_HANDLE_VALUE)
+    {
+      ReturnLastError (pipe, L"CreateFile for stdout");
+      goto out;
+    }
+
   if (!CreatePipe(&stdin_read, &stdin_write, &inheritable, 0) ||
-      !CreatePipe(&stdout_read, &stdout_write, &inheritable, 0) ||
-      !SetHandleInformation(stdin_write, HANDLE_FLAG_INHERIT, 0) ||
-      !SetHandleInformation(stdout_read, HANDLE_FLAG_INHERIT, 0))
+      !SetHandleInformation(stdin_write, HANDLE_FLAG_INHERIT, 0))
     {
       ReturnLastError (pipe, L"CreatePipe");
       goto out;

@@ -985,8 +992,13 @@ RunOpenvpn (LPVOID p)
   if (exit_code == STILL_ACTIVE)
     TerminateProcess (proc_info.hProcess, 1);
   else if (exit_code != 0)
-    ReturnOpenvpnOutput (pipe, stdout_read, 1, &exit_event);
-
+    {
+      WCHAR buf[256];
+      int len = _snwprintf (buf, _countof (buf),
+                         L"OpenVPN exited with error: exit code = %lu", exit_code);
+      buf[_countof (buf) - 1] =  L'\0';
+      ReturnError (pipe, ERROR_OPENVPN_STARTUP, buf, 1, &exit_event);
+    }
   Undo (&undo_lists);
 
 out:

@@ -1002,7 +1014,6 @@ out:
   CloseHandleEx (&proc_info.hThread);
   CloseHandleEx (&stdin_read);
   CloseHandleEx (&stdin_write);
-  CloseHandleEx (&stdout_read);
   CloseHandleEx (&stdout_write);
   CloseHandleEx (&svc_token);
   CloseHandleEx (&imp_token);
Signature · 1 line
1.7.10.4
#1 lore raw

ACK. Fixes the "OpenVPN.exe hanging with no understandable reason when the log file cannot be written to" issue for good (logging on management interface will still work).

Again, I might overlook something Windows special, but from a generic C and "on unix, the equivalent stuff would work correctly" perspective, the code looks good. More review welcome, though.

Your patch has been applied to the master branch.

commit 852f1e49c7e692c6392fe07160cfbc8d6b17f0d0

Author: Selva Nair
Date:   Sun Feb 14 12:28:01 2016 -0500
     Send stdout and stderr of OpenVPN started by interactive service to NUL

     Signed-off-by: Selva Nair <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11161
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering