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
