Re: [Openvpn-users] OpenVPN eats all my CPU when using the mgmnt interface

[James Yonan <jim@xxxxxxxxx>]

On Wed, 5 Jan 2005, James Yonan wrote:

> On Wed, 5 Jan 2005, Mathias Sundman wrote:
> 
> > On Wed, 5 Jan 2005, Mathias Sundman wrote:
> > 
> > > I just observed that one of my OpenVPN processes were using 99% of my CPU on 
> > > my newly installed password auth based firewall.
> > >
> > > After some testing I've found that it's the management interface that's 
> > > causing it. Starting OpenVPN and everything is calm, but as soon as I connect 
> > > to the management interface, OpenVPN starts looping eating up all CPU, and it 
> > > does not stop after I disconnect from the management interface. I have to 
> > > restart OpenVPN to get rid of the problem.
> > >
> > > Connecting with a OpenVPN client also causes openvpn to exit this loop.
> > >
> > > openvpn@fw-ktn:/etc/openvpn$ openvpn --version
> > > OpenVPN 2.0_rc5 i686-pc-linux [SSL] [LZO] built on Dec 16 2004
> > >
> > > openvpn@fw-ktn:/etc/openvpn$ uname -a
> > > Linux fw-ktn 2.4.26 #2 Wed Jun 30 15:30:57 CEST 2004 i686 unknown
> > >
> > > This is what I get when running strace on the process:
> > >
> > > poll([{fd=5, events=POLLIN|POLLPRI}, {fd=6, events=POLLIN|POLLPRI}, {fd=3, 
> > > events=POLLIN|POLLPRI}, {fd=7, events=POLLIN|POLLPRI, revents=POLLNVAL}], 4, 
> > > 10000) = 1
> > > time(NULL)                              = 1104923675
> > > poll([{fd=5, events=POLLIN|POLLPRI}, {fd=6, events=POLLIN|POLLPRI}, {fd=3, 
> > > events=POLLIN|POLLPRI}, {fd=7, events=POLLIN|POLLPRI, revents=POLLNVAL}], 4, 
> > > 10000) = 1
> > 
> > A little follow-up on this problem.
> > 
> > I had no problem reproducing it on an other machine, but I found another 
> > interesting thing. The problem only occurs when running openvpn as a 
> > tcp-server. If I use --proto udp the problem never occurs.
> 
> I've already found the bug.  It only occurs in tcp server mode, in event 
> poll mode, and when the management interface is being used.  Event poll 
> mode is used by all *nix OSes except for Linux 2.6, so the bug won't occur 
> there.
> 
> The bug occurs because a subtle difference in behavior between poll and
> epoll breaks the OpenVPN event abstraction layer which is supposed to
> (from an API perspective) hide the differences between poll, epoll,
> select, and WSAWaitForMultipleEvents. Basically epoll doesn't care if you
> close a file descriptor in the event list -- it will do the sane thing and
> remove it from the list automatically.  Poll on the other hand doesn't
> like to see a closed fd in the event list, and will return immediately
> with POLLNVAL.  So the fix is that the OpenVPN poll driver needs to 
> emulate the epoll behavior -- or OpenVPN needs to make sure that it 
> manually deletes all closed fds from any relevant event lists.
> 
> I should have a fix shortly.

Okay, here's a patch (attached) which fixes the above bug.  Please test.

This patch also includes:

* --ifconfig-push now accepts DNS names as well as
  IP addresses.
* Added sanity check errors when --pull or
  --auth-user-pass is used in an incorrect mode.

James

diff -ur openvpn-2.0_rc6/event.c openvpn-2.0_rc6c/event.c
--- openvpn-2.0_rc6/event.c	2004-12-12 23:21:04.000000000 -0600
+++ openvpn-2.0_rc6c/event.c	2005-01-06 20:28:59.000000000 -0600
@@ -495,12 +495,13 @@
 ep_del (struct event_set *es, event_t event)
 {
   struct epoll_event ev;
-
   struct ep_set *eps = (struct ep_set *) es;
+
+  dmsg (D_EVENT_WAIT, "EP_DEL ev=%d", (int)event);
+
   ASSERT (!eps->fast);
   CLEAR (ev);
-  if (epoll_ctl (eps->epfd, EPOLL_CTL_DEL, event, &ev) < 0)
-    msg (M_ERR, "EVENT: epoll_ctl EPOLL_CTL_DEL epfd=%d fd=%d failed", (int)eps->epfd, (int)event);
+  epoll_ctl (eps->epfd, EPOLL_CTL_DEL, event, &ev);
 }
 
 static void
@@ -641,6 +642,9 @@
 {
   struct po_set *pos = (struct po_set *) es;
   int i;
+
+  dmsg (D_EVENT_WAIT, "PO_DEL ev=%d", (int)event);
+
   ASSERT (!pos->fast);
   for (i = 0; i < pos->n_events; ++i)
     {
@@ -750,6 +754,10 @@
 	      ++out;
 	      ++j;
 	    }
+	  else if (pfdp->revents)
+	    {
+	      msg (D_EVENT_ERRORS, "Error: poll: unknown revents=0x%04x", (unsigned int)pfdp->revents);
+	    }
 	  ++pfdp;
 	}
       return j;
@@ -835,7 +843,7 @@
   struct se_set *ses = (struct se_set *) es;
   ASSERT (!ses->fast);
 
-  dmsg (D_EVENT_WAIT, "SE_DEL ev=%d", event);
+  dmsg (D_EVENT_WAIT, "SE_DEL ev=%d", (int)event);
 
   if (event >= 0 && event < ses->capacity)
     {
diff -ur openvpn-2.0_rc6/manage.c openvpn-2.0_rc6c/manage.c
--- openvpn-2.0_rc6/manage.c	2004-12-12 22:29:17.000000000 -0600
+++ openvpn-2.0_rc6c/manage.c	2005-01-06 18:22:39.000000000 -0600
@@ -189,6 +189,20 @@
 }
 
 static void
+man_close_socket (struct management *man, const socket_descriptor_t sd)
+{
+#ifndef WIN32
+  /*
+   * Windows doesn't need this because the ne32 event is permanently
+   * enabled at struct management scope.
+   */
+  if (man->persist.callback.delete_event)
+    (*man->persist.callback.delete_event) (man->persist.callback.arg, sd);
+#endif
+  openvpn_close_socket (sd);
+}
+
+static void
 virtual_output_callback_func (void *arg, const unsigned int flags, const char *str)
 {
   static int recursive_level = 0; /* GLOBAL */
@@ -744,7 +758,7 @@
 #ifdef WIN32
 	  man_stop_ne32 (man);
 #endif
-	  openvpn_close_socket (man->connection.sd_top);
+	  man_close_socket (man, man->connection.sd_top);
 	  man->connection.sd_top = SOCKET_UNDEFINED;
 	}
 
@@ -834,7 +848,7 @@
 #ifdef WIN32
       man_stop_ne32 (man);
 #endif
-      openvpn_close_socket (man->connection.sd_cli);
+      man_close_socket (man, man->connection.sd_cli);
       command_line_reset (man->connection.in);
       output_list_reset (man->connection.out);
     }
@@ -1179,17 +1193,19 @@
 }
 
 static void
-man_connection_close (struct man_connection *mc)
+man_connection_close (struct management *man)
 {
+  struct man_connection *mc = &man->connection;
+
   if (mc->es)
     event_free (mc->es);
 #ifdef WIN32
   net_event_win32_close (&mc->ne32);
 #endif
   if (socket_defined (mc->sd_top))
-    openvpn_close_socket (mc->sd_top);
+    man_close_socket (man, mc->sd_top);
   if (socket_defined (mc->sd_cli))
-    openvpn_close_socket (mc->sd_cli);
+    man_close_socket (man, mc->sd_cli);
   if (mc->in)
     command_line_free (mc->in);
   if (mc->out)
@@ -1269,7 +1285,7 @@
 void
 management_close (struct management *man)
 {
-  man_connection_close (&man->connection);
+  man_connection_close (man);
   man_settings_close (&man->settings);
   man_persist_close (&man->persist);
   free (man);
@@ -1375,7 +1391,7 @@
 management_pre_tunnel_close (struct management *man)
 {
   if (man->settings.management_over_tunnel)
-    man_connection_close (&man->connection);
+    man_connection_close (man);
 }
 
 void
diff -ur openvpn-2.0_rc6/manage.h openvpn-2.0_rc6c/manage.h
--- openvpn-2.0_rc6/manage.h	2004-12-11 18:37:03.000000000 -0600
+++ openvpn-2.0_rc6c/manage.h	2005-01-06 18:22:40.000000000 -0600
@@ -150,6 +150,7 @@
   void (*show_net) (void *arg, const int msglevel);
   int (*kill_by_cn) (void *arg, const char *common_name);
   int (*kill_by_addr) (void *arg, const in_addr_t addr, const int port);
+  void (*delete_event) (void *arg, event_t event);
 };
 
 /*
diff -ur openvpn-2.0_rc6/mtcp.c openvpn-2.0_rc6c/mtcp.c
--- openvpn-2.0_rc6/mtcp.c	2004-12-17 01:36:39.000000000 -0600
+++ openvpn-2.0_rc6c/mtcp.c	2005-01-06 18:16:40.000000000 -0600
@@ -194,6 +194,13 @@
 }
 
 void
+multi_tcp_delete_event (struct multi_tcp *mtcp, event_t event)
+{
+  if (mtcp && mtcp->es)
+    event_del (mtcp->es, event);
+}
+
+void
 multi_tcp_free (struct multi_tcp *mtcp)
 {
   if (mtcp)
diff -ur openvpn-2.0_rc6/mtcp.h openvpn-2.0_rc6c/mtcp.h
--- openvpn-2.0_rc6/mtcp.h	2004-12-16 08:16:39.000000000 -0600
+++ openvpn-2.0_rc6c/mtcp.h	2005-01-06 18:16:38.000000000 -0600
@@ -63,5 +63,7 @@
 
 void tunnel_server_tcp (struct context *top);
 
+void multi_tcp_delete_event (struct multi_tcp *mtcp, event_t event);
+
 #endif
 #endif
diff -ur openvpn-2.0_rc6/multi.c openvpn-2.0_rc6c/multi.c
--- openvpn-2.0_rc6/multi.c	2004-12-16 08:23:53.000000000 -0600
+++ openvpn-2.0_rc6c/multi.c	2005-01-06 18:23:08.000000000 -0600
@@ -2024,6 +2024,14 @@
   return count;
 }
 
+static void
+management_delete_event (void *arg, event_t event)
+{
+  struct multi_context *m = (struct multi_context *) arg;
+  if (m->mtcp)
+    multi_tcp_delete_event (m->mtcp, event);
+}
+
 #endif
 
 void
@@ -2039,6 +2047,7 @@
       cb.show_net = management_show_net_callback;
       cb.kill_by_cn = management_callback_kill_by_cn;
       cb.kill_by_addr = management_callback_kill_by_addr;
+      cb.delete_event = management_delete_event;
       management_set_callback (management, &cb);
     }
 #endif
diff -ur openvpn-2.0_rc6/options.c openvpn-2.0_rc6c/options.c
--- openvpn-2.0_rc6/options.c	2004-12-19 07:46:13.000000000 -0600
+++ openvpn-2.0_rc6c/options.c	2004-12-29 15:32:14.000000000 -0600
@@ -1634,6 +1634,9 @@
       MUST_BE_UNDEF (crl_file);
       MUST_BE_UNDEF (key_method);
       MUST_BE_UNDEF (ns_cert_type);
+
+      if (pull)
+	msg (M_USAGE, err, "--pull");
     }
 #undef MUST_BE_UNDEF
 #endif /* USE_CRYPTO */
@@ -1653,6 +1656,10 @@
       options->ping_rec_timeout = PRE_PULL_INITIAL_PING_RESTART;
       options->ping_rec_timeout_action = PING_RESTART;
     }
+  else if (options->auth_user_pass_file)
+    {
+      msg (M_USAGE, "--auth-user-pass requires --pull");
+    }
 
   /*
    * Save certain parms before modifying options via --pull
@@ -3697,8 +3704,8 @@
 
       i += 2;
       VERIFY_PERMISSION (OPT_P_INSTANCE);
-      local = getaddr (GETADDR_HOST_ORDER, p[1], 0, NULL, NULL);
-      remote_netmask = getaddr (GETADDR_HOST_ORDER, p[2], 0, NULL, NULL);
+      local = getaddr (GETADDR_HOST_ORDER|GETADDR_RESOLVE, p[1], 0, NULL, NULL);
+      remote_netmask = getaddr (GETADDR_HOST_ORDER|GETADDR_RESOLVE, p[2], 0, NULL, NULL);
       if (local && remote_netmask)
 	{
 	  options->push_ifconfig_defined = true;
diff -ur openvpn-2.0_rc6/plugin.c openvpn-2.0_rc6c/plugin.c
--- openvpn-2.0_rc6/plugin.c	2004-12-02 00:16:37.000000000 -0600
+++ openvpn-2.0_rc6c/plugin.c	2004-12-30 00:44:27.000000000 -0600
@@ -219,7 +219,7 @@
   gc_free (&gc);
 }
 
-static bool
+static int
 plugin_call_item (const struct plugin *p, const int type, const char *args, const char **envp)
 {
   int status = OPENVPN_PLUGIN_FUNC_SUCCESS;
@@ -296,7 +296,7 @@
 int
 plugin_call (const struct plugin_list *pl, const int type, const char *args, struct env_set *es)
 {
-  int ret = false;
+  int ret = 0;
 
   if (plugin_defined (pl, type))
     {
@@ -313,7 +313,7 @@
 	{
 	  if (plugin_call_item (&pl->plugins[i], type, args, envp)) /* if any one plugin in the chain fails, return failure */
 	    {
-	      ret = true;
+	      ret = 1;
 	      break;
 	    }
 	}
@@ -342,7 +342,7 @@
 bool
 plugin_defined (const struct plugin_list *pl, const int type)
 {
-  int ret = false;
+  bool ret = false;
   if (pl)
     {
       int i;