[PATCH v24 0/3] MR6315: plugplay: Rework how PnP notifications are sent over RPC to support DBT_DEVTYP_HANDLE events.

List overview All Threads

newer

older

[PATCH v27 0/3] MR6315: plugplay:...

[PATCH v23 0/3] MR6315: plugplay:...

Vibhav Pant (＠vibhavp)

21 Aug 2024 21 Aug '24

9:45 p.m.

PnP device notifications/events are currently sent and received from the `plugplay` service as the raw-bytes for the `DBT_DEVTYP_DEVICEINTERFACE` struct that needs to be sent from the PnP code in `ntoskrnl.exe`. While simple, it does not support sending `DBT_DEVTYP_HANDLE` events, as they also include a `HANDLE` to the device the event comes from, which cannot simply be sent as raw bytes. This MR reworks how notifications are sent and received from the `plugplay` service, by:

1. Introducing new structs `DEVICE_BROADCAST_HANDLE` and `DEVICE_BROADCAST_DEVICEINTERFACE` in `plugplay.idl`. The structs are similar to their counterparts in `dbt.h` without the `devicetype` and `size` fields added for the header. 2. Re-write `plugplay_{send,get}_event` to use the newly added union `DEVICE_BROADCAST`, which uses the `devicetype` value as the discriminant. A deep copy of this union is then appended to the event queue of every registered listener. 3. Re-write `I_ScRegisterDeviceNotification` and `device_notify_proc` in `sechost/service.c` to use the newly added types instead. Additionally, add support for registering for `DBT_DEVTYPE_HANDLE` notifications, for which the `OBJECT_NAME_INFORMATION` value for the device `HANDLE` is used to filter notifications in order to correctly dispatch them to their registered callbacks. 4. Re-write the callbacks in `user32/input.c` to accept the `DEVICE_BROADCAST` union instead, and construct the appropriate `DEV_BROADCAST_*` struct from it, which gets sent to `SendMessageTimeoutW` as usual.

Support for `DBT_DEVTYP_HANDLE` is needed for the Bluetooth driver stack I have been working on recently, which uses `DBT_DEVTYP_HANDLE` events to notify userspace about newly found Bluetooth devices during discovery and incoming authentication requests, as documented by MS [here](https://learn.microsoft.com/en-us/windows/win32/bluetooth/bluetooth-and-wm-d...).

-- v24: plugplay: Broadcast PnP events on a dedicated thread. plugplay: Rewrite RPC interface to send and receive events using the newly introduced DEVICE_BROADCAST types.

https://gitlab.winehq.org/wine/wine/-/merge_requests/6315

Show replies by date

Vibhav Pant

21 Aug 21 Aug

9:45 p.m.

New subject: [PATCH v24 1/3] include/wine/plugplay: Separate device broadcast event types into their own structs.

From: Vibhav Pant vibhavp@gmail.com

Instead of sending and receiving events from plugplay as single array of bytes, introduce a new union, DEVICE_BROADCAST. The union currently supports the types DEVICE_BROADCAST_HANDLE and DEVICE_BROADCAST_DEVICEINTERFACE for the DBT_DEVTYP_HANDLE and DBT_DEVTYP_DEVICEINTERFACE event codes respectively, and define individual fields corresponding to their equivalent structs in dbt.h.

While this introduces some additional complexity while converting from a DEVICE_BROADCAST to a DEV_BROADCAST_* type, it also allows support for DBT_DEVTYP_HANDLE events to be sent, which was not supported with the previous raw-bytes method, as the DEV_BROADCAST_HANDLE struct was identified with a HANDLE to the originating device file. --- include/wine/plugplay.idl | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)

diff --git a/include/wine/plugplay.idl b/include/wine/plugplay.idl index a3e7b04bf30..777f55db5ba 100644 --- a/include/wine/plugplay.idl +++ b/include/wine/plugplay.idl @@ -1,5 +1,6 @@ /* * Copyright (C) 2020 Zebediah Figura for CodeWeavers + * Copyright (C) 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 @@ -16,8 +17,33 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */

+#pragma makedep header + import "wtypes.idl";

+struct device_broadcast_deviceinterface +{ + GUID class_guid; + [string] LPWSTR name; +}; + +struct device_broadcast_handle +{ + LPWSTR handle_file_path; + GUID event_guid; + [string] LPWSTR name; + DWORD data_size; + [unique, size_is(data_size)] BYTE *data; +}; + +union device_broadcast switch (DWORD devicetype) event +{ + /* DBT_DEVTYP_DEVICEINTERFACE */ + case 5: struct device_broadcast_deviceinterface device_interface; + /* BROADCAST_DEVICE_TYPE_HANDLE */ + case 6: struct device_broadcast_handle handle; +}; + [ uuid(57c680ac-7bce-4f39-97fd-ffea566754d5), endpoint("ncacn_np:[\pipe\wine_plugplay]"),

-- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6315

Vibhav Pant

9:45 p.m.

New subject: [PATCH v24 2/3] plugplay: Rewrite RPC interface to send and receive events using the newly introduced DEVICE_BROADCAST types.

From: Vibhav Pant vibhavp@gmail.com

--- dlls/ntoskrnl.exe/pnp.c | 26 +++--- dlls/sechost/service.c | 111 +++++++++++++++++-------- dlls/user32/input.c | 167 ++++++++++++++++++++++++++++---------- include/Makefile.in | 1 + include/wine/dbt.h | 76 +++++++++++++++++ include/wine/plugplay.idl | 4 +- programs/plugplay/main.c | 110 ++++++++++++++++++++----- 7 files changed, 386 insertions(+), 109 deletions(-) create mode 100644 include/wine/dbt.h

diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 7444b81823c..66374f6e263 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -695,11 +695,11 @@ static LONG WINAPI rpc_filter( EXCEPTION_POINTERS *eptr ) return I_RpcExceptionFilter( eptr->ExceptionRecord->ExceptionCode ); }

-static void send_devicechange( DWORD code, void *data, unsigned int size ) +static void send_devicechange( DWORD code, struct device_broadcast *data ) { __TRY { - plugplay_send_event( code, data, size ); + plugplay_send_event( code, data ); } __EXCEPT(rpc_filter) { @@ -722,7 +722,7 @@ NTSTATUS WINAPI IoSetDeviceInterfaceState( UNICODE_STRING *name, BOOLEAN enable static const WCHAR hashW[] = {'#',0};

size_t namelen = name->Length / sizeof(WCHAR); - DEV_BROADCAST_DEVICEINTERFACE_W *broadcast; + struct device_broadcast broadcast = {0}; struct device_interface *iface; HANDLE iface_key, control_key; OBJECT_ATTRIBUTES attr = {0}; @@ -805,18 +805,16 @@ NTSTATUS WINAPI IoSetDeviceInterfaceState( UNICODE_STRING *name, BOOLEAN enable

iface->enabled = enable;

- len = offsetof(DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_name[namelen + 1]); - - if ((broadcast = heap_alloc( len ))) + broadcast.devicetype = DBT_DEVTYP_DEVICEINTERFACE; + broadcast.event.device_interface.name = heap_alloc( name->Length + 1 ); + if ( broadcast.event.device_interface.name ) { - broadcast->dbcc_size = len; - broadcast->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE; - broadcast->dbcc_reserved = 0; - broadcast->dbcc_classguid = iface->interface_class; - lstrcpynW( broadcast->dbcc_name, name->Buffer, namelen + 1 ); - if (namelen > 1) broadcast->dbcc_name[1] = '\'; - send_devicechange( enable ? DBT_DEVICEARRIVAL : DBT_DEVICEREMOVECOMPLETE, broadcast, len ); - heap_free( broadcast ); + broadcast.event.device_interface.class_guid = iface->interface_class; + lstrcpyW( broadcast.event.device_interface.name, name->Buffer ); + if ( namelen > 1 ) broadcast.event.device_interface.name[1] = '\'; + send_devicechange( enable ? DBT_DEVICEARRIVAL : DBT_DEVICEREMOVECOMPLETE, + &broadcast ); + heap_free( broadcast.event.device_interface.name ); } return ret; } diff --git a/dlls/sechost/service.c b/dlls/sechost/service.c index 915b0b4ebe0..374bc30d546 100644 --- a/dlls/sechost/service.c +++ b/dlls/sechost/service.c @@ -36,6 +36,8 @@ #include "svcctl.h" #include "plugplay.h"

+#include "wine/dbt.h" + WINE_DEFAULT_DEBUG_CHANNEL(service);

struct notify_data @@ -1973,17 +1975,6 @@ BOOL WINAPI DECLSPEC_HOTPATCH StartServiceCtrlDispatcherW( const SERVICE_TABLE_E return service_run_main_thread(); }

@@ -1993,23 +1984,53 @@ struct device_notify_registration struct device_notification_details details; };

-static BOOL notification_filter_matches( DEV_BROADCAST_HDR *filter, DEV_BROADCAST_HDR *event ) +static inline const char * +debugstr_device_notification_details( const struct device_notification_details *details ) +{ + if (!details) return "(null)"; + switch (details->devicetype) + { + case DBT_DEVTYP_DEVICEINTERFACE: + if (details->filter.deviceinterface.all_classes) + return wine_dbg_sprintf( "{%p %p DBT_DEVTYP_DEVICEINTERFACE {all_classes=1}}", + details->cb, details->handle ); + return wine_dbg_sprintf( "{%p %p DBT_DEVTYP_DEVICEINTERFACE {class=%s}}", details->cb, + details->handle, + debugstr_guid( &details->filter.deviceinterface.class ) ); + case DBT_DEVTYP_HANDLE: + return wine_dbg_sprintf( "{%p %p DBT_DEVTYP_HANDLE {%p %p}}", details->cb, details->handle, + details->filter.device.name_info, details->filter.device.device ); + default: + return wine_dbg_sprintf( "{%p %p (unknown %#lx)}", details->cb, details->handle, details->devicetype); + } +} + +static BOOL notification_filter_matches( struct device_notification_details *details, + const struct device_broadcast *data ) { - if (!filter->dbch_devicetype) return TRUE; - if (filter->dbch_devicetype != event->dbch_devicetype) return FALSE; + if (!details->devicetype) return TRUE; + if (details->devicetype != data->devicetype) return FALSE;

- if (filter->dbch_devicetype == DBT_DEVTYP_DEVICEINTERFACE) + if (details->devicetype == DBT_DEVTYP_DEVICEINTERFACE) { - DEV_BROADCAST_DEVICEINTERFACE_W *filter_iface = (DEV_BROADCAST_DEVICEINTERFACE_W *)filter; - DEV_BROADCAST_DEVICEINTERFACE_W *event_iface = (DEV_BROADCAST_DEVICEINTERFACE_W *)event; - if (filter_iface->dbcc_size == offsetof(DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_classguid)) return TRUE; - return IsEqualGUID( &filter_iface->dbcc_classguid, &event_iface->dbcc_classguid ); + if (details->filter.deviceinterface.all_classes) return TRUE; + return IsEqualGUID( &details->filter.deviceinterface.class, &data->event.device_interface.class_guid ); } - - FIXME( "Filter dbch_devicetype %lu not implemented\n", filter->dbch_devicetype ); - return TRUE; + else if (details->devicetype == DBT_DEVTYP_HANDLE) + { + return wcsicmp( details->filter.device.name_info->Name.Buffer, + data->event.handle.handle_file_path ); + } + FIXME( "unknown devicetype value %#lx\n", details->devicetype ); + return FALSE; }

+struct device_notification_details_with_notify +{ + HDEVNOTIFY notify_handle; + struct device_notification_details details; +}; + static DWORD WINAPI device_notify_proc( void *arg ) { WCHAR endpoint[] = L"\pipe\wine_plugplay"; @@ -2017,12 +2038,11 @@ static DWORD WINAPI device_notify_proc( void *arg ) RPC_WSTR binding_str; DWORD err = ERROR_SUCCESS; struct device_notify_registration *registration; - struct device_notification_details *details_copy; + struct device_notification_details_with_notify *details_copy; unsigned int details_copy_nelems, details_copy_size; plugplay_rpc_handle handle = NULL; DWORD code = 0; - unsigned int i, size; - BYTE *buf; + unsigned int i;

SetThreadDescription( GetCurrentThread(), L"wine_sechost_device_notify" );

@@ -2060,10 +2080,12 @@ static DWORD WINAPI device_notify_proc( void *arg )

for (;;) { - buf = NULL; + struct device_broadcast broadcast_data = { 0 }; + WCHAR *str; + __TRY { - code = plugplay_get_event( handle, &buf, &size ); + code = plugplay_get_event( handle, &broadcast_data ); err = ERROR_SUCCESS; } __EXCEPT(rpc_filter) @@ -2084,7 +2106,8 @@ static DWORD WINAPI device_notify_proc( void *arg ) EnterCriticalSection( &service_cs ); LIST_FOR_EACH_ENTRY(registration, &device_notify_list, struct device_notify_registration, entry) { - details_copy[i++] = registration->details; + details_copy[i].details = registration->details; + details_copy[i++].notify_handle = registration; details_copy_nelems++; if (i == details_copy_size) { @@ -2092,14 +2115,33 @@ static DWORD WINAPI device_notify_proc( void *arg ) details_copy = realloc( details_copy, details_copy_size * sizeof(*details_copy) ); } } - LeaveCriticalSection(&service_cs); + LeaveCriticalSection( &service_cs );

for (i = 0; i < details_copy_nelems; i++) { - if (!notification_filter_matches( &details_copy[i].filter.header, (DEV_BROADCAST_HDR *)buf )) continue; - details_copy[i].cb( details_copy[i].handle, code, (DEV_BROADCAST_HDR *)buf ); + struct device_notification_details *details = &details_copy[i].details; + HANDLE device = code == DBT_DEVTYP_HANDLE ? details->filter.device.device : NULL; + + if (!notification_filter_matches( details, &broadcast_data )) continue; + TRACE( "dispatching %s, matched by filter %s\n", + debugstr_device_broadcast( &broadcast_data ), + debugstr_device_notification_details( details ) ); + details->cb( details->handle, details_copy[i].notify_handle, code, &broadcast_data, device ); + } + switch (broadcast_data.devicetype) + { + case DBT_DEVTYP_DEVICEINTERFACE: + str = broadcast_data.event.device_interface.name; + break; + case DBT_DEVTYP_HANDLE: + str = broadcast_data.event.handle.name; + MIDL_user_free( broadcast_data.event.handle.handle_file_path ); + if (broadcast_data.event.handle.name) + MIDL_user_free( broadcast_data.event.handle.name ); + if (broadcast_data.event.handle.data_size != 0) + MIDL_user_free( broadcast_data.event.handle.data ); } - MIDL_user_free(buf); + if (str) MIDL_user_free( str ); }

__TRY @@ -2124,7 +2166,8 @@ HDEVNOTIFY WINAPI I_ScRegisterDeviceNotification( struct device_notification_det { struct device_notify_registration *registration;

- TRACE("callback %p, handle %p, filter %p, flags %#lx\n", details->cb, details->handle, filter, flags); + TRACE( "details %s, filter %p, flags %#lx\n", debugstr_device_notification_details( details ), + filter, flags );

if (!(registration = malloc( sizeof(struct device_notify_registration) ))) { @@ -2160,6 +2203,8 @@ BOOL WINAPI I_ScUnregisterDeviceNotification( HDEVNOTIFY handle ) EnterCriticalSection( &service_cs ); list_remove( &registration->entry ); LeaveCriticalSection(&service_cs); + if (registration->details.devicetype == DBT_DEVTYP_HANDLE) + free( registration->details.filter.device.name_info ); free( registration ); return TRUE; } diff --git a/dlls/user32/input.c b/dlls/user32/input.c index 108592f018f..8fead917b27 100644 --- a/dlls/user32/input.c +++ b/dlls/user32/input.c @@ -24,11 +24,16 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */

+#include "ntstatus.h" +#define WIN32_NO_STATUS #include "user_private.h" #include "dbt.h" #include "wine/server.h" #include "wine/debug.h"

+#include <wine/plugplay.h> +#include "wine/dbt.h" + WINE_DEFAULT_DEBUG_CHANNEL(win); WINE_DECLARE_DEBUG_CHANNEL(keyboard);

@@ -497,74 +502,133 @@ BOOL WINAPI UnloadKeyboardLayout( HKL layout ) return FALSE; }

- -static DWORD CALLBACK devnotify_window_callbackW(HANDLE handle, DWORD flags, DEV_BROADCAST_HDR *header) +static DWORD CALLBACK devnotify_window_callbackW( HANDLE handle, HDEVNOTIFY notify, DWORD flags, + struct device_broadcast *data, HANDLE device_handle ) { - SendMessageTimeoutW(handle, WM_DEVICECHANGE, flags, (LPARAM)header, SMTO_ABORTIFHUNG, 2000, NULL); + TRACE( "(%p, %p, %#lx, %s)\n", handle, notify, flags, debugstr_device_broadcast( data ) ); + + switch (data->devicetype) + { + case DBT_DEVTYP_DEVICEINTERFACE: + { + DEV_BROADCAST_DEVICEINTERFACE_W *iface; + size_t len = wcslen( data->event.device_interface.name ); + DWORD dbcc_size = offsetof( DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_name[len + 1] ); + + iface = calloc( 1, dbcc_size ); + if (!iface) + return 0; + wcscpy( iface->dbcc_name, data->event.device_interface.name ); + iface->dbcc_size = dbcc_size; + iface->dbcc_devicetype = data->devicetype; + iface->dbcc_classguid = data->event.device_interface.class_guid; + SendMessageTimeoutW( handle, WM_DEVICECHANGE, flags, (LPARAM)iface, SMTO_ABORTIFHUNG, 2000, + NULL ); + free( iface ); + break; + } + case DBT_DEVTYP_HANDLE: + { + DEV_BROADCAST_HANDLE *bc_handle; + DWORD data_size = data->event.handle.data_size; + DWORD name_size = data->event.handle.name ? wcslen( data->event.handle.name ) + 1 : 0; + DWORD dbch_size = offsetof( DEV_BROADCAST_HANDLE, + dbch_data[name_size + data_size] ); + bc_handle = calloc( 1, dbch_size ); + if (!bc_handle) + return 0; + bc_handle->dbch_size = dbch_size; + bc_handle->dbch_devicetype = DBT_DEVTYP_HANDLE; + bc_handle->dbch_handle = device_handle; + bc_handle->dbch_hdevnotify = notify; + bc_handle->dbch_eventguid = data->event.handle.event_guid; + bc_handle->dbch_nameoffset = name_size ? data_size : -1; + memcpy( bc_handle->dbch_data, data->event.handle.data, data_size ); + memcpy( &bc_handle->dbch_data[data_size], data->event.handle.name, name_size ); + SendMessageTimeoutW( handle, WM_DEVICECHANGE, flags, (LPARAM)bc_handle, SMTO_ABORTIFHUNG, + 2000, NULL ); + free( bc_handle ); + break; + } + default: + FIXME("unimplemented devicetype %#x\n", data->devicetype); + break; + } + return 0; }

-static DWORD CALLBACK devnotify_window_callbackA(HANDLE handle, DWORD flags, DEV_BROADCAST_HDR *header) +static DWORD CALLBACK devnotify_window_callbackA( HANDLE handle, HDEVNOTIFY notify, DWORD flags, + struct device_broadcast *data, HANDLE device_handle ) { + TRACE("(%p, %p, %#lx, %s)\n", handle, notify, flags, debugstr_device_broadcast( data )); + if (flags & 0x8000) { - switch (header->dbch_devicetype) + switch (data->devicetype) { case DBT_DEVTYP_DEVICEINTERFACE: { - const DEV_BROADCAST_DEVICEINTERFACE_W *ifaceW = (const DEV_BROADCAST_DEVICEINTERFACE_W *)header; - size_t lenW = wcslen( ifaceW->dbcc_name ); + size_t lenW = wcslen( data->event.device_interface.name ); DEV_BROADCAST_DEVICEINTERFACE_A *ifaceA; DWORD lenA;

- if (!(ifaceA = malloc( offsetof(DEV_BROADCAST_DEVICEINTERFACE_A, dbcc_name[lenW * 3 + 1]) ))) + if (!(ifaceA = calloc( 1, offsetof(DEV_BROADCAST_DEVICEINTERFACE_A, dbcc_name[lenW * 3 + 1]) ))) return 0; - lenA = WideCharToMultiByte( CP_ACP, 0, ifaceW->dbcc_name, lenW + 1, + lenA = WideCharToMultiByte( CP_ACP, 0, data->event.device_interface.name, lenW + 1, ifaceA->dbcc_name, lenW * 3 + 1, NULL, NULL );

ifaceA->dbcc_size = offsetof(DEV_BROADCAST_DEVICEINTERFACE_A, dbcc_name[lenA + 1]); - ifaceA->dbcc_devicetype = ifaceW->dbcc_devicetype; - ifaceA->dbcc_reserved = ifaceW->dbcc_reserved; - ifaceA->dbcc_classguid = ifaceW->dbcc_classguid; + ifaceA->dbcc_devicetype = data->devicetype; + ifaceA->dbcc_classguid = data->event.device_interface.class_guid; SendMessageTimeoutA( handle, WM_DEVICECHANGE, flags, (LPARAM)ifaceA, SMTO_ABORTIFHUNG, 2000, NULL ); free( ifaceA ); - return 0; + break; }

- default: - FIXME( "unimplemented W to A mapping for %#lx\n", header->dbch_devicetype ); - /* fall through */ case DBT_DEVTYP_HANDLE: - case DBT_DEVTYP_OEM: + { + DEV_BROADCAST_HANDLE *bc_handle; + size_t lenW = data->event.handle.name ? wcslen( data->event.handle.name ) + 1: 0; + DWORD data_size = data->event.handle.data_size; + DWORD lenA = 0; + + bc_handle = calloc( 1, offsetof( DEV_BROADCAST_HANDLE, dbch_data[lenW * 3 + 1 + data_size] ) ); + if (!bc_handle) + return 0; + + if (lenW) + lenA = WideCharToMultiByte( CP_ACP, 0, data->event.handle.name, lenW + 1, + (char *)&bc_handle->dbch_data[data_size], lenW * 3 + 1, + NULL, NULL ); + bc_handle->dbch_nameoffset = lenW ? data_size : -1; + bc_handle->dbch_size = offsetof( DEV_BROADCAST_HANDLE, dbch_data[lenA + 1] ); + bc_handle->dbch_devicetype = DBT_DEVTYP_HANDLE; + bc_handle->dbch_handle = device_handle; + bc_handle->dbch_hdevnotify = notify; + bc_handle->dbch_eventguid = data->event.handle.event_guid; + memcpy( bc_handle->dbch_data, data->event.handle.data, data_size ); + SendMessageTimeoutA( handle, WM_DEVICECHANGE, flags, (LPARAM)bc_handle, + SMTO_ABORTIFHUNG, 200, NULL ); + free( bc_handle ); + break; + } + default: + FIXME( "unimplemented W to A mapping for %#x\n", data->devicetype ); break; } }

- SendMessageTimeoutA( handle, WM_DEVICECHANGE, flags, (LPARAM)header, SMTO_ABORTIFHUNG, 2000, NULL ); return 0; }

-static DWORD CALLBACK devnotify_service_callback(HANDLE handle, DWORD flags, DEV_BROADCAST_HDR *header) +static DWORD CALLBACK devnotify_service_callback( HANDLE handle, HDEVNOTIFY notify, DWORD flags, + struct device_broadcast *data, HANDLE device_handle ) { FIXME("Support for service handles is not yet implemented!\n"); return 0; }

-struct device_notification_details -{ - DWORD (CALLBACK *cb)(HANDLE handle, DWORD flags, DEV_BROADCAST_HDR *header); - HANDLE handle; - union - { - DEV_BROADCAST_HDR header; - DEV_BROADCAST_DEVICEINTERFACE_W iface; - } filter; -}; - -extern HDEVNOTIFY WINAPI I_ScRegisterDeviceNotification( struct device_notification_details *details, - void *filter, DWORD flags ); -extern BOOL WINAPI I_ScUnregisterDeviceNotification( HDEVNOTIFY handle ); - /*********************************************************************** * RegisterDeviceNotificationA (USER32.@) * @@ -580,7 +644,7 @@ HDEVNOTIFY WINAPI RegisterDeviceNotificationA( HANDLE handle, void *filter, DWOR */ HDEVNOTIFY WINAPI RegisterDeviceNotificationW( HANDLE handle, void *filter, DWORD flags ) { - struct device_notification_details details; + struct device_notification_details details = {0}; DEV_BROADCAST_HDR *header = filter;

TRACE("handle %p, filter %p, flags %#lx\n", handle, filter, flags); @@ -597,21 +661,42 @@ HDEVNOTIFY WINAPI RegisterDeviceNotificationW( HANDLE handle, void *filter, DWOR return NULL; }

- if (!header) details.filter.header.dbch_devicetype = 0; + if (!header) details.devicetype = 0; else if (header->dbch_devicetype == DBT_DEVTYP_DEVICEINTERFACE) { DEV_BROADCAST_DEVICEINTERFACE_W *iface = (DEV_BROADCAST_DEVICEINTERFACE_W *)header; - details.filter.iface = *iface;

+ details.devicetype = DBT_DEVTYP_DEVICEINTERFACE; if (flags & DEVICE_NOTIFY_ALL_INTERFACE_CLASSES) - details.filter.iface.dbcc_size = offsetof( DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_classguid ); + details.filter.deviceinterface.all_classes = TRUE; else - details.filter.iface.dbcc_size = offsetof( DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_name ); + details.filter.deviceinterface.class = iface->dbcc_classguid; } else if (header->dbch_devicetype == DBT_DEVTYP_HANDLE) { - FIXME( "DBT_DEVTYP_HANDLE filter type not implemented\n" ); - details.filter.header.dbch_devicetype = 0; + OBJECT_NAME_INFORMATION *name_info; + DEV_BROADCAST_HANDLE *handle = (DEV_BROADCAST_HANDLE *)header; + HANDLE device = handle->dbch_handle; + NTSTATUS status; + DWORD size; + + details.devicetype = DBT_DEVTYP_HANDLE; + status = NtQueryObject( device, ObjectNameInformation, NULL, 0, &size ); + if (status != STATUS_INFO_LENGTH_MISMATCH) + { + SetLastError( RtlNtStatusToDosError( status ) ); + return NULL; + } + + name_info = malloc( size ); + if (!name_info) + { + SetLastError( ERROR_NOT_ENOUGH_MEMORY ); + return NULL; + } + + details.filter.device.device = device; + details.filter.device.name_info = name_info; } else { diff --git a/include/Makefile.in b/include/Makefile.in index 000214484ed..cc35c12827c 100644 --- a/include/Makefile.in +++ b/include/Makefile.in @@ -901,6 +901,7 @@ SOURCES = \ wine/asm.h \ wine/atsvc.idl \ wine/condrv.h \ + wine/dbt.h \ wine/dcetypes.idl \ wine/debug.h \ wine/dplaysp.h \ diff --git a/include/wine/dbt.h b/include/wine/dbt.h new file mode 100644 index 00000000000..e85767925a5 --- /dev/null +++ b/include/wine/dbt.h @@ -0,0 +1,76 @@ +/* + * Definitions for registering device notifications + * + * 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#ifndef __WINE_WINE_DBT_H_ +#define __WINE_WINE_DBT_H_ + +#include <wine/plugplay.h> +#include <wine/debug.h> + +struct device_notification_details +{ + DWORD (CALLBACK *cb)(HANDLE handle, HDEVNOTIFY notify, DWORD flags, struct device_broadcast *data, HANDLE device_handle); + HANDLE handle; + DWORD devicetype; + union + { + struct + { + /* Used to implement DEVICE_NOTIFY_ALL_INTERFACE_CLASSES. If true, the class field below + * should be ignored. */ + BOOL all_classes; + GUID class; + } deviceinterface; + struct + { + /* The path for the device file the notification is originating from. */ + OBJECT_NAME_INFORMATION *name_info; + /* A handle to the device file. It is passed as is to the callback. */ + HANDLE device; + } device; + } filter; +}; + +extern HDEVNOTIFY WINAPI I_ScRegisterDeviceNotification( struct device_notification_details *details, + void *filter, DWORD flags ); +extern BOOL WINAPI I_ScUnregisterDeviceNotification( HDEVNOTIFY handle ); + +static inline const char *debugstr_device_broadcast(const struct device_broadcast *data) +{ + if (!data) return "(null)"; + + switch (data->devicetype) + { + case DBT_DEVTYP_DEVICEINTERFACE: + return wine_dbg_sprintf( "{DBT_DEVTYP_DEVICEINTERFACE %s %s}", + debugstr_guid( &data->event.device_interface.class_guid ), + debugstr_w( data->event.device_interface.name ) ); + case DBT_DEVTYP_HANDLE: + return wine_dbg_sprintf( "{DBT_DEVTYP_HANDLE %s %s %s %lu %p}", + debugstr_w( data->event.handle.handle_file_path ), + debugstr_guid( &data->event.handle.event_guid ), + debugstr_w( data->event.handle.name ), + (int)data->event.handle.data_size, data->event.handle.data ); + default: + return wine_dbg_sprintf( "{type=%#x %p}\n", data->devicetype, data ); + } +} + +#endif /* __WINE_WINE_DBT_H_ */ diff --git a/include/wine/plugplay.idl b/include/wine/plugplay.idl index 777f55db5ba..47a95240c7d 100644 --- a/include/wine/plugplay.idl +++ b/include/wine/plugplay.idl @@ -54,7 +54,7 @@ interface plugplay typedef [context_handle] void *plugplay_rpc_handle;

plugplay_rpc_handle plugplay_register_listener(); - DWORD plugplay_get_event([in] plugplay_rpc_handle handle, [out, size_is(,*size)] BYTE **data, [out] unsigned int *size); void plugplay_unregister_listener([in] plugplay_rpc_handle handle); - void plugplay_send_event([in] DWORD event_code, [in, size_is(size)] const BYTE *data, [in] unsigned int size); + void plugplay_send_event([in] DWORD event_code, [in] const union device_broadcast *broadcast); + DWORD plugplay_get_event([in] plugplay_rpc_handle handle, [out] union device_broadcast *broadcast); } diff --git a/programs/plugplay/main.c b/programs/plugplay/main.c index 8426f2204a3..76c06ff3ef1 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 @@ -19,12 +20,15 @@ #define WIN32_LEAN_AND_MEAN

#include <windows.h> +#include <winternl.h> #include <dbt.h> #include "winsvc.h" #include "wine/debug.h" #include "wine/list.h" #include "plugplay.h"

+#include "wine/dbt.h" + WINE_DEFAULT_DEBUG_CHANNEL(plugplay);

static WCHAR plugplayW[] = L"PlugPlay"; @@ -91,6 +95,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; @@ -98,8 +110,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 ); @@ -108,7 +119,7 @@ plugplay_rpc_handle __cdecl plugplay_register_listener(void) return listener; }

-DWORD __cdecl plugplay_get_event( plugplay_rpc_handle handle, BYTE **data, unsigned int *size ) +DWORD __cdecl plugplay_get_event( plugplay_rpc_handle handle, struct device_broadcast *data ) { struct listener *listener = handle; struct event *event; @@ -120,14 +131,13 @@ DWORD __cdecl plugplay_get_event( plugplay_rpc_handle handle, BYTE **data, unsig while (!(entry = list_head( &listener->events ))) SleepConditionVariableCS( &listener->cv, &plugplay_cs, INFINITE );

- event = LIST_ENTRY(entry, struct event, entry); + event = LIST_ENTRY( entry, struct event, entry ); list_remove( &event->entry );

LeaveCriticalSection( &plugplay_cs );

ret = event->code; *data = event->data; - *size = event->size; free( event ); return ret; } @@ -137,34 +147,96 @@ void __cdecl plugplay_unregister_listener( plugplay_rpc_handle handle ) destroy_listener( handle ); }

-void __cdecl plugplay_send_event( DWORD code, const BYTE *data, unsigned int size ) +static struct event *new_event( DWORD event_code, const struct device_broadcast *src ) { - struct listener *listener; struct event *event;

- BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, code, (LPARAM)data ); - BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, DBT_DEVNODES_CHANGED, 0 ); + event = calloc( 1, sizeof( *event ) ); + if (!event) return NULL;

- EnterCriticalSection( &plugplay_cs ); + event->code = event_code; + event->data = *src; + if (src->devicetype == DBT_DEVTYP_DEVICEINTERFACE) + { + 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 );

- LIST_FOR_EACH_ENTRY(listener, &listener_list, struct listener, entry) + if (!name) + { + free( event ); + return NULL; + } + memcpy( name, event_iface->name, name_size ); + event->data.event.device_interface.name = name; + } + else { - if (!(event = malloc( sizeof(*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 );

- if (!(event->data = malloc( size ))) + if (!path) { free( event ); - break; + return NULL; + } + memcpy( path, event_handle->handle_file_path, path_size ); + event->data.event.handle.handle_file_path = path; + + if (event_handle->data_size != 0) + { + void *data = malloc( event_handle->data_size ); + if (!data) + { + free( path ); + free( event ); + return NULL; + } + memcpy( data, event_handle->data, event_handle->data_size ); + event->data.event.handle.data = data; + } + if (event_handle->name) + { + SIZE_T name_size = (wcslen( event_handle->name ) + 1) * sizeof(WCHAR); + WCHAR *name = malloc( name_size ); + if (!name) + { + free( path ); + if (event_handle->data_size != 0) free( event->data.event.handle.data ); + free( event ); + return NULL; + } + memcpy( name, event_handle->name, name_size ); + event->data.event.handle.name = name; + } + } + + return event; +} + +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 ); } + LIST_FOR_EACH_ENTRY(listener, &listener_list, struct listener, entry) + { + struct event *event = new_event( code, data ); + if (!event) break;

- event->code = code; - memcpy( event->data, data, size ); - event->size = size; list_add_tail( &listener->events, &event->entry ); WakeConditionVariable( &listener->cv ); } - LeaveCriticalSection( &plugplay_cs ); }

-- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6315

Vibhav Pant

9:45 p.m.

New subject: [PATCH v24 3/3] plugplay: Broadcast PnP events on a dedicated thread.

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. --- dlls/sechost/service.c | 4 +- programs/plugplay/main.c | 111 +++++++++++++++++++++++++++++++++++---- 2 files changed, 103 insertions(+), 12 deletions(-)

diff --git a/dlls/sechost/service.c b/dlls/sechost/service.c index 374bc30d546..adf0fa4404a 100644 --- a/dlls/sechost/service.c +++ b/dlls/sechost/service.c @@ -2021,8 +2021,8 @@ static BOOL notification_filter_matches( struct device_notification_details *det return wcsicmp( details->filter.device.name_info->Name.Buffer, data->event.handle.handle_file_path ); } - FIXME( "unknown devicetype value %#lx\n", details->devicetype ); - return FALSE; + FIXME( "unknown filter devicetype value %#lx\n", details->devicetype ); + return TRUE; }

struct device_notification_details_with_notify 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();

-- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6315

Marvin

10:42 p.m.

New subject: [PATCH v24 3/3] plugplay: Broadcast PnP events on a dedicated thread.

Hi,

It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=147907

Your paranoid android.

=== debian11b (64 bit WoW report) ===

dinput: joystick8.c:5762: Test failed: input 1: WaitForSingleObject returned 0x102 joystick8.c:5763: Test failed: input 1: got 0 WM_INPUT messages joystick8.c:5766: Test failed: input 1: got dwType 0 joystick8.c:5767: Test failed: input 1: got header.dwSize 0 joystick8.c:5769: Test failed: input 1: got hDevice 0000000000000000 joystick8.c:5771: Test failed: input 1: got dwSizeHid 0 joystick8.c:5772: Test failed: input 1: got dwCount 0

Vibhav Pant (＠vibhavp)

9:46 p.m.

On Tue Aug 20 22:57:48 2024 +0000, Elizabeth Figura wrote:

...

The changes to this function are not the easiest to review, since it both refactors the existing code (to use the new RPC structures) and adds handle support at the same time. It would be nice if these could be split into separate commits. There are also unnecessary changes like "code" -> "event_code".

Hm, I wasn't able to split the changes here to commits cleanly here :/. However, some of the functionality being separated into `new_event` should make the code easier to read, perhaps?

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6315#note_79530

Vibhav Pant (＠vibhavp)

9:47 p.m.

On Wed Aug 21 14:08:28 2024 +0000, Vibhav Pant wrote:

...

changed this line in [version 15 of the diff](/wine/wine/-/merge_requests/6315/diffs?diff_id=127699&start_sha=200a214bfb66f40ec311b78086abe9c3b95ba36a#721aad1030e6ad6dff29b13561e0a024bc8a2d3d_199_197)

Ah, apologies. clang-format is being a little uncooperative

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6315#note_79531

169

Age (days ago)

169

Last active (days ago)

wine-gitlab@winehq.org

6 comments

3 participants

tags (0)

participants (3)

Marvin
Vibhav Pant
Vibhav Pant (＠vibhavp)