Re: [PATCH 2/4] ntoskrnl.exe: Implement loading plug and play devices
On 29.08.2016 20:49, Aric Stewart wrote:
Signed-off-by: Aric Stewart <aric(a)codeweavers.com> --- dlls/ntoskrnl.exe/pnp_manager.c | 150 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+)
0002-ntoskrnl.exe-Implement-loading-plug-and-play-devices.txt
diff --git a/dlls/ntoskrnl.exe/pnp_manager.c b/dlls/ntoskrnl.exe/pnp_manager.c index 5b42a0a..f1f3109 100644 --- a/dlls/ntoskrnl.exe/pnp_manager.c +++ b/dlls/ntoskrnl.exe/pnp_manager.c @@ -17,6 +17,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */
+#define NONAMELESSUNION #include <stdarg.h> #include "ntstatus.h" #define WIN32_NO_STATUS @@ -27,6 +28,7 @@ #include "cfgmgr32.h" #include "winsvc.h" #include "winreg.h" +#include "wine/unicode.h" #include "wine/debug.h" #include "wine/list.h"
@@ -34,6 +36,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(plugplay);
#define MAX_SERVICE_NAME 260
+typedef NTSTATUS WINAPI (*pAddDevice)(DRIVER_OBJECT *DriverObject, DEVICE_OBJECT *PhysicalDeviceObject);
My Win10 header files contain DRIVER_EXTENSION with the appropriate definition, which would make the casts unnecessary.
+ typedef struct _device_driver { struct list entry;
@@ -105,6 +109,143 @@ static device_driver* register_device_driver(const WCHAR *drivername, const WCHA return ptr; }
+static NTSTATUS WINAPI internalComplete(DEVICE_OBJECT *deviceObject, IRP *irp, + void *context ) +{ + SetEvent(irp->UserEvent); + return STATUS_MORE_PROCESSING_REQUIRED; +} + +static NTSTATUS get_device_id(DEVICE_OBJECT *device, BUS_QUERY_ID_TYPE type, WCHAR **id) +{ + NTSTATUS status; + IO_STACK_LOCATION *irpsp; + IO_STATUS_BLOCK irp_status; + IRP *irp; + HANDLE event = CreateEventA(NULL, FALSE, FALSE, NULL); + + irp = IoBuildSynchronousFsdRequest(IRP_MJ_PNP, device, NULL, 0, NULL, NULL, &irp_status); + if (irp == NULL) + return STATUS_NO_MEMORY;
The return path here will leak the event handle. The same issue was recently fixed in hidclass.sys. Also I'm wondering a bit, do we need to keep this identical code at both places?
+ + irp->UserEvent = event; + irpsp = IoGetNextIrpStackLocation(irp); + irpsp->MinorFunction = IRP_MN_QUERY_ID; + irpsp->Parameters.QueryId.IdType = type; + irpsp->CompletionRoutine = internalComplete; + irpsp->Control = SL_INVOKE_ON_SUCCESS | SL_INVOKE_ON_ERROR; + + IoCallDriver(device, irp); + + if (irp->IoStatus.u.Status == STATUS_PENDING) + WaitForSingleObject(event, INFINITE); + + *id = (WCHAR*)irp->IoStatus.Information; + status = irp->IoStatus.u.Status; + IoCompleteRequest(irp, IO_NO_INCREMENT ); + CloseHandle(event); + + return status; +} + +static void handle_bus_relations(DEVICE_OBJECT *device) +{ + static const WCHAR szDriverW[] = {'\\','D','r','i','v','e','r','\\',0}; + + NTSTATUS status; + WCHAR *id, *idptr; + device_driver *driver = NULL; + WCHAR fullname[MAX_PATH]; + DRIVER_OBJECT *driver_obj; + UNICODE_STRING driverName; + + TRACE("Device %p\n", device); + + /* We could(should?) do a full IRP_MN_QUERY_DEVICE_RELATIONS query, + * but we dont have to, We have the DEVICE_OBJECT of the new device + * so we can simply handle the process here */ + + status = get_device_id(device, BusQueryCompatibleIDs, &id); + if (status != ERROR_SUCCESS || !id) + { + ERR("Failed to get device IDs\n"); + return; + } + + idptr = id; + do { + device_driver *ptr; + + TRACE("Checking for id %s\n",debugstr_w(idptr)); + + /* Check loaded drivers */ + LIST_FOR_EACH_ENTRY(ptr, &device_drivers, device_driver, entry) + { + int i; + for (i = 0; i < ptr->id_count; i++) + { + if (lstrcmpW(idptr, ptr->matching_ids[i]) == 0) + { + driver = ptr; + break;
Is it intentional that this does not abort the outer loop?
+ } + } + } + idptr += (lstrlenW(idptr) + 1); + } while (!driver && *idptr != 0); + + HeapFree(GetProcessHeap(), 0, id); + + if (!driver) + { + ERR("No matching driver found for device\n"); + return; + } + + strcpyW(fullname, szDriverW); + strcatW(fullname, driver->driver_name); + RtlInitUnicodeString(&driverName, fullname); + + if (!driver->loaded) + { + static const WCHAR servicesW[] = {'\\','R','e','g','i','s','t','r','y', + '\\','M','a','c','h','i','n','e', + '\\','S','y','s','t','e','m', + '\\','C','u','r','r','e','n','t','C','o','n','t','r','o','l','S','e','t', + '\\','S','e','r','v','i','c','e','s', + '\\',0}; + WCHAR driverKey[MAX_PATH]; + UNICODE_STRING driverPath; + + strcpyW(driverKey, servicesW); + strcatW(driverKey, driver->driver_name); + RtlInitUnicodeString(&driverPath, driverKey); + if (ZwLoadDriver(&driverPath) != STATUS_SUCCESS) + { + ERR("Failed to load driver %s\n",debugstr_w(driver->driver_name)); + return; + } + driver->loaded = TRUE; + } + + if (ObReferenceObjectByName(&driverName, OBJ_CASE_INSENSITIVE, NULL, + 0, NULL, KernelMode, NULL, (void**)&driver_obj) != STATUS_SUCCESS) + { + ERR("Failed to locate loaded driver\n"); + return; + } + + status = ((pAddDevice)driver_obj->DriverExtension->AddDevice)(driver_obj, device);
It would be better to handle the lack of a AddDevice routine.
+ + ObDereferenceObject(driver_obj); + + if (status != STATUS_SUCCESS) + { + ERR("AddDevice failed\n"); + return; + } +} + static void initialize_driver_store(void) { static const WCHAR criticalW[] = @@ -181,5 +322,14 @@ VOID WINAPI IoInvalidateDeviceRelations(DEVICE_OBJECT *DeviceObject, DEVICE_RELA initialized = TRUE; initialize_driver_store(); } + + switch (Type) + { + case BusRelations: + handle_bus_relations(DeviceObject); + break; + default: + FIXME("Unhandled Relation %i\n", Type); + } LeaveCriticalSection(&pnp_cs); }
participants (1)
-
Sebastian Lackner