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 | 246 ++++++++++++++++++++++++++------------- 1 file changed, 165 insertions(+), 81 deletions(-)
diff --git a/programs/plugplay/main.c b/programs/plugplay/main.c index 37251ff6ee7..4c1097a891f 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 @@ -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 struct 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,57 @@ static void destroy_listener( struct listener *listener ) list_remove( &event->entry ); free( event ); } +} + +static DWORD WINAPI broadcast_listener_proc( void *arg ) +{ + while (TRUE) + { + struct device_broadcast broadcast = {0}; + const struct 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 = calloc( 1, dbcc_size ); + if (!iface) continue; + + iface->dbcc_size = dbcc_size; + iface->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE; + iface->dbcc_classguid = event->class_guid; + memcpy( iface->dbcc_name, event->name, name_len ); + 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,6 +157,14 @@ 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; @@ -110,8 +172,7 @@ plugplay_rpc_handle __cdecl plugplay_register_listener(void) 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,103 +209,108 @@ void __cdecl plugplay_unregister_listener( plugplay_rpc_handle handle ) destroy_listener( handle ); }
-void __cdecl plugplay_send_event( DWORD code, const struct device_broadcast *data ) +static struct event *new_event( DWORD event_code, const struct device_broadcast *src ) { - struct listener *listener; - const WCHAR *name_src = NULL; - DWORD str_size = 0; + struct event *event;
- TRACE( "(%#lx, %s)\n", code, debugstr_device_broadcast( data ) ); + event = calloc( 1, sizeof( *event ) ); + if (!event) return NULL;
- switch(data->devicetype) - { - case DBT_DEVTYP_HANDLE: - name_src = data->event.handle.name; - break; - case DBT_DEVTYP_DEVICEINTERFACE: + event->code = event_code; + event->data = *src; + if (src->devicetype == DBT_DEVTYP_DEVICEINTERFACE) { - const struct 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; - - name_src = event->name; + const struct device_broadcast_deviceinterface *event_iface = &src->event.device_interface; + SIZE_T name_size = (wcslen( event_iface->name ) + 1) * sizeof(WCHAR); + WCHAR *name = malloc( name_size );
- iface = malloc( dbcc_size ); - if (!iface) break; - iface->dbcc_size = dbcc_size; - iface->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE; - iface->dbcc_classguid = event->class_guid; - memcpy( iface->dbcc_name, event->name, name_len ); - BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, code, (LPARAM)iface ); - BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, DBT_DEVNODES_CHANGED, 0 ); - free( iface ); - break; - } - default: - ERR( "unknown devicetype value: %#x\n", data->devicetype ); - return; + if (!name) + { + free( event ); + return NULL; + } + memcpy( name, event_iface->name, name_size ); + event->data.event.device_interface.name = name; } - - str_size = name_src ? (lstrlenW(name_src) + 1) * sizeof(WCHAR) : 0; - - EnterCriticalSection( &plugplay_cs ); - LIST_FOR_EACH_ENTRY(listener, &listener_list, struct listener, entry) + else { - struct event *event = calloc( 1, sizeof(*event)); - if (!event) break; + const struct device_broadcast_handle *event_handle = &src->event.handle; + SIZE_T path_size = sizeof(WCHAR) * (wcslen( event_handle->handle_file_path ) + 1); + WCHAR *path = malloc( path_size );
- event->code = code; + if (!path) + { + free( event ); + return NULL; + } + memcpy( path, event_handle->handle_file_path, path_size ); + event->data.event.handle.handle_file_path = path;
- event->data = *data; - if (data->devicetype == DBT_DEVTYP_HANDLE) + if (event_handle->data_size != 0) { - 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) + void *data = malloc( event_handle->data_size ); + if (!data) { + free( 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); + return NULL; } + memcpy( data, event_handle->data, event_handle->data_size ); + event->data.event.handle.data = data; } - if (str_size != 0) + if (event_handle->name) { - WCHAR *str = malloc( str_size ); - if (!str) + SIZE_T name_size = (wcslen( event_handle->name ) + 1) * sizeof(WCHAR); + WCHAR *name = malloc( name_size ); + if (!name) { - if (data->devicetype == DBT_DEVTYP_HANDLE) - { - free( event->data.event.handle.handle_file_path ); - free( event->data.event.handle.data ); - } + free( path ); + if (event_handle->data_size != 0) free( event->data.event.handle.data ); free( event ); - break; + return NULL; } - memcpy( str, name_src, str_size ); + memcpy( name, event_handle->name, name_size ); + event->data.event.handle.name = name; + } + } + + return event; +}
- if (data->devicetype == DBT_DEVTYP_HANDLE) - event->data.event.handle.name = str; - else - event->data.event.device_interface.name = str; +void __cdecl plugplay_send_event( DWORD code, const struct device_broadcast *data ) +{ + struct listener *listener; + + TRACE( "(%#lx, %s)\n", code, debugstr_device_broadcast( data ) ); + + if (data->devicetype != DBT_DEVTYP_HANDLE && data->devicetype != DBT_DEVTYP_DEVICEINTERFACE) + { + ERR( "unknown devicetype value: %#lx\n", data->devicetype ); + return; + } + + EnterCriticalSection( &plugplay_cs ); + /* 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 = new_event( code, data ); + if (event) + { + list_add_tail( &broadcast_listener.events, &event->entry ); + WakeConditionVariable( &broadcast_listener.cv ); } + } + + LIST_FOR_EACH_ENTRY(listener, &listener_list, struct listener, entry) + { + struct event *event = new_event( code, data ); + if (!event) break; + list_add_tail( &listener->events, &event->entry ); WakeConditionVariable( &listener->cv ); } LeaveCriticalSection( &plugplay_cs ); - return; }
static DWORD WINAPI service_handler( DWORD ctrl, DWORD event_type, LPVOID event_data, LPVOID context ) @@ -282,6 +348,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" );
@@ -300,9 +367,18 @@ 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; @@ -318,6 +394,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();