From: Lev Stipakov Date: Mon, 17 Jan 2022 09:49:17 +0000 (+0200) Subject: tun: remove tun_finalize() X-Git-Tag: v2.6_beta1~313 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bcf04b0b8e3c0f142ab0ec97627ea140c80c7962;p=thirdparty%2Fopenvpn.git tun: remove tun_finalize() tun_finalize() is essentially subset of socket_finalize() apart from: - using WSAFoo() functions instead of Foo() - "from" address is not returned There is no clear official statement that one can use non-WSA API on handles, so let's be on a safe side and use both. Introduce sockethandle_t abstraction, which represents socket and handle. Add SocketHandle* routines which call proper API depends on underlying type in abstraction. Rename socket_finalize() to sockethandle_finalize(), take sockethandle_t and new routines into use and kick tun_finalize(). Signed-off-by: Lev Stipakov Acked-by: Selva Nair Message-Id: <20220117094917.178-1-lstipakov@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23555.html Signed-off-by: Gert Doering --- diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index f82386a1d..a905f993a 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1115,7 +1115,8 @@ read_incoming_tun(struct context *c) } else { - read_tun_buffered(c->c1.tuntap, &c->c2.buf); + sockethandle_t sh = { .is_handle = true, .h = c->c1.tuntap->hand }; + sockethandle_finalize(sh, &c->c1.tuntap->reads, &c->c2.buf, NULL); } #else /* ifdef _WIN32 */ ASSERT(buf_init(&c->c2.buf, FRAME_HEADROOM(&c->c2.frame))); diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index df7367469..780c5cb3f 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -3198,7 +3198,8 @@ link_socket_read_tcp(struct link_socket *sock, if (!sock->stream_buf.residual_fully_formed) { #ifdef _WIN32 - len = socket_finalize(sock->sd, &sock->reads, buf, NULL); + sockethandle_t sh = { .s = sock->sd }; + len = sockethandle_finalize(sh, &sock->reads, buf, NULL); #else struct buffer frag; stream_buf_get_next(&sock->stream_buf, &frag); @@ -3664,10 +3665,10 @@ socket_send_queue(struct link_socket *sock, struct buffer *buf, const struct lin } int -socket_finalize(SOCKET s, - struct overlapped_io *io, - struct buffer *buf, - struct link_socket_actual *from) +sockethandle_finalize(sockethandle_t sh, + struct overlapped_io *io, + struct buffer *buf, + struct link_socket_actual *from) { int ret = -1; BOOL status; @@ -3675,13 +3676,7 @@ socket_finalize(SOCKET s, switch (io->iostate) { case IOSTATE_QUEUED: - status = WSAGetOverlappedResult( - s, - &io->overlapped, - &io->size, - FALSE, - &io->flags - ); + status = SocketHandleGetOverlappedResult(sh, io); if (status) { /* successful return for a queued operation */ @@ -3693,18 +3688,18 @@ socket_finalize(SOCKET s, io->iostate = IOSTATE_INITIAL; ASSERT(ResetEvent(io->overlapped.hEvent)); - dmsg(D_WIN32_IO, "WIN32 I/O: Socket Completion success [%d]", ret); + dmsg(D_WIN32_IO, "WIN32 I/O: Completion success [%d]", ret); } else { /* error during a queued operation */ ret = -1; - if (WSAGetLastError() != WSA_IO_INCOMPLETE) + if (SocketHandleGetLastError(sh) != ERROR_IO_INCOMPLETE) { /* if no error (i.e. just not finished yet), then DON'T execute this code */ io->iostate = IOSTATE_INITIAL; ASSERT(ResetEvent(io->overlapped.hEvent)); - msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Socket Completion error"); + msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Completion error"); } } break; @@ -3715,9 +3710,9 @@ socket_finalize(SOCKET s, if (io->status) { /* error return for a non-queued operation */ - WSASetLastError(io->status); + SocketHandleSetLastError(sh, io->status); ret = -1; - msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Socket Completion non-queued error"); + msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Completion non-queued error"); } else { @@ -3727,14 +3722,14 @@ socket_finalize(SOCKET s, *buf = io->buf; } ret = io->size; - dmsg(D_WIN32_IO, "WIN32 I/O: Socket Completion non-queued success [%d]", ret); + dmsg(D_WIN32_IO, "WIN32 I/O: Completion non-queued success [%d]", ret); } break; case IOSTATE_INITIAL: /* were we called without proper queueing? */ - WSASetLastError(WSAEINVAL); + SocketHandleSetInvalError(sh); ret = -1; - dmsg(D_WIN32_IO, "WIN32 I/O: Socket Completion BAD STATE"); + dmsg(D_WIN32_IO, "WIN32 I/O: Completion BAD STATE"); break; default: @@ -3742,7 +3737,7 @@ socket_finalize(SOCKET s, } /* return from address if requested */ - if (from) + if (!sh.is_handle && from) { if (ret >= 0 && io->addr_defined) { diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h index cc1e0c366..77c3d7209 100644 --- a/src/openvpn/socket.h +++ b/src/openvpn/socket.h @@ -262,11 +262,44 @@ int socket_send_queue(struct link_socket *sock, struct buffer *buf, const struct link_socket_actual *to); -int socket_finalize( - SOCKET s, - struct overlapped_io *io, - struct buffer *buf, - struct link_socket_actual *from); +typedef struct { + union { + SOCKET s; + HANDLE h; + }; + bool is_handle; +} sockethandle_t; + +int sockethandle_finalize(sockethandle_t sh, + struct overlapped_io *io, + struct buffer *buf, + struct link_socket_actual *from); + +static inline BOOL +SocketHandleGetOverlappedResult(sockethandle_t sh, struct overlapped_io *io) +{ + return sh.is_handle ? + GetOverlappedResult(sh.h, &io->overlapped, &io->size, FALSE) : + WSAGetOverlappedResult(sh.s, &io->overlapped, &io->size, FALSE, &io->flags); +} + +static inline int +SocketHandleGetLastError(sockethandle_t sh) +{ + return sh.is_handle ? (int)GetLastError() : WSAGetLastError(); +} + +inline static void +SocketHandleSetLastError(sockethandle_t sh, DWORD err) +{ + sh.is_handle ? SetLastError(err) : WSASetLastError(err); +} + +static inline void +SocketHandleSetInvalError(sockethandle_t sh) +{ + sh.is_handle ? SetLastError(ERROR_INVALID_FUNCTION) : WSASetLastError(WSAEINVAL); +} #else /* ifdef _WIN32 */ @@ -1020,7 +1053,8 @@ link_socket_read_udp_win32(struct link_socket *sock, struct buffer *buf, struct link_socket_actual *from) { - return socket_finalize(sock->sd, &sock->reads, buf, from); + sockethandle_t sh = { .s = sock->sd }; + return sockethandle_finalize(sh, &sock->reads, buf, from); } #else /* ifdef _WIN32 */ @@ -1078,9 +1112,10 @@ link_socket_write_win32(struct link_socket *sock, { int err = 0; int status = 0; + sockethandle_t sh = { .s = sock->sd }; if (overlapped_io_active(&sock->writes)) { - status = socket_finalize(sock->sd, &sock->writes, NULL, NULL); + status = sockethandle_finalize(sh, &sock->writes, NULL, NULL); if (status < 0) { err = WSAGetLastError(); diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 12bdd2005..fe32127b6 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -3561,87 +3561,29 @@ tun_write_queue(struct tuntap *tt, struct buffer *buf) } int -tun_finalize( - HANDLE h, - struct overlapped_io *io, - struct buffer *buf) +tun_write_win32(struct tuntap *tt, struct buffer *buf) { - int ret = -1; - BOOL status; - - switch (io->iostate) + int err = 0; + int status = 0; + if (overlapped_io_active(&tt->writes)) { - case IOSTATE_QUEUED: - status = GetOverlappedResult( - h, - &io->overlapped, - &io->size, - FALSE - ); - if (status) - { - /* successful return for a queued operation */ - if (buf) - { - *buf = io->buf; - } - ret = io->size; - io->iostate = IOSTATE_INITIAL; - ASSERT(ResetEvent(io->overlapped.hEvent)); - dmsg(D_WIN32_IO, "WIN32 I/O: TAP Completion success [%d]", ret); - } - else - { - /* error during a queued operation */ - ret = -1; - if (GetLastError() != ERROR_IO_INCOMPLETE) - { - /* if no error (i.e. just not finished yet), - * then DON'T execute this code */ - io->iostate = IOSTATE_INITIAL; - ASSERT(ResetEvent(io->overlapped.hEvent)); - msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: TAP Completion error"); - } - } - break; - - case IOSTATE_IMMEDIATE_RETURN: - io->iostate = IOSTATE_INITIAL; - ASSERT(ResetEvent(io->overlapped.hEvent)); - if (io->status) - { - /* error return for a non-queued operation */ - SetLastError(io->status); - ret = -1; - msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: TAP Completion non-queued error"); - } - else - { - /* successful return for a non-queued operation */ - if (buf) - { - *buf = io->buf; - } - ret = io->size; - dmsg(D_WIN32_IO, "WIN32 I/O: TAP Completion non-queued success [%d]", ret); - } - break; - - case IOSTATE_INITIAL: /* were we called without proper queueing? */ - SetLastError(ERROR_INVALID_FUNCTION); - ret = -1; - dmsg(D_WIN32_IO, "WIN32 I/O: TAP Completion BAD STATE"); - break; - - default: - ASSERT(0); + sockethandle_t sh = { .is_handle = true, .h = tt->hand }; + status = sockethandle_finalize(sh, &tt->writes, NULL, NULL); + if (status < 0) + { + err = GetLastError(); + } } - - if (buf) + tun_write_queue(tt, buf); + if (status < 0) { - buf->len = ret; + SetLastError(err); + return status; + } + else + { + return BLEN(buf); } - return ret; } static const struct device_instance_id_interface * diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h index d4657537c..739e93d76 100644 --- a/src/openvpn/tun.h +++ b/src/openvpn/tun.h @@ -437,8 +437,6 @@ int tun_read_queue(struct tuntap *tt, int maxsize); int tun_write_queue(struct tuntap *tt, struct buffer *buf); -int tun_finalize(HANDLE h, struct overlapped_io *io, struct buffer *buf); - static inline bool tuntap_stop(int status) { @@ -466,36 +464,7 @@ tuntap_abort(int status) return false; } -static inline int -tun_write_win32(struct tuntap *tt, struct buffer *buf) -{ - int err = 0; - int status = 0; - if (overlapped_io_active(&tt->writes)) - { - status = tun_finalize(tt->hand, &tt->writes, NULL); - if (status < 0) - { - err = GetLastError(); - } - } - tun_write_queue(tt, buf); - if (status < 0) - { - SetLastError(err); - return status; - } - else - { - return BLEN(buf); - } -} - -static inline int -read_tun_buffered(struct tuntap *tt, struct buffer *buf) -{ - return tun_finalize(tt->hand, &tt->reads, buf); -} +int tun_write_win32(struct tuntap *tt, struct buffer *buf); static inline ULONG wintun_ring_packet_align(ULONG size)