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 | 111 +++++++++++++++++++++++++++++++++++---- 1 file changed, 101 insertions(+), 10 deletions(-)
diff --git a/programs/plugplay/main.c b/programs/plugplay/main.c index 76c06ff3ef1..4c1097a891f 100644 --- a/programs/plugplay/main.c +++ b/programs/plugplay/main.c @@ -64,29 +64,91 @@ 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; DWORD code; - BYTE *data; - unsigned int size; + 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) { - MIDL_user_free( event->data ); + switch (event->data.devicetype) + { + case DBT_DEVTYP_HANDLE: + MIDL_user_free( event->data.event.handle.name ); + MIDL_user_free( event->data.event.handle.handle_file_path ); + MIDL_user_free( event->data.event.handle.data ); + break; + case DBT_DEVTYP_DEVICEINTERFACE: + MIDL_user_free( event->data.event.device_interface.name ); + break; + } + 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 ); }
@@ -228,7 +290,18 @@ void __cdecl plugplay_send_event( DWORD code, const struct device_broadcast *dat }
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 ); @@ -275,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" );
@@ -293,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; @@ -311,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();