Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

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

to review the following change.


Change subject: dco-win: Fix crash when cancelling pending operation
......................................................................

dco-win: Fix crash when cancelling pending operation

The OVERLAPPED structure must remain valid for the entire duration of an
asynchronous operation. Previously, when a TCP connection was pending
inside the NEW_PEER call, the OVERLAPPED structure was defined as a
local variable within dco_p2p_new_peer().

When CancelIo() was called later from close_tun_handle(), the OVERLAPPED
structure was already out of scope, resulting in undefined behavior and
stack corruption.

This fix moves the OVERLAPPED structure to the tuntap struct, ensuring
it remains valid throughout the operation's lifetime.

GitHub: #715

Change-Id: Ib1db457c42a80f6b8fc0e3ceb4a895d4cf7f0155
Signed-off-by: Lev Stipakov <l...@openvpn.net>
---
M src/openvpn/dco_win.c
M src/openvpn/dco_win.h
M src/openvpn/socket.c
M src/openvpn/tun.h
4 files changed, 7 insertions(+), 6 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/28/928/1

diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 8b47124..a386e53 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -321,7 +321,7 @@
 }

 void
-dco_p2p_new_peer(HANDLE handle, struct link_socket *sock, struct signal_info 
*sig_info)
+dco_p2p_new_peer(HANDLE handle, OVERLAPPED *ov, struct link_socket *sock, 
struct signal_info *sig_info)
 {
     msg(D_DCO_DEBUG, "%s", __func__);

@@ -395,8 +395,8 @@
         ASSERT(0);
     }

-    OVERLAPPED ov = { 0 };
-    if (!DeviceIoControl(handle, OVPN_IOCTL_NEW_PEER, &peer, sizeof(peer), 
NULL, 0, NULL, &ov))
+    CLEAR(*ov);
+    if (!DeviceIoControl(handle, OVPN_IOCTL_NEW_PEER, &peer, sizeof(peer), 
NULL, 0, NULL, ov))
     {
         DWORD err = GetLastError();
         if (err != ERROR_IO_PENDING)
@@ -405,7 +405,7 @@
         }
         else
         {
-            dco_connect_wait(handle, &ov, 
get_server_poll_remaining_time(sock->server_poll_timeout), sig_info);
+            dco_connect_wait(handle, ov, 
get_server_poll_remaining_time(sock->server_poll_timeout), sig_info);
         }
     }
 }
diff --git a/src/openvpn/dco_win.h b/src/openvpn/dco_win.h
index 95c95c8..e8e4e22 100644
--- a/src/openvpn/dco_win.h
+++ b/src/openvpn/dco_win.h
@@ -63,7 +63,7 @@
 dco_mp_start_vpn(HANDLE handle, struct link_socket *sock);

 void
-dco_p2p_new_peer(HANDLE handle, struct link_socket *sock, struct signal_info 
*sig_info);
+dco_p2p_new_peer(HANDLE handle, OVERLAPPED *ov, struct link_socket *sock, 
struct signal_info *sig_info);

 void
 dco_start_tun(struct tuntap *tt);
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 09de1b0..beb31fa 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -2242,7 +2242,7 @@
     }
     else
     {
-        dco_p2p_new_peer(c->c1.tuntap->hand, sock, sig_info);
+        dco_p2p_new_peer(c->c1.tuntap->hand, &c->c1.tuntap->dco_new_peer_ov, 
sock, sig_info);
     }
     sock->sockflags |= SF_DCO_WIN;

diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index b616f5d..bcc23b4 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -215,6 +215,7 @@

 #ifdef _WIN32
     HANDLE hand;
+    OVERLAPPED dco_new_peer_ov; /* used for async NEW_PEER dco call, which 
might wait for TCP connect */
     struct overlapped_io reads;
     struct overlapped_io writes;
     struct rw_handle rw_handle;

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ib1db457c42a80f6b8fc0e3ceb4a895d4cf7f0155
Gerrit-Change-Number: 928
Gerrit-PatchSet: 1
Gerrit-Owner: stipa <lstipa...@gmail.com>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to