Avoid recursion in virtual_output_callback_func()

From: David Sommerseth <[email protected]...>

This solves a SEGV situation when using the management API while OpenVPN is closing down.

The situation happens when the management socket has closed and OpenVPN tries to write an error about this to the management socket. What happens is that

virtual_output_callback_func() is called, which then calls -> man_output_list_push_finalize()

    -> man_output_standalone()
       -> man_write()  <-- this does the socket write
          -> man_io_error()
             -> x_msg()
                -> virtual_output_print()
                   -> virtual_output_callback_func() (recursion start)

virtual_output_callback_func() do have a mechanism to avoid recursion, but that did not keep the recurse counter when man_output_list_push_finalize() is called.

This patch just reorganise the recursion block to also keep the counter while calling the other functions from virtual_output_callback_func()

---
 src/openvpn/manage.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

index d0bb416..0a4542a 100644
@@ -287,13 +287,13 @@ virtual_output_callback_func (void *arg, const unsigned int flags, const char *s
 
 # define AF_DID_PUSH  (1<<0)
 # define AF_DID_RESET (1<<1)
-  unsigned int action_flags = 0;
 
   if (!recursive_level) /* don't allow recursion */
     {
       struct gc_arena gc = gc_new ();
       struct log_entry e;
       const char *out = NULL;
+      unsigned int action_flags = 0;
 
       ++recursive_level;
 

@@ -334,14 +334,15 @@ virtual_output_callback_func (void *arg, const unsigned int flags, const char *s
 	    }
 	}
 
-      --recursive_level;
       gc_free (&gc);
-    }
 
-  if (action_flags & AF_DID_PUSH)
-    man_output_list_push_finalize (man);
-  if (action_flags & AF_DID_RESET)
-    man_reset_client_socket (man, true);
+      if (action_flags & AF_DID_PUSH)
+        man_output_list_push_finalize (man);
+      if (action_flags & AF_DID_RESET)
+        man_reset_client_socket (man, true);
+
+      --recursive_level;
+    }
 }
 
 /*

1.7.10.2

Hi,

ACK.

Basically, what it does is "*any* activity of virtual_output_callback_func() only takes place if not in-recursion". So in normal circumstances, nothing changes, and in the recursion case, nothing at all(!) is attempted
- which sounds like a sane thing to do.

gert

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Applied to master and beta/2.3 branches

commit b2b66179f6dcc37de9582d5c3044f0357dda3df3 (master) commit 5541ea21691b5b39adc4bd3e1ff1af86a050c71d (beta/2.3)

Author: David Sommerseth <[email protected]...>
Date:   Fri Nov 16 12:02:06 2012 +0100

    Avoid recursion in virtual_output_callback_func()
    
    Signed-off-by: David Sommerseth <[email protected]...>
    Acked-by: Gert Doering <[email protected]...>
    Message-Id: [email protected]...
    URL: http://article.gmane.org/gmane.network.openvpn.devel/7130

kind regards,

David Sommerseth