[S] Change in openvpn[master]: Avoid recursive call of man_read

Attention is currently required from: plaisthos.

Hello plaisthos, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1639?usp=email

to look at the new patch set (#2).

The following approvals got outdated and were removed:
Code-Review+1 by plaisthos


Change subject: Avoid recursive call of man_read
......................................................................

Avoid recursive call of man_read

Recursion to `man_read()` happens on Windows when `man_process_command`
tries to log the received command because `management_io`
first checks FD_READ and calls `man_read()` again if there is data is
pending in the receive buffer. This is buggy as `man_read()` is not
designed to be re-entrant.

Fix this by disallowing recursion -- just return 0 so that managament_io
will complete without recursion. Check this condition and do not clear
FD_READ from the event mask in management_io when man_read returns
without calling recv(). Read will get serviced next time when the main event loop
calls `management_io` thorugh `process_io`.  Note that `management_io`
always calls `reset_net_event_win32()` (manage.c: 3298) which will pickup
if any new data to read is pending (win32.c: 357) as well as retain FD_READ if
already set.

Giithub: Fixes OpenVPN/openvpn#1023

Change-Id: I9433baee1f5a66d43d4e43e03128ae6a790a7937
Signed-off-by: Selva Nair <[email protected]>
---
M src/openvpn/manage.c
M src/openvpn/manage.h
2 files changed, 16 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/39/1639/2

diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 2a3023c..31c4bb5 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -2426,6 +2426,12 @@
     unsigned char buf[256];
     ssize_t len = 0;
 
+    if (man->connection.man_read_state != MAN_READ_IDLE)
+    {
+        return 0; /* not ready to read now */
+    }
+    man->connection.man_read_state = MAN_READ_ACTIVE;
+
 #ifdef TARGET_ANDROID
     int fd;
     len = man_recv_with_fd(man->connection.sd_cli, buf, sizeof(buf), MSG_NOSIGNAL, &fd);
@@ -2508,6 +2514,7 @@
             man_reset_client_socket(man, false);
         }
     }
+    man->connection.man_read_state = MAN_READ_IDLE;
     return len;
 }
 
@@ -3309,10 +3316,14 @@
             {
                 if (net_events & FD_READ)
                 {
-                    while (man_read(man) > 0)
+                    if (man->connection.man_read_state == MAN_READ_IDLE)
                     {
+                        while (man_read(man) > 0)
+                        {
+                        }
+                        net_event_win32_clear_selected_events(&man->connection.ne32, FD_READ);
                     }
-                    net_event_win32_clear_selected_events(&man->connection.ne32, FD_READ);
+                    /* if ACTIVE: reentrancy, skip read and skip clearing FD_READ */
                 }
 
                 if (net_events & FD_WRITE)
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index 27d3b60..0632500 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -328,6 +328,9 @@
     int lastfdreceived;
 #endif
     int client_version;
+#define MAN_READ_IDLE   0
+#define MAN_READ_ACTIVE 1
+    int man_read_state;
 };
 
 struct management

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1639?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I9433baee1f5a66d43d4e43e03128ae6a790a7937
Gerrit-Change-Number: 1639
Gerrit-PatchSet: 2
Gerrit-Owner: selvanair <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel