From: Vibhav Pant vibhavp@gmail.com
As incoming events in plugplay_send_event are additionally broadcast asynchronously, free-ing them immediately after the call to BroadcastSystemMessageW would result in receiving threads potentially accessing invalid memory.
To prevent this, perform these broadcasts synchronously on a dedicated thread with its own listener and event queue, which is spawned during service startup. For events of type DBT_DEVTYP_DEVICEINTERFACE, plugplay_send_event will stick an extra copy of the event into the listener's event queue, which then gets broadcast synchronously (and then freed) in the "broadcast listener" thread. --- programs/plugplay/main.c | 252 ++++++++++++++++++++++++++------------- 1 file changed, 167 insertions(+), 85 deletions(-)
diff --git a/programs/plugplay/main.c b/programs/plugplay/main.c index ea1978493f8..6e2e06ad14e 100644 --- a/programs/plugplay/main.c +++ b/programs/plugplay/main.c @@ -1,5 +1,6 @@ /* * Copyright 2011 Hans Leidekker for CodeWeavers + * Copyright 2024 Vibhav Pant * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -28,7 +29,7 @@
#include "wine/dbt.h"
-WINE_DEFAULT_DEBUG_CHANNEL(plugplay); +WINE_DEFAULT_DEBUG_CHANNEL(plugplaysrv);
static WCHAR plugplayW[] = L"PlugPlay";
@@ -63,6 +64,12 @@ struct listener CONDITION_VARIABLE cv; };
+/* A separate listener used to broadcast DBT_DEVTYP_DEVICEINTERFACE events received + * in plugplay_send_event. */ +static struct listener broadcast_listener; +#define EVENT_CODE_SHUTDOWN (~0) +static HANDLE broadcast_listener_thread; + struct event { struct list entry; @@ -70,15 +77,11 @@ struct event device_broadcast data; };
-static void destroy_listener( struct listener *listener ) +static void listener_free_events( struct list *events ) { struct event *event, *next;
- EnterCriticalSection( &plugplay_cs ); - list_remove( &listener->entry ); - LeaveCriticalSection( &plugplay_cs ); - - LIST_FOR_EACH_ENTRY_SAFE(event, next, &listener->events, struct event, entry) + LIST_FOR_EACH_ENTRY_SAFE(event, next, events, struct event, entry) { switch (event->data.devicetype) { @@ -95,6 +98,59 @@ static void destroy_listener( struct listener *listener ) list_remove( &event->entry ); free( event ); } +} + +static DWORD WINAPI broadcast_listener_proc( void *arg ) +{ + while (TRUE) + { + device_broadcast broadcast = {0}; + const device_broadcast_deviceinterface *event; + DEV_BROADCAST_DEVICEINTERFACE_W *iface; + DWORD name_len; + DWORD dbcc_size; + DWORD code; + + code = plugplay_get_event( &broadcast_listener, &broadcast ); + if (code == EVENT_CODE_SHUTDOWN) break; + + event = &broadcast.event.device_interface; + name_len = wcslen( event->name ) + 1; + dbcc_size = offsetof( DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_name[name_len] ); + + iface = malloc( dbcc_size ); + if (!iface) continue; + + iface->dbcc_size = dbcc_size; + iface->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE; + iface->dbcc_reserved = event->header.reserved; + iface->dbcc_classguid = event->class_guid; + memcpy( iface->dbcc_name, event->name, name_len ); + TRACE( "sending %s\n", debugstr_device_broadcast( &broadcast ) ); + BroadcastSystemMessageW( 0, NULL, BSF_FORCEIFHUNG | BSF_QUERY, code, (LPARAM)iface ); + BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, DBT_DEVNODES_CHANGED, 0 ); + free( iface ); + free( event->name ); + } + + EnterCriticalSection(&plugplay_cs); + /* Since broacast_listener is accessed outside of listener_list (and therefore can still be + * accessed by plugplay_send_event), we need to hold plugplay_cs while freeing any remaining + * events. */ + listener_free_events( &broadcast_listener.events ); + LeaveCriticalSection( &plugplay_cs ); + + TRACE( "shutting down the broadcast listener thread\n" ); + return 0; +} + +static void destroy_listener( struct listener *listener ) +{ + EnterCriticalSection( &plugplay_cs ); + list_remove( &listener->entry ); + LeaveCriticalSection( &plugplay_cs ); + + listener_free_events( &listener->events ); free( listener ); }
@@ -103,15 +159,22 @@ void __RPC_USER plugplay_rpc_handle_rundown( plugplay_rpc_handle handle ) destroy_listener( handle ); }
+static plugplay_rpc_handle plugplay_init_listener( struct listener *listener ) +{ + list_init( &listener->events ); + InitializeConditionVariable( &listener->cv ); + + return listener; +} + plugplay_rpc_handle __cdecl plugplay_register_listener(void) { struct listener *listener;
- if (!(listener = calloc( 1, sizeof(*listener) ))) + if (!(listener = calloc( 1, sizeof(*listener)))) return NULL;
- list_init( &listener->events ); - InitializeConditionVariable( &listener->cv ); + plugplay_init_listener( listener );
EnterCriticalSection( &plugplay_cs ); list_add_tail( &listener_list, &listener->entry ); @@ -148,99 +211,99 @@ void __cdecl plugplay_unregister_listener( plugplay_rpc_handle handle ) destroy_listener( handle ); }
-void __cdecl plugplay_send_event( DWORD event_code, const device_broadcast *data ) +static struct event *new_event( DWORD event_code, const device_broadcast *src ) { - struct listener *listener; const WCHAR *name_src = NULL; - DWORD str_size = 0; + WCHAR *name_copy = NULL; + struct event *event;
- TRACE( "(%#lx, %s)\n", event_code, debugstr_device_broadcast( data ) ); + event = calloc( 1, sizeof( *event ) ); + if (!event) return NULL;
- switch(data->devicetype) + name_src = src->devicetype == DBT_DEVTYP_HANDLE ? src->event.handle.name + : src->event.device_interface.name; + if (name_src) { - case DBT_DEVTYP_HANDLE: - name_src = data->event.handle.name; - break; - case DBT_DEVTYP_DEVICEINTERFACE: - { - const device_broadcast_deviceinterface *event = &data->event.device_interface; - DWORD name_len = wcslen( event->name ) + 1; - DWORD dbcc_size = offsetof( DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_name[name_len] ); - DEV_BROADCAST_DEVICEINTERFACE_W *iface; + SIZE_T str_size = (lstrlenW(name_src) + 1) * sizeof(WCHAR); + name_copy = malloc( str_size ); + if (!name_copy) + { + free( event ); + return NULL; + } + memcpy( name_copy, name_src, str_size ); + }
- name_src = event->name; + event->code = event_code; + event->data = *src; + if (src->devicetype == DBT_DEVTYP_HANDLE) + { + const device_broadcast_handle *event_handle = &src->event.handle; + DWORD size = sizeof(WCHAR) * (lstrlenW( event_handle->handle_file_path ) + 1); + WCHAR *path = malloc( size ); + if (!path) + { + if (name_copy) free( name_copy ); + free( event ); + return NULL; + } + memcpy( path, event_handle->handle_file_path, size ); + event->data.event.handle.handle_file_path = path;
- iface = malloc( dbcc_size ); - if (!iface) break; - iface->dbcc_size = dbcc_size; - iface->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE; - iface->dbcc_reserved = event->header.reserved; - iface->dbcc_classguid = event->class_guid; - memcpy( iface->dbcc_name, event->name, name_len ); - BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, event_code, (LPARAM)iface ); - BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, DBT_DEVNODES_CHANGED, 0 ); - free( iface ); - break; + if (event_handle->data_size != 0) + { + void *data = malloc( event_handle->data_size ); + if (!data) + { + if (name_copy) free( name_copy ); + free( path ); + free( event ); + return NULL; + } + memcpy( data, event_handle->data, event_handle->data_size ); + event->data.event.handle.data = data; + } + event->data.event.handle.name = name_copy; } - default: + else /* DBT_DEVTYP_DEVICEINTERFACE */ + { + event->data.event.device_interface.name = name_copy; + } + + return event; +} + +void __cdecl plugplay_send_event( DWORD event_code, const device_broadcast *data ) +{ + struct listener *listener; + + TRACE( "(%#lx, %s)\n", event_code, debugstr_device_broadcast( data ) ); + + if (data->devicetype != DBT_DEVTYP_HANDLE && data->devicetype != DBT_DEVTYP_DEVICEINTERFACE) + { ERR("unknown devicetype value: %#x\n", data->devicetype); return; }
- str_size = name_src ? (lstrlenW(name_src) + 1) * sizeof(WCHAR) : 0;
EnterCriticalSection( &plugplay_cs ); - LIST_FOR_EACH_ENTRY(listener, &listener_list, struct listener, entry) + /* For DBT_DEVTYP_DEVICEINTERFACE events, stick an extra copy of the event into + * broadcast_listener's queue. */ + if (data->devicetype == DBT_DEVTYP_DEVICEINTERFACE) { - struct event *event = calloc( 1, sizeof(*event)); - if (!event) break; - - event->code = event_code; - - event->data = *data; - if (data->devicetype == DBT_DEVTYP_HANDLE) + struct event *event = new_event( event_code, data ); + if (event) { - DWORD size = sizeof(WCHAR) * ( lstrlenW( data->event.handle.handle_file_path ) + 1 ); - event->data.event.handle.handle_file_path = malloc( size ); - if (!event->data.event.handle.handle_file_path) - { - free( event ); - break; - } - memcpy( event->data.event.handle.handle_file_path, data->event.handle.handle_file_path, size ); - - if (data->event.handle.data_size != 0) - { - event->data.event.handle.data = malloc( data->event.handle.data_size ); - if (!event->data.event.handle.data) - { - free( event->data.event.handle.handle_file_path ); - free( event ); - break; - } - memcpy( event->data.event.handle.data, data->event.handle.data, data->event.handle.data_size); - } + list_add_tail( &broadcast_listener.events, &event->entry ); + WakeConditionVariable( &broadcast_listener.cv ); } - if (str_size != 0) - { - WCHAR *str = malloc( str_size ); - if (!str) - { - if (data->devicetype == DBT_DEVTYP_HANDLE) - { - free( event->data.event.handle.handle_file_path ); - free( event->data.event.handle.data ); - } - free( event ); - break; - } - memcpy( str, name_src, str_size ); + } + + LIST_FOR_EACH_ENTRY(listener, &listener_list, struct listener, entry) + { + struct event *event = new_event( event_code, data ); + if (!event) break;
- if (data->devicetype == DBT_DEVTYP_HANDLE) - event->data.event.handle.name = str; - else - event->data.event.device_interface.name = str; - } list_add_tail( &listener->events, &event->entry ); WakeConditionVariable( &listener->cv ); } @@ -283,6 +346,7 @@ static void WINAPI ServiceMain( DWORD argc, LPWSTR *argv ) unsigned char protseq[] = "ncacn_np"; SERVICE_STATUS status; RPC_STATUS err; + struct event *shutdown_event;
WINE_TRACE( "starting service\n" );
@@ -301,9 +365,19 @@ static void WINAPI ServiceMain( DWORD argc, LPWSTR *argv ) ERR("RpcServerListen() failed, error %lu\n", err); return; } + if (!(shutdown_event = calloc(1, sizeof(*shutdown_event)))) + { + ERR("Failed to allocate memory for shutdown event\n"); + return; + } + shutdown_event->code = EVENT_CODE_SHUTDOWN;
stop_event = CreateEventW( NULL, TRUE, FALSE, NULL );
+ plugplay_init_listener(&broadcast_listener); + broadcast_listener_thread = + CreateThread( NULL, 0, broadcast_listener_proc, NULL, 0, NULL ); + service_handle = RegisterServiceCtrlHandlerExW( plugplayW, service_handler, NULL ); if (!service_handle) return; @@ -319,6 +393,14 @@ static void WINAPI ServiceMain( DWORD argc, LPWSTR *argv )
WaitForSingleObject( stop_event, INFINITE );
+ /* First, shutdown our broadcast listener. */ + EnterCriticalSection( &plugplay_cs ); + list_add_tail( &broadcast_listener.events, &shutdown_event->entry ); + LeaveCriticalSection( &plugplay_cs ); + + WaitForSingleObject( broadcast_listener_thread, INFINITE ); + CloseHandle( broadcast_listener_thread ); + RpcMgmtStopServerListening( NULL ); RpcServerUnregisterIf( plugplay_v0_0_s_ifspec, NULL, TRUE ); RpcMgmtWaitServerListen();