[prev in list] [next in list] [prev in thread] [next in thread]
List: openvpn-devel
Subject: Re: [Openvpn-devel] [PATCH v2 3/7] tun.c: make wintun_register_ring_buffer() non-fatal on failures
From: Lev Stipakov <lstipakov () gmail ! com>
Date: 2019-12-20 18:25:21
Message-ID: CAGyAFMXy4FenefxAgJc+dSP2sYAn_994qvwJK8-44B1Ktk0m2g () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
No changes since v1.
Acked-by: Lev Stipakov <lstipakov@gmail.com>
pe 20. jouluk. 2019 klo 18.14 Simon Rozman (simon@rozman.si) kirjoitti:
> Wintun allows multiple handles to be opened on it's NDIS device pipe.
> Just by succeeding to open the pipe does not warrant the adapter is
> unused.
>
> When iterating for available Wintun adapter, we will need to try
> registering ring buffers with each one to actually determine which one
> is used and which one is not.
>
> Therefore, a failure to register ring buffers should be detectable, but
> not M_FATAL.
>
> Signed-off-by: Simon Rozman <simon@rozman.si>
> ---
> src/openvpn/tun.c | 26 ++++++++++++++++++--------
> 1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index f90f201d..c6bbbd41 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -5647,11 +5647,12 @@ register_dns_service(const struct tuntap *tt)
> gc_free(&gc);
> }
>
> -static void
> +static bool
> service_register_ring_buffers(const struct tuntap *tt)
> {
> HANDLE msg_channel = tt->options.msg_channel;
> ack_message_t ack;
> + bool ret = true;
> struct gc_arena gc = gc_new();
>
> register_ring_buffers_message_t msg = {
> @@ -5669,13 +5670,13 @@ service_register_ring_buffers(const struct tuntap
> *tt)
>
> if (!send_msg_iservice(msg_channel, &msg, sizeof(msg), &ack,
> "Register ring buffers"))
> {
> - gc_free(&gc);
> - return;
> + ret = false;
> }
> else if (ack.error_number != NO_ERROR)
> {
> - msg(M_FATAL, "Register ring buffers failed using service: %s
> [status=0x%x]",
> + msg(M_NONFATAL, "Register ring buffers failed using service: %s
> [status=0x%x]",
> strerror_win32(ack.error_number, &gc), ack.error_number);
> + ret = false;
> }
> else
> {
> @@ -5683,6 +5684,7 @@ service_register_ring_buffers(const struct tuntap
> *tt)
> }
>
> gc_free(&gc);
> + return ret;
> }
>
> void
> @@ -5922,9 +5924,11 @@ tuntap_set_ip_addr(struct tuntap *tt,
> gc_free(&gc);
> }
>
> -static void
> +static bool
> wintun_register_ring_buffer(struct tuntap *tt)
> {
> + bool ret = true;
> +
> tt->wintun_send_ring = (struct tun_ring
> *)MapViewOfFile(tt->wintun_send_ring_handle,
>
> FILE_MAP_ALL_ACCESS,
> 0,
> @@ -5939,7 +5943,7 @@ wintun_register_ring_buffer(struct tuntap *tt)
>
> if (tt->options.msg_channel)
> {
> - service_register_ring_buffers(tt);
> + ret = service_register_ring_buffers(tt);
> }
> else
> {
> @@ -5953,13 +5957,16 @@ wintun_register_ring_buffer(struct tuntap *tt)
> tt->rw_handle.read,
> tt->rw_handle.write))
> {
> - msg(M_FATAL, "ERROR: Failed to register ring buffers: %lu",
> GetLastError());
> + msg(M_NONFATAL, "Failed to register ring buffers: %lu",
> GetLastError());
> + ret = false;
> }
> if (!RevertToSelf())
> {
> msg(M_FATAL, "ERROR: RevertToSelf error: %lu",
> GetLastError());
> }
> }
> +
> + return ret;
> }
>
> static void
> @@ -6367,7 +6374,10 @@ open_tun(const char *dev, const char *dev_type,
> const char *dev_node, struct tun
>
> if (tt->wintun)
> {
> - wintun_register_ring_buffer(tt);
> + if (!wintun_register_ring_buffer(tt))
> + {
> + msg(M_FATAL, "Failed to register ring buffers");
> + }
> }
> else
> {
> --
> 2.24.1.windows.2
>
>
>
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
--
-Lev
[Attachment #5 (text/html)]
<div dir="ltr">No changes since v1.<div><br></div><div>Acked-by: Lev Stipakov <<a \
href="mailto:lstipakov@gmail.com">lstipakov@gmail.com</a>></div></div><br><div \
class="gmail_quote"><div dir="ltr" class="gmail_attr">pe 20. jouluk. 2019 klo 18.14 \
Simon Rozman (<a href="mailto:simon@rozman.si">simon@rozman.si</a>) \
kirjoitti:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Wintun allows multiple \
handles to be opened on it's NDIS device pipe.<br> Just by succeeding to open the \
pipe does not warrant the adapter is<br> unused.<br>
<br>
When iterating for available Wintun adapter, we will need to try<br>
registering ring buffers with each one to actually determine which one<br>
is used and which one is not.<br>
<br>
Therefore, a failure to register ring buffers should be detectable, but<br>
not M_FATAL.<br>
<br>
Signed-off-by: Simon Rozman <<a href="mailto:simon@rozman.si" \
target="_blank">simon@rozman.si</a>><br>
---<br>
src/openvpn/tun.c | 26 ++++++++++++++++++--------<br>
1 file changed, 18 insertions(+), 8 deletions(-)<br>
<br>
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c<br>
index f90f201d..c6bbbd41 100644<br>
--- a/src/openvpn/tun.c<br>
+++ b/src/openvpn/tun.c<br>
@@ -5647,11 +5647,12 @@ register_dns_service(const struct tuntap *tt)<br>
gc_free(&gc);<br>
}<br>
<br>
-static void<br>
+static bool<br>
service_register_ring_buffers(const struct tuntap *tt)<br>
{<br>
HANDLE msg_channel = tt->options.msg_channel;<br>
ack_message_t ack;<br>
+ bool ret = true;<br>
struct gc_arena gc = gc_new();<br>
<br>
register_ring_buffers_message_t msg = {<br>
@@ -5669,13 +5670,13 @@ service_register_ring_buffers(const struct tuntap *tt)<br>
<br>
if (!send_msg_iservice(msg_channel, &msg, sizeof(msg), &ack, \
"Register ring buffers"))<br> {<br>
- gc_free(&gc);<br>
- return;<br>
+ ret = false;<br>
}<br>
else if (ack.error_number != NO_ERROR)<br>
{<br>
- msg(M_FATAL, "Register ring buffers failed using service: %s \
[status=0x%x]",<br> + msg(M_NONFATAL, "Register ring buffers \
failed using service: %s [status=0x%x]",<br>
strerror_win32(ack.error_number, &gc), ack.error_number);<br>
+ ret = false;<br>
}<br>
else<br>
{<br>
@@ -5683,6 +5684,7 @@ service_register_ring_buffers(const struct tuntap *tt)<br>
}<br>
<br>
gc_free(&gc);<br>
+ return ret;<br>
}<br>
<br>
void<br>
@@ -5922,9 +5924,11 @@ tuntap_set_ip_addr(struct tuntap *tt,<br>
gc_free(&gc);<br>
}<br>
<br>
-static void<br>
+static bool<br>
wintun_register_ring_buffer(struct tuntap *tt)<br>
{<br>
+ bool ret = true;<br>
+<br>
tt->wintun_send_ring = (struct tun_ring \
*)MapViewOfFile(tt->wintun_send_ring_handle,<br>
\
FILE_MAP_ALL_ACCESS,<br>
\
0,<br> @@ -5939,7 +5943,7 @@ wintun_register_ring_buffer(struct tuntap *tt)<br>
<br>
if (tt->options.msg_channel)<br>
{<br>
- service_register_ring_buffers(tt);<br>
+ ret = service_register_ring_buffers(tt);<br>
}<br>
else<br>
{<br>
@@ -5953,13 +5957,16 @@ wintun_register_ring_buffer(struct tuntap *tt)<br>
tt->rw_handle.read,<br>
tt->rw_handle.write))<br>
{<br>
- msg(M_FATAL, "ERROR: Failed to register ring buffers: \
%lu", GetLastError());<br> + msg(M_NONFATAL, "Failed to \
register ring buffers: %lu", GetLastError());<br> + ret = \
false;<br> }<br>
if (!RevertToSelf())<br>
{<br>
msg(M_FATAL, "ERROR: RevertToSelf error: %lu", \
GetLastError());<br> }<br>
}<br>
+<br>
+ return ret;<br>
}<br>
<br>
static void<br>
@@ -6367,7 +6374,10 @@ open_tun(const char *dev, const char *dev_type, const char \
*dev_node, struct tun<br> <br>
if (tt->wintun)<br>
{<br>
- wintun_register_ring_buffer(tt);<br>
+ if (!wintun_register_ring_buffer(tt))<br>
+ {<br>
+ msg(M_FATAL, "Failed to register ring buffers");<br>
+ }<br>
}<br>
else<br>
{<br>
-- <br>
2.24.1.windows.2<br>
<br>
<br>
<br>
_______________________________________________<br>
Openvpn-devel mailing list<br>
<a href="mailto:Openvpn-devel@lists.sourceforge.net" \
target="_blank">Openvpn-devel@lists.sourceforge.net</a><br> <a \
href="https://lists.sourceforge.net/lists/listinfo/openvpn-devel" rel="noreferrer" \
target="_blank">https://lists.sourceforge.net/lists/listinfo/openvpn-devel</a><br> \
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" \
class="gmail_signature">-Lev</div>
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic