diff mbox series

[Openvpn-devel,v3,4/7] wintun: register ring buffers when iterating adapters

Message ID 20191220193805.34-1-simon@rozman.si
State Accepted
Headers
Series None |

Commit Message

Simon Rozman Dec. 20, 2019, 8:38 a.m. UTC 
  Wintun adapters may be considered available if ring buffer registration
succeeded. Therefore, we must attempt to register ring buffers when
iterating adapters and continue on failure.

Signed-off-by: Simon Rozman <simon@rozman.si>
---
 src/openvpn/tun.c | 111 +++++++++++++++++++++++++++-------------------
 1 file changed, 65 insertions(+), 46 deletions(-)

Comments

Lev Stipakov Dec. 20, 2019, 9:15 a.m. UTC | #1 
Compiled, tested.

Acked-by: Lev Stipakov <lstipakov@gmail.com>

PS

When you send v2 etc, could you add changelog to git mail after "---" line.

See https://patchwork.openvpn.net/patch/388/ as an example:

    Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
    ---

    Changes from v2:
    - patchset rebased on top of pre-ipv6-only patchset
    - change commit subject/message
    - move ifconfig-ipv6-pool check change into 4/8

    src/openvpn/helper.c | 8 ++++++--
    1 file changed, 6 insertions(+), 2 deletions(-)

    diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c


pe 20. jouluk. 2019 klo 21.39 Simon Rozman (simon@rozman.si) kirjoitti:

> Wintun adapters may be considered available if ring buffer registration
> succeeded. Therefore, we must attempt to register ring buffers when
> iterating adapters and continue on failure.
>
> Signed-off-by: Simon Rozman <simon@rozman.si>
> ---
>  src/openvpn/tun.c | 111 +++++++++++++++++++++++++++-------------------
>  1 file changed, 65 insertions(+), 46 deletions(-)
>
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index c6bbbd41..aef2c7f8 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -6129,11 +6129,72 @@ tuntap_dhcp_mask(const struct tuntap *tt, const
> char *device_guid)
>      gc_free(&gc);
>  }
>
> +static bool
> +tun_try_open_device(struct tuntap *tt, const char *device_guid, const
> struct device_instance_id_interface *device_instance_id_interface)
> +{
> +    const char *path = NULL;
> +    char tuntap_device_path[256];
> +
> +    if (tt->wintun)
> +    {
> +        const struct device_instance_id_interface *dev_if;
> +
> +        /* Open Wintun adapter */
> +        for (dev_if = device_instance_id_interface; dev_if != NULL;
> dev_if = dev_if->next)
> +        {
> +            if (strcmp(dev_if->net_cfg_instance_id, device_guid) == 0)
> +            {
> +                path = dev_if->device_interface_list;
> +                break;
> +            }
> +        }
> +        if (path == NULL)
> +        {
> +            return false;
> +        }
> +    }
> +    else
> +    {
> +        /* Open TAP-Windows adapter */
> +        openvpn_snprintf(tuntap_device_path, sizeof(tuntap_device_path),
> "%s%s%s",
> +                            USERMODEDEVICEDIR,
> +                            device_guid,
> +                            TAP_WIN_SUFFIX);
> +        path = tuntap_device_path;
> +    }
> +
> +    tt->hand = CreateFile(path,
> +                   GENERIC_READ | GENERIC_WRITE,
> +                   0,                /* was: FILE_SHARE_READ */
> +                   0,
> +                   OPEN_EXISTING,
> +                   FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED,
> +                   0);
> +    if (tt->hand == INVALID_HANDLE_VALUE)
> +    {
> +        msg(D_TUNTAP_INFO, "CreateFile failed on %s device: %s",
> tt->wintun ? "Wintun" : "TAP-Windows", path);
> +        return false;
> +    }
> +
> +    if (tt->wintun)
> +    {
> +        /* Wintun adapter may be considered "open" after ring buffers are
> successfuly registered. */
> +        if (!wintun_register_ring_buffer(tt))
> +        {
> +            msg(D_TUNTAP_INFO, "Failed to register %s adapter ring
> buffers", device_guid);
> +            CloseHandle(tt->hand);
> +            tt->hand = NULL;
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
>  static void
>  tun_open_device(struct tuntap *tt, const char *dev_node, const char
> **device_guid)
>  {
>      struct gc_arena gc = gc_new();
> -    char *path = NULL;
>      char tuntap_device_path[256];
>      const struct tap_reg* tap_reg = get_tap_reg(&gc);
>      const struct panel_reg* panel_reg = get_panel_reg(&gc);
> @@ -6197,27 +6258,11 @@ tun_open_device(struct tuntap *tt, const char
> *dev_node, const char **device_gui
>
>              if (tt->wintun)
>              {
> -                const struct device_instance_id_interface* dev_if;
> -
>                  if (!is_picked_device_wintun)
>                  {
>                      /* wintun driver specified but picked adapter is not
> wintun, proceed to next one */
>                      goto next;
>                  }
> -
> -                path = NULL;
> -                for (dev_if = device_instance_id_interface; dev_if !=
> NULL; dev_if = dev_if->next)
> -                {
> -                    if (strcmp(dev_if->net_cfg_instance_id, *device_guid)
> == 0)
> -                    {
> -                        path = (char*)dev_if->device_interface_list;
> -                        break;
> -                    }
> -                }
> -                if (path == NULL)
> -                {
> -                    goto next;
> -                }
>              }
>              else
>              {
> @@ -6226,28 +6271,9 @@ tun_open_device(struct tuntap *tt, const char
> *dev_node, const char **device_gui
>                      /* tap-windows6 driver specified but picked adapter
> is wintun, proceed to next one */
>                      goto next;
>                  }
> -
> -                /* Open Windows TAP-Windows adapter */
> -                openvpn_snprintf(tuntap_device_path,
> sizeof(tuntap_device_path), "%s%s%s",
> -                                 USERMODEDEVICEDIR,
> -                                 *device_guid,
> -                                 TAP_WIN_SUFFIX);
> -                path = tuntap_device_path;
>              }
>
> -            tt->hand = CreateFile(path,
> -                                  GENERIC_READ | GENERIC_WRITE,
> -                                  0,                /* was:
> FILE_SHARE_READ */
> -                                  0,
> -                                  OPEN_EXISTING,
> -                                  FILE_ATTRIBUTE_SYSTEM |
> FILE_FLAG_OVERLAPPED,
> -                                  0);
> -
> -            if (tt->hand == INVALID_HANDLE_VALUE)
> -            {
> -                msg(D_TUNTAP_INFO, "CreateFile failed on %s device: %s",
> tt->wintun ? "wintun" : "TAP", tuntap_device_path);
> -            }
> -            else
> +            if (tun_try_open_device(tt, *device_guid,
> device_instance_id_interface))
>              {
>                  break;
>              }
> @@ -6261,7 +6287,7 @@ tun_open_device(struct tuntap *tt, const char
> *dev_node, const char **device_gui
>       * GUID using the registry */
>      tt->actual_name = string_alloc(actual_buffer, NULL);
>
> -    msg(M_INFO, "%s device [%s] opened: %s", tt->wintun ? "Wintun" :
> "TAP-WIN32", tt->actual_name, path);
> +    msg(M_INFO, "%s device [%s] opened", tt->wintun ? "Wintun" :
> "TAP-WIN32", tt->actual_name);
>      tt->adapter_index = get_adapter_index(*device_guid);
>
>      gc_free(&gc);
> @@ -6372,14 +6398,7 @@ open_tun(const char *dev, const char *dev_type,
> const char *dev_node, struct tun
>
>      tun_open_device(tt, dev_node, &device_guid);
>
> -    if (tt->wintun)
> -    {
> -        if (!wintun_register_ring_buffer(tt))
> -        {
> -            msg(M_FATAL, "Failed to register ring buffers");
> -        }
> -    }
> -    else
> +    if (!tt->wintun)
>      {
>          tuntap_post_open(tt, device_guid);
>      }
> --
> 2.24.1.windows.2
>
> Argh, I had --windows-driver accidentially left on TAP-Windows6 when
> testing v2 patchset. :(
>
>
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
Gert Doering Dec. 22, 2019, 8:09 a.m. UTC | #2 
Your patch has been applied to the master branch.

Compile tested on Ubuntu 1604/mingw.  Lightly stared at the code.  

Explanation makes sense (but I have not looked hard enough to see if
it actually does what it says :) ).

commit c1a555585c90aa26390ef2f50cadd479b47541f2
Author: Simon Rozman
Date:   Fri Dec 20 20:38:05 2019 +0100

     wintun: register ring buffers when iterating adapters

     Signed-off-by: Simon Rozman <simon@rozman.si>
     Acked-by: Lev Stipakov <lstipakov@gmail.com>
     Message-Id: <20191220193805.34-1-simon@rozman.si>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19288.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index c6bbbd41..aef2c7f8 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -6129,11 +6129,72 @@  tuntap_dhcp_mask(const struct tuntap *tt, const char *device_guid)
     gc_free(&gc);
 }
 
+static bool
+tun_try_open_device(struct tuntap *tt, const char *device_guid, const struct device_instance_id_interface *device_instance_id_interface)
+{
+    const char *path = NULL;
+    char tuntap_device_path[256];
+
+    if (tt->wintun)
+    {
+        const struct device_instance_id_interface *dev_if;
+
+        /* Open Wintun adapter */
+        for (dev_if = device_instance_id_interface; dev_if != NULL; dev_if = dev_if->next)
+        {
+            if (strcmp(dev_if->net_cfg_instance_id, device_guid) == 0)
+            {
+                path = dev_if->device_interface_list;
+                break;
+            }
+        }
+        if (path == NULL)
+        {
+            return false;
+        }
+    }
+    else
+    {
+        /* Open TAP-Windows adapter */
+        openvpn_snprintf(tuntap_device_path, sizeof(tuntap_device_path), "%s%s%s",
+                            USERMODEDEVICEDIR,
+                            device_guid,
+                            TAP_WIN_SUFFIX);
+        path = tuntap_device_path;
+    }
+
+    tt->hand = CreateFile(path,
+                   GENERIC_READ | GENERIC_WRITE,
+                   0,                /* was: FILE_SHARE_READ */
+                   0,
+                   OPEN_EXISTING,
+                   FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED,
+                   0);
+    if (tt->hand == INVALID_HANDLE_VALUE)
+    {
+        msg(D_TUNTAP_INFO, "CreateFile failed on %s device: %s", tt->wintun ? "Wintun" : "TAP-Windows", path);
+        return false;
+    }
+
+    if (tt->wintun)
+    {
+        /* Wintun adapter may be considered "open" after ring buffers are successfuly registered. */
+        if (!wintun_register_ring_buffer(tt))
+        {
+            msg(D_TUNTAP_INFO, "Failed to register %s adapter ring buffers", device_guid);
+            CloseHandle(tt->hand);
+            tt->hand = NULL;
+            return false;
+        }
+    }
+
+    return true;
+}
+
 static void
 tun_open_device(struct tuntap *tt, const char *dev_node, const char **device_guid)
 {
     struct gc_arena gc = gc_new();
-    char *path = NULL;
     char tuntap_device_path[256];
     const struct tap_reg* tap_reg = get_tap_reg(&gc);
     const struct panel_reg* panel_reg = get_panel_reg(&gc);
@@ -6197,27 +6258,11 @@  tun_open_device(struct tuntap *tt, const char *dev_node, const char **device_gui
 
             if (tt->wintun)
             {
-                const struct device_instance_id_interface* dev_if;
-
                 if (!is_picked_device_wintun)
                 {
                     /* wintun driver specified but picked adapter is not wintun, proceed to next one */
                     goto next;
                 }
-
-                path = NULL;
-                for (dev_if = device_instance_id_interface; dev_if != NULL; dev_if = dev_if->next)
-                {
-                    if (strcmp(dev_if->net_cfg_instance_id, *device_guid) == 0)
-                    {
-                        path = (char*)dev_if->device_interface_list;
-                        break;
-                    }
-                }
-                if (path == NULL)
-                {
-                    goto next;
-                }
             }
             else
             {
@@ -6226,28 +6271,9 @@  tun_open_device(struct tuntap *tt, const char *dev_node, const char **device_gui
                     /* tap-windows6 driver specified but picked adapter is wintun, proceed to next one */
                     goto next;
                 }
-
-                /* Open Windows TAP-Windows adapter */
-                openvpn_snprintf(tuntap_device_path, sizeof(tuntap_device_path), "%s%s%s",
-                                 USERMODEDEVICEDIR,
-                                 *device_guid,
-                                 TAP_WIN_SUFFIX);
-                path = tuntap_device_path;
             }
 
-            tt->hand = CreateFile(path,
-                                  GENERIC_READ | GENERIC_WRITE,
-                                  0,                /* was: FILE_SHARE_READ */
-                                  0,
-                                  OPEN_EXISTING,
-                                  FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED,
-                                  0);
-
-            if (tt->hand == INVALID_HANDLE_VALUE)
-            {
-                msg(D_TUNTAP_INFO, "CreateFile failed on %s device: %s", tt->wintun ? "wintun" : "TAP", tuntap_device_path);
-            }
-            else
+            if (tun_try_open_device(tt, *device_guid, device_instance_id_interface))
             {
                 break;
             }
@@ -6261,7 +6287,7 @@  tun_open_device(struct tuntap *tt, const char *dev_node, const char **device_gui
      * GUID using the registry */
     tt->actual_name = string_alloc(actual_buffer, NULL);
 
-    msg(M_INFO, "%s device [%s] opened: %s", tt->wintun ? "Wintun" : "TAP-WIN32", tt->actual_name, path);
+    msg(M_INFO, "%s device [%s] opened", tt->wintun ? "Wintun" : "TAP-WIN32", tt->actual_name);
     tt->adapter_index = get_adapter_index(*device_guid);
 
     gc_free(&gc);
@@ -6372,14 +6398,7 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
 
     tun_open_device(tt, dev_node, &device_guid);
 
-    if (tt->wintun)
-    {
-        if (!wintun_register_ring_buffer(tt))
-        {
-            msg(M_FATAL, "Failed to register ring buffers");
-        }
-    }
-    else
+    if (!tt->wintun)
     {
         tuntap_post_open(tt, device_guid);
     }