The static declaration of tap_overlapped does not work when multiple
"-netdev tap" are used, tap_win32_overlapped_init will be called with
the same struct but a different handler, and the already running thread
with tap_win32_thread_entry will have the semaphores and free lists
changed under its feet.

Make tap_win32_overlapped_t a member of TAPState to be allocated with
qemu_new_net_client, and move out the tap_win32_overlapped_init and
CreateThread calls from tap_win32_open to tap_win32_init.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3314
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
I'm no longer seeing the segfaults with this patch, but I only have
connectivity when using the e1000 or virtio-net-pci devices. With
virtio-net-device (as shown in the issue with aarch64) I cannot ping
anything when two interfaces are used. I'm not sure if it's a problem
with this patch or a different problem, since it works with the other
devices.
---
 net/tap-win32.c | 47 +++++++++++++++++++++--------------------------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/net/tap-win32.c b/net/tap-win32.c
index 38baf90e0b..af20c62868 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -114,8 +114,6 @@ typedef struct tap_win32_overlapped {
     tun_buffer_t* output_queue_back;
 } tap_win32_overlapped_t;
 
-static tap_win32_overlapped_t tap_overlapped;
-
 static tun_buffer_t* get_buffer_from_free_list(tap_win32_overlapped_t* const overlapped)
 {
     tun_buffer_t* buffer = NULL;
@@ -589,8 +587,7 @@ static void tap_win32_free_buffer(tap_win32_overlapped_t *overlapped,
     put_buffer_on_free_list(overlapped, buffer);
 }
 
-static int tap_win32_open(tap_win32_overlapped_t **phandle,
-                          const char *preferred_name)
+static HANDLE tap_win32_open(const char *preferred_name)
 {
     g_autofree char *device_path = NULL;
     char device_guid[0x100];
@@ -604,7 +601,6 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
         unsigned long debug;
     } version;
     DWORD version_len;
-    DWORD idThread;
 
     if (preferred_name != NULL) {
         snprintf(name_buffer, sizeof(name_buffer), "%s", preferred_name);
@@ -612,7 +608,7 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
 
     rc = get_device_guid(device_guid, sizeof(device_guid), name_buffer, sizeof(name_buffer));
     if (rc)
-        return -1;
+        return INVALID_HANDLE_VALUE;
 
     device_path = g_strdup_printf("%s%s%s",
               USERMODEDEVICEDIR,
@@ -629,7 +625,7 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
         0 );
 
     if (handle == INVALID_HANDLE_VALUE) {
-        return -1;
+        return INVALID_HANDLE_VALUE;
     }
 
     bret = DeviceIoControl(handle, TAP_IOCTL_GET_VERSION,
@@ -638,34 +634,28 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
 
     if (bret == FALSE) {
         CloseHandle(handle);
-        return -1;
+        return INVALID_HANDLE_VALUE;
     }
 
     if (!tap_win32_set_status(handle, TRUE)) {
-        return -1;
+        return INVALID_HANDLE_VALUE;
     }
 
-    tap_win32_overlapped_init(&tap_overlapped, handle);
-
-    *phandle = &tap_overlapped;
-
-    CreateThread(NULL, 0, tap_win32_thread_entry,
-                 (LPVOID)&tap_overlapped, 0, &idThread);
-    return 0;
+    return handle;
 }
 
 /********************************************/
 
  typedef struct TAPState {
      NetClientState nc;
-     tap_win32_overlapped_t *handle;
+     tap_win32_overlapped_t tap_overlapped;
  } TAPState;
 
 static void tap_cleanup(NetClientState *nc)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
 
-    qemu_del_wait_object(s->handle->tap_semaphore, NULL, NULL);
+    qemu_del_wait_object(s->tap_overlapped.tap_semaphore, NULL, NULL);
 
     /* FIXME: need to kill thread and close file handle:
        tap_win32_close(s);
@@ -676,7 +666,7 @@ static ssize_t tap_receive(NetClientState *nc, const uint8_t *buf, size_t size)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
 
-    return tap_win32_write(s->handle, buf, size);
+    return tap_win32_write(&s->tap_overlapped, buf, size);
 }
 
 static void tap_win32_send(void *opaque)
@@ -688,7 +678,7 @@ static void tap_win32_send(void *opaque)
     uint8_t min_pkt[ETH_ZLEN];
     size_t min_pktsz = sizeof(min_pkt);
 
-    size = tap_win32_read(s->handle, &buf, max_size);
+    size = tap_win32_read(&s->tap_overlapped, &buf, max_size);
     if (size > 0) {
         orig_buf = buf;
 
@@ -700,7 +690,7 @@ static void tap_win32_send(void *opaque)
         }
 
         qemu_send_packet(&s->nc, buf, size);
-        tap_win32_free_buffer(s->handle, orig_buf);
+        tap_win32_free_buffer(&s->tap_overlapped, orig_buf);
     }
 }
 
@@ -716,9 +706,11 @@ static int tap_win32_init(NetClientState *peer, const char *model,
 {
     NetClientState *nc;
     TAPState *s;
-    tap_win32_overlapped_t *handle;
+    HANDLE handle;
+    DWORD idThread;
 
-    if (tap_win32_open(&handle, ifname) < 0) {
+    handle = tap_win32_open(ifname);
+    if (handle == INVALID_HANDLE_VALUE) {
         printf("tap: Could not open '%s'\n", ifname);
         return -1;
     }
@@ -727,11 +719,14 @@ static int tap_win32_init(NetClientState *peer, const char *model,
 
     s = DO_UPCAST(TAPState, nc, nc);
 
+    tap_win32_overlapped_init(&s->tap_overlapped, handle);
+
+    CreateThread(NULL, 0, tap_win32_thread_entry,
+                 (LPVOID)&s->tap_overlapped, 0, &idThread);
+
     qemu_set_info_str(&s->nc, "tap: ifname=%s", ifname);
 
-    s->handle = handle;
-
-    qemu_add_wait_object(s->handle->tap_semaphore, tap_win32_send, s);
+    qemu_add_wait_object(s->tap_overlapped.tap_semaphore, tap_win32_send, s);
 
     return 0;
 }
-- 
2.43.0

ping

Em sex., 13 de mar. de 2026 às 17:19, Matheus Ferst via qemu development <
qemu-devel@nongnu.org> escreveu:

> The static declaration of tap_overlapped does not work when multiple
> "-netdev tap" are used, tap_win32_overlapped_init will be called with
> the same struct but a different handler, and the already running thread
> with tap_win32_thread_entry will have the semaphores and free lists
> changed under its feet.
>
> Make tap_win32_overlapped_t a member of TAPState to be allocated with
> qemu_new_net_client, and move out the tap_win32_overlapped_init and
> CreateThread calls from tap_win32_open to tap_win32_init.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3314
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
> I'm no longer seeing the segfaults with this patch, but I only have
> connectivity when using the e1000 or virtio-net-pci devices. With
> virtio-net-device (as shown in the issue with aarch64) I cannot ping
> anything when two interfaces are used. I'm not sure if it's a problem
> with this patch or a different problem, since it works with the other
> devices.
> ---
>  net/tap-win32.c | 47 +++++++++++++++++++++--------------------------
>  1 file changed, 21 insertions(+), 26 deletions(-)
>
> diff --git a/net/tap-win32.c b/net/tap-win32.c
> index 38baf90e0b..af20c62868 100644
> --- a/net/tap-win32.c
> +++ b/net/tap-win32.c
> @@ -114,8 +114,6 @@ typedef struct tap_win32_overlapped {
>      tun_buffer_t* output_queue_back;
>  } tap_win32_overlapped_t;
>
> -static tap_win32_overlapped_t tap_overlapped;
> -
>  static tun_buffer_t* get_buffer_from_free_list(tap_win32_overlapped_t*
> const overlapped)
>  {
>      tun_buffer_t* buffer = NULL;
> @@ -589,8 +587,7 @@ static void
> tap_win32_free_buffer(tap_win32_overlapped_t *overlapped,
>      put_buffer_on_free_list(overlapped, buffer);
>  }
>
> -static int tap_win32_open(tap_win32_overlapped_t **phandle,
> -                          const char *preferred_name)
> +static HANDLE tap_win32_open(const char *preferred_name)
>  {
>      g_autofree char *device_path = NULL;
>      char device_guid[0x100];
> @@ -604,7 +601,6 @@ static int tap_win32_open(tap_win32_overlapped_t
> **phandle,
>          unsigned long debug;
>      } version;
>      DWORD version_len;
> -    DWORD idThread;
>
>      if (preferred_name != NULL) {
>          snprintf(name_buffer, sizeof(name_buffer), "%s", preferred_name);
> @@ -612,7 +608,7 @@ static int tap_win32_open(tap_win32_overlapped_t
> **phandle,
>
>      rc = get_device_guid(device_guid, sizeof(device_guid), name_buffer,
> sizeof(name_buffer));
>      if (rc)
> -        return -1;
> +        return INVALID_HANDLE_VALUE;
>
>      device_path = g_strdup_printf("%s%s%s",
>                USERMODEDEVICEDIR,
> @@ -629,7 +625,7 @@ static int tap_win32_open(tap_win32_overlapped_t
> **phandle,
>          0 );
>
>      if (handle == INVALID_HANDLE_VALUE) {
> -        return -1;
> +        return INVALID_HANDLE_VALUE;
>      }
>
>      bret = DeviceIoControl(handle, TAP_IOCTL_GET_VERSION,
> @@ -638,34 +634,28 @@ static int tap_win32_open(tap_win32_overlapped_t
> **phandle,
>
>      if (bret == FALSE) {
>          CloseHandle(handle);
> -        return -1;
> +        return INVALID_HANDLE_VALUE;
>      }
>
>      if (!tap_win32_set_status(handle, TRUE)) {
> -        return -1;
> +        return INVALID_HANDLE_VALUE;
>      }
>
> -    tap_win32_overlapped_init(&tap_overlapped, handle);
> -
> -    *phandle = &tap_overlapped;
> -
> -    CreateThread(NULL, 0, tap_win32_thread_entry,
> -                 (LPVOID)&tap_overlapped, 0, &idThread);
> -    return 0;
> +    return handle;
>  }
>
>  /********************************************/
>
>   typedef struct TAPState {
>       NetClientState nc;
> -     tap_win32_overlapped_t *handle;
> +     tap_win32_overlapped_t tap_overlapped;
>   } TAPState;
>
>  static void tap_cleanup(NetClientState *nc)
>  {
>      TAPState *s = DO_UPCAST(TAPState, nc, nc);
>
> -    qemu_del_wait_object(s->handle->tap_semaphore, NULL, NULL);
> +    qemu_del_wait_object(s->tap_overlapped.tap_semaphore, NULL, NULL);
>
>      /* FIXME: need to kill thread and close file handle:
>         tap_win32_close(s);
> @@ -676,7 +666,7 @@ static ssize_t tap_receive(NetClientState *nc, const
> uint8_t *buf, size_t size)
>  {
>      TAPState *s = DO_UPCAST(TAPState, nc, nc);
>
> -    return tap_win32_write(s->handle, buf, size);
> +    return tap_win32_write(&s->tap_overlapped, buf, size);
>  }
>
>  static void tap_win32_send(void *opaque)
> @@ -688,7 +678,7 @@ static void tap_win32_send(void *opaque)
>      uint8_t min_pkt[ETH_ZLEN];
>      size_t min_pktsz = sizeof(min_pkt);
>
> -    size = tap_win32_read(s->handle, &buf, max_size);
> +    size = tap_win32_read(&s->tap_overlapped, &buf, max_size);
>      if (size > 0) {
>          orig_buf = buf;
>
> @@ -700,7 +690,7 @@ static void tap_win32_send(void *opaque)
>          }
>
>          qemu_send_packet(&s->nc, buf, size);
> -        tap_win32_free_buffer(s->handle, orig_buf);
> +        tap_win32_free_buffer(&s->tap_overlapped, orig_buf);
>      }
>  }
>
> @@ -716,9 +706,11 @@ static int tap_win32_init(NetClientState *peer, const
> char *model,
>  {
>      NetClientState *nc;
>      TAPState *s;
> -    tap_win32_overlapped_t *handle;
> +    HANDLE handle;
> +    DWORD idThread;
>
> -    if (tap_win32_open(&handle, ifname) < 0) {
> +    handle = tap_win32_open(ifname);
> +    if (handle == INVALID_HANDLE_VALUE) {
>          printf("tap: Could not open '%s'\n", ifname);
>          return -1;
>      }
> @@ -727,11 +719,14 @@ static int tap_win32_init(NetClientState *peer,
> const char *model,
>
>      s = DO_UPCAST(TAPState, nc, nc);
>
> +    tap_win32_overlapped_init(&s->tap_overlapped, handle);
> +
> +    CreateThread(NULL, 0, tap_win32_thread_entry,
> +                 (LPVOID)&s->tap_overlapped, 0, &idThread);
> +
>      qemu_set_info_str(&s->nc, "tap: ifname=%s", ifname);
>
> -    s->handle = handle;
> -
> -    qemu_add_wait_object(s->handle->tap_semaphore, tap_win32_send, s);
> +    qemu_add_wait_object(s->tap_overlapped.tap_semaphore, tap_win32_send,
> s);
>
>      return 0;
>  }
> --
> 2.43.0
>
>
>