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 | 92 +++++++++++++++++++++++++++++++++------- 1 file changed, 76 insertions(+), 16 deletions(-)
diff --git a/programs/plugplay/main.c b/programs/plugplay/main.c index 2c7baaaad6c..c84b967c284 100644 --- a/programs/plugplay/main.c +++ b/programs/plugplay/main.c @@ -64,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; @@ -89,6 +95,47 @@ static void listener_free_events( struct list *events ) } }
+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( BSF_FORCEIFHUNG | BSF_QUERY, NULL, WM_DEVICECHANGE, 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 ) { @@ -187,9 +234,6 @@ static struct event *new_event( DWORD event_code, const struct device_broadcast void __cdecl plugplay_send_event( DWORD code, const struct device_broadcast *data ) { struct listener *listener; - DEV_BROADCAST_DEVICEINTERFACE_W *iface; - SIZE_T name_len; - DWORD dbcc_size;
TRACE( "(%#lx, %s)\n", code, debugstr_device_broadcast( data ) );
@@ -199,19 +243,18 @@ void __cdecl plugplay_send_event( DWORD code, const struct device_broadcast *dat return; }
- - name_len = wcslen( data->event.device_interface.name ) + 1; - dbcc_size = offsetof( DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_name[name_len] ); - iface = calloc( 1, dbcc_size ); - if (!iface) return; - iface->dbcc_size = dbcc_size; - iface->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE; - iface->dbcc_classguid = data->event.device_interface.class_guid; - memcpy( iface->dbcc_name, data->event.device_interface.name, name_len ); - BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, code, (LPARAM)iface ); - BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, DBT_DEVNODES_CHANGED, 0 ); - 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) { @@ -222,7 +265,6 @@ void __cdecl plugplay_send_event( DWORD code, const struct device_broadcast *dat WakeConditionVariable( &listener->cv ); } LeaveCriticalSection( &plugplay_cs ); - free( iface ); }
static DWORD WINAPI service_handler( DWORD ctrl, DWORD event_type, LPVOID event_data, LPVOID context ) @@ -260,6 +302,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" );
@@ -278,9 +321,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; @@ -296,6 +348,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();