Currently when more than one tap is created on Windows, QEMU immediately crashes with a null-deref since the code incorrectly uses a static global for the tap state.
Instead, this patch allocates a structure for each tap at startup. Signed-off-by: Gal Horowitz <[email protected]> --- Changes in v2: - Add cleanup of the structure fields - Terminate the thread before freeing the structure - Link to v1: https://lore.kernel.org/qemu-devel/[email protected] --- net/tap-win32.c | 44 +++++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/net/tap-win32.c b/net/tap-win32.c index 38baf90e0b3f121f74eb32f1bff779c84ce03114..1b83423191c5a6f248c771d5eb5582cc80e8abcb 100644 --- a/net/tap-win32.c +++ b/net/tap-win32.c @@ -104,6 +104,7 @@ typedef struct tap_win32_overlapped { HANDLE output_queue_semaphore; HANDLE free_list_semaphore; HANDLE tap_semaphore; + HANDLE thread_handle; CRITICAL_SECTION output_queue_cs; CRITICAL_SECTION free_list_cs; OVERLAPPED read_overlapped; @@ -114,8 +115,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; @@ -402,8 +401,10 @@ static int tap_win32_set_status(HANDLE handle, int status) &status, sizeof (status), &len, NULL); } -static void tap_win32_overlapped_init(tap_win32_overlapped_t* const overlapped, const HANDLE handle) +static tap_win32_overlapped_t *tap_win32_overlapped_new(const HANDLE handle) { + tap_win32_overlapped_t *overlapped = g_new0(tap_win32_overlapped_t, 1); + overlapped->handle = handle; overlapped->read_event = CreateEvent(NULL, FALSE, FALSE, NULL); @@ -454,6 +455,8 @@ static void tap_win32_overlapped_init(tap_win32_overlapped_t* const overlapped, overlapped->tap_semaphore = CreateSemaphore(NULL, 0, TUN_MAX_BUFFER_COUNT, NULL); if(!overlapped->tap_semaphore) fprintf(stderr, "error creating tap_semaphore.\n"); + + return overlapped; } static int tap_win32_write(tap_win32_overlapped_t *overlapped, @@ -604,7 +607,7 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle, unsigned long debug; } version; DWORD version_len; - DWORD idThread; + tap_win32_overlapped_t *tap_overlapped = NULL; if (preferred_name != NULL) { snprintf(name_buffer, sizeof(name_buffer), "%s", preferred_name); @@ -645,15 +648,35 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle, return -1; } - tap_win32_overlapped_init(&tap_overlapped, handle); + tap_overlapped = tap_win32_overlapped_new(handle); - *phandle = &tap_overlapped; + tap_overlapped->thread_handle = CreateThread(NULL, 0, + tap_win32_thread_entry, (LPVOID)tap_overlapped, 0, NULL); + + *phandle = tap_overlapped; - CreateThread(NULL, 0, tap_win32_thread_entry, - (LPVOID)&tap_overlapped, 0, &idThread); return 0; } +static void tap_win32_close(tap_win32_overlapped_t *overlapped) +{ + TerminateThread(overlapped->thread_handle, 0); + + CloseHandle(overlapped->tap_semaphore); + CloseHandle(overlapped->free_list_semaphore); + CloseHandle(overlapped->output_queue_semaphore); + + DeleteCriticalSection(&overlapped->free_list_cs); + DeleteCriticalSection(&overlapped->output_queue_cs); + + CloseHandle(overlapped->write_event); + CloseHandle(overlapped->read_event); + + CloseHandle(overlapped->handle); + + g_free(overlapped); +} + /********************************************/ typedef struct TAPState { @@ -667,9 +690,8 @@ static void tap_cleanup(NetClientState *nc) qemu_del_wait_object(s->handle->tap_semaphore, NULL, NULL); - /* FIXME: need to kill thread and close file handle: - tap_win32_close(s); - */ + tap_win32_close(s->handle); + s->handle = NULL; } static ssize_t tap_receive(NetClientState *nc, const uint8_t *buf, size_t size) --- base-commit: ab8008b231e758e03c87c1c483c03afdd9c02e19 change-id: 20250920-fix-win32-multiple-taps-ed16ccefbd17 Best regards, -- Gal Horowitz <[email protected]>
