[PATCH v2 0/8] MR11263: ntoskrnl.exe: Various fixes around PDO behavior.
This series of patches are prerequisites for changing how device instance IDs (and eventually container IDs) are assigned by the PnP manager. MR !8015 touched the same area, this should fix the same bug that MR intended to address. -- v2: ntoskrnl: Use cached device instance ID for DevicePropertyEnumeratorName. ntoskrnl: Cache device instance ID for PDOs. ntoskrnl: Add checks for DEVICE_OBJECTs passed into various PnP functions. https://gitlab.winehq.org/wine/wine/-/merge_requests/11263
From: Connor McAdams <cmcadams@codeweavers.com> Signed-off-by: Connor McAdams <cmcadams@codeweavers.com> --- include/ddk/wdm.h | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/include/ddk/wdm.h b/include/ddk/wdm.h index 406870d8f06..d450db17803 100644 --- a/include/ddk/wdm.h +++ b/include/ddk/wdm.h @@ -477,6 +477,26 @@ typedef struct _DEVICE_OBJECT { } DEVICE_OBJECT; typedef struct _DEVICE_OBJECT *PDEVICE_OBJECT; +struct _DEVICE_OBJECT_POWER_EXTENSION; +typedef struct _DEVOBJ_EXTENSION { + CSHORT Type; + USHORT Size; + PDEVICE_OBJECT DeviceObject; + ULONG PowerFlags; + struct _DEVICE_OBJECT_POWER_EXTENSION *Dope; + ULONG ExtensionFlags; + PVOID DeviceNode; + PDEVICE_OBJECT AttachedTo; + LONG StartIoCount; + LONG StartIoKey; + ULONG StartIoFlags; + PVPB Vpb; + PVOID DependencyNode; + PVOID InterruptContext; + LONG InterruptCount; + PVOID VerifierContext; +} DEVOBJ_EXTENSION, *PDEVOBJ_EXTENSION; + typedef struct _DEVICE_RELATIONS { ULONG Count; PDEVICE_OBJECT Objects[1]; @@ -909,7 +929,10 @@ typedef enum { DevicePropertyAddress, DevicePropertyUINumber, DevicePropertyInstallState, - DevicePropertyRemovalPolicy + DevicePropertyRemovalPolicy, + DevicePropertyResourceRequirements, + DevicePropertyAllocatedResources, + DevicePropertyContainerID, } DEVICE_REGISTRY_PROPERTY; typedef enum _DEVICE_TEXT_TYPE { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11263
From: Connor McAdams <cmcadams@codeweavers.com> Signed-off-by: Connor McAdams <cmcadams@codeweavers.com> --- dlls/ntoskrnl.exe/tests/driver_pnp.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/dlls/ntoskrnl.exe/tests/driver_pnp.c b/dlls/ntoskrnl.exe/tests/driver_pnp.c index c33a8d6a082..c87d3b58d91 100644 --- a/dlls/ntoskrnl.exe/tests/driver_pnp.c +++ b/dlls/ntoskrnl.exe/tests/driver_pnp.c @@ -693,6 +693,16 @@ static void test_device_properties( DEVICE_OBJECT *device ) const DEVPROPKEY *key = deviceprops[i].key; void *value = &deviceprops[i].value; + /* + * Io{Get,Set}DevicePropertyData() being used with a non-PDO device causes a + * BSoD on native with an error code of PNP_DETECTED_FATAL_ERROR. + */ + if (0) + { + status = IoSetDevicePropertyData( bus_fdo, key, LOCALE_NEUTRAL, 0, type, size, value ); + ok( status == STATUS_SUCCESS, "Failed to set device property, status %#lx.\n", status ); + } + status = IoSetDevicePropertyData( device, key, LOCALE_NEUTRAL, 0, type, size, value ); ok( status == STATUS_SUCCESS, "Failed to set device property, status %#lx.\n", status ); if (status == STATUS_SUCCESS) @@ -726,6 +736,17 @@ static void test_device_properties( DEVICE_OBJECT *device ) if (status == STATUS_SUCCESS) ok( kmemcmp( buf, value, size ) == 0, "Got unexpected device property value.\n" ); + /* Crashes in the same manner as IoSetDevicePropertyData(). */ + if (0) + { + req_size = 0; + stored_type = DEVPROP_TYPE_EMPTY; + memset( buf, 0, size ); + status = IoGetDevicePropertyData( bus_fdo, key, LOCALE_NEUTRAL, 0, size, buf, + &req_size, &stored_type ); + ok( status == STATUS_SUCCESS, "Failed to get device property, status %#lx.\n", + status ); + } ExFreePool( buf ); } } @@ -834,11 +855,13 @@ static void test_enumerator_name(void) status = IoGetDeviceProperty(bus_fdo, DevicePropertyEnumeratorName, sizeof(buffer), buffer, &req_size); todo_wine ok(status == STATUS_INVALID_DEVICE_REQUEST, "got unexpected status %#lx\n", status); + ok(!(bus_fdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected bus_fdo flags %#lx.\n", bus_fdo->Flags); req_size = 0; memset(buffer, 0, sizeof(buffer)); status = IoGetDeviceProperty(bus_pdo, DevicePropertyEnumeratorName, sizeof(buffer), buffer, &req_size); ok(status == STATUS_SUCCESS, "IoGetDeviceProperty failed: %#lx\n", status); + todo_wine ok(!!(bus_pdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected bus_pdo flags %#lx.\n", bus_pdo->Flags); ok(req_size == sizeof(root), "unexpected size %lu\n", req_size); if (status == STATUS_SUCCESS) ok(!wcscmp(root, buffer), "unexpected property value '%ls'\n", buffer); @@ -873,9 +896,11 @@ static void test_device_registry_key(void) status = IoOpenDeviceRegistryKey(bus_fdo, PLUGPLAY_REGKEY_DEVICE, KEY_ALL_ACCESS, &hkey); todo_wine ok(status == STATUS_INVALID_PARAMETER, "got unexpected status %#lx\n", status); + ok(!(bus_fdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected bus_fdo flags %#lx.\n", bus_fdo->Flags); status = IoOpenDeviceRegistryKey(bus_pdo, PLUGPLAY_REGKEY_DEVICE, KEY_ALL_ACCESS, &hkey); ok(status == STATUS_SUCCESS, "IoOpenDeviceRegistryKey failed: %#lx\n", status); + todo_wine ok(!!(bus_pdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected bus_pdo flags %#lx.\n", bus_pdo->Flags); if (status == STATUS_SUCCESS) { RtlInitUnicodeString(&name_str, foobar); @@ -1146,6 +1171,7 @@ static NTSTATUS WINAPI driver_add_device(DRIVER_OBJECT *driver, DEVICE_OBJECT *p { NTSTATUS ret; + todo_wine ok(!!(pdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected pdo flags %#lx.\n", pdo->Flags); if ((ret = IoCreateDevice(driver, 0, NULL, FILE_DEVICE_BUS_EXTENDER, 0, FALSE, &bus_fdo))) return ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11263
From: Connor McAdams <cmcadams@codeweavers.com> Signed-off-by: Connor McAdams <cmcadams@codeweavers.com> --- dlls/ntoskrnl.exe/tests/driver.c | 27 +++++++++++++++++++++++++++ dlls/ntoskrnl.exe/tests/driver_pnp.c | 9 +++++++++ 2 files changed, 36 insertions(+) diff --git a/dlls/ntoskrnl.exe/tests/driver.c b/dlls/ntoskrnl.exe/tests/driver.c index 030c6476a52..882af32eaa7 100644 --- a/dlls/ntoskrnl.exe/tests/driver.c +++ b/dlls/ntoskrnl.exe/tests/driver.c @@ -1855,17 +1855,30 @@ static void test_completion(void) static void test_IoAttachDeviceToDeviceStack(void) { DEVICE_OBJECT *dev1, *dev2, *dev3, *ret; + DEVOBJ_EXTENSION *ext1, *ext2, *ext3; NTSTATUS status; status = IoCreateDevice(driver_obj, 0, NULL, FILE_DEVICE_UNKNOWN, FILE_DEVICE_SECURE_OPEN, FALSE, &dev1); ok(status == STATUS_SUCCESS, "IoCreateDevice failed\n"); + todo_wine ok(!!dev1->DeviceObjectExtension, "Got NULL DeviceObjectExtension.\n"); + ext1 = dev1->DeviceObjectExtension; status = IoCreateDevice(driver_obj, 0, NULL, FILE_DEVICE_UNKNOWN, FILE_DEVICE_SECURE_OPEN, FALSE, &dev2); ok(status == STATUS_SUCCESS, "IoCreateDevice failed\n"); + todo_wine ok(!!dev2->DeviceObjectExtension, "Got NULL DeviceObjectExtension.\n"); + ext2 = dev2->DeviceObjectExtension; status = IoCreateDevice(driver_obj, 0, NULL, FILE_DEVICE_UNKNOWN, FILE_DEVICE_SECURE_OPEN, FALSE, &dev3); ok(status == STATUS_SUCCESS, "IoCreateDevice failed\n"); + todo_wine ok(!!dev3->DeviceObjectExtension, "Got NULL DeviceObjectExtension.\n"); + ext3 = dev3->DeviceObjectExtension; + if (ext1) + { + ok(!ext1->AttachedTo, "Unexpected AttachedTo %p.\n", ext1->AttachedTo); + ok(!ext2->AttachedTo, "Unexpected AttachedTo %p.\n", ext2->AttachedTo); + ok(!ext3->AttachedTo, "Unexpected AttachedTo %p.\n", ext3->AttachedTo); + } /* TODO: initialize devices properly */ dev1->Flags &= ~DO_DEVICE_INITIALIZING; @@ -1875,7 +1888,11 @@ static void test_IoAttachDeviceToDeviceStack(void) ok(ret == dev1, "IoAttachDeviceToDeviceStack returned %p, expected %p\n", ret, dev1); ok(dev1->AttachedDevice == dev2, "dev1->AttachedDevice = %p, expected %p\n", dev1->AttachedDevice, dev2); + if (ext1) + ok(!ext1->AttachedTo, "Unexpected AttachedTo %p.\n", ext1->AttachedTo); ok(!dev2->AttachedDevice, "dev2->AttachedDevice = %p\n", dev2->AttachedDevice); + if (ext2) + ok(ext2->AttachedTo == dev1, "Unexpected AttachedTo %p.\n", ext2->AttachedTo); ok(dev1->StackSize == 1, "dev1->StackSize = %d\n", dev1->StackSize); ok(dev2->StackSize == 2, "dev2->StackSize = %d\n", dev2->StackSize); @@ -1883,9 +1900,15 @@ static void test_IoAttachDeviceToDeviceStack(void) ok(ret == dev2, "IoAttachDeviceToDeviceStack returned %p, expected %p\n", ret, dev2); ok(dev1->AttachedDevice == dev2, "dev1->AttachedDevice = %p, expected %p\n", dev1->AttachedDevice, dev2); + if (ext1) + ok(!ext1->AttachedTo, "Unexpected AttachedTo %p.\n", ext1->AttachedTo); ok(dev2->AttachedDevice == dev3, "dev2->AttachedDevice = %p, expected %p\n", dev2->AttachedDevice, dev3); + if (ext2) + ok(ext2->AttachedTo == dev1, "Unexpected AttachedTo %p.\n", ext2->AttachedTo); ok(!dev3->AttachedDevice, "dev3->AttachedDevice = %p\n", dev3->AttachedDevice); + if (ext3) + ok(ext3->AttachedTo == dev2, "Unexpected AttachedTo %p.\n", ext3->AttachedTo); ok(dev1->StackSize == 1, "dev1->StackSize = %d\n", dev1->StackSize); ok(dev2->StackSize == 2, "dev2->StackSize = %d\n", dev2->StackSize); ok(dev3->StackSize == 3, "dev3->StackSize = %d\n", dev3->StackSize); @@ -1893,9 +1916,13 @@ static void test_IoAttachDeviceToDeviceStack(void) IoDetachDevice(dev1); ok(!dev1->AttachedDevice, "dev1->AttachedDevice = %p\n", dev1->AttachedDevice); ok(dev2->AttachedDevice == dev3, "dev2->AttachedDevice = %p\n", dev2->AttachedDevice); + if (ext2) + ok(!ext2->AttachedTo, "Unexpected AttachedTo %p.\n", ext2->AttachedTo); IoDetachDevice(dev2); ok(!dev2->AttachedDevice, "dev2->AttachedDevice = %p\n", dev2->AttachedDevice); + if (ext3) + ok(!ext3->AttachedTo, "Unexpected AttachedTo %p.\n", ext3->AttachedTo); ok(dev1->StackSize == 1, "dev1->StackSize = %d\n", dev1->StackSize); ok(dev2->StackSize == 2, "dev2->StackSize = %d\n", dev2->StackSize); ok(dev3->StackSize == 3, "dev3->StackSize = %d\n", dev3->StackSize); diff --git a/dlls/ntoskrnl.exe/tests/driver_pnp.c b/dlls/ntoskrnl.exe/tests/driver_pnp.c index c87d3b58d91..d166a1ae4b9 100644 --- a/dlls/ntoskrnl.exe/tests/driver_pnp.c +++ b/dlls/ntoskrnl.exe/tests/driver_pnp.c @@ -1169,12 +1169,15 @@ static NTSTATUS WINAPI driver_ioctl(DEVICE_OBJECT *device, IRP *irp) static NTSTATUS WINAPI driver_add_device(DRIVER_OBJECT *driver, DEVICE_OBJECT *pdo) { + DEVOBJ_EXTENSION *pdo_ext = pdo->DeviceObjectExtension; + DEVOBJ_EXTENSION *fdo_ext; NTSTATUS ret; todo_wine ok(!!(pdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected pdo flags %#lx.\n", pdo->Flags); if ((ret = IoCreateDevice(driver, 0, NULL, FILE_DEVICE_BUS_EXTENDER, 0, FALSE, &bus_fdo))) return ret; + fdo_ext = bus_fdo->DeviceObjectExtension; if ((ret = IoRegisterDeviceInterface(pdo, &control_class, NULL, &control_symlink))) { IoDeleteDevice(bus_fdo); @@ -1182,6 +1185,12 @@ static NTSTATUS WINAPI driver_add_device(DRIVER_OBJECT *driver, DEVICE_OBJECT *p } IoAttachDeviceToDeviceStack(bus_fdo, pdo); + ok(pdo->AttachedDevice == bus_fdo, "Unexpected AttachedDevice %p.\n", pdo->AttachedDevice); + if (pdo_ext) + ok(!pdo_ext->AttachedTo, "Unexpected AttachedTo %p.\n", pdo_ext->AttachedTo); + ok(!bus_fdo->AttachedDevice, "Unexpected AttachedDevice %p.\n", bus_fdo->AttachedDevice); + if (fdo_ext) + ok(fdo_ext->AttachedTo == pdo, "Unexpected AttachedTo %p.\n", fdo_ext->AttachedTo); bus_pdo = pdo; bus_fdo->Flags &= ~DO_DEVICE_INITIALIZING; return STATUS_SUCCESS; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11263
From: Connor McAdams <cmcadams@codeweavers.com> Signed-off-by: Connor McAdams <cmcadams@codeweavers.com> --- dlls/ntoskrnl.exe/ntoskrnl.c | 5 ++++ dlls/ntoskrnl.exe/ntoskrnl_private.h | 1 + dlls/ntoskrnl.exe/tests/driver.c | 36 ++++++++++------------------ 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index bf6554debe1..b2078e51b8b 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -1355,6 +1355,8 @@ DEVICE_OBJECT* WINAPI IoGetAttachedDevice( DEVICE_OBJECT *device ) void WINAPI IoDetachDevice( DEVICE_OBJECT *device ) { + if (device->AttachedDevice) + device->AttachedDevice->DeviceObjectExtension->AttachedTo = NULL; device->AttachedDevice = NULL; } @@ -1367,6 +1369,7 @@ PDEVICE_OBJECT WINAPI IoAttachDeviceToDeviceStack( DEVICE_OBJECT *source, TRACE( "%p, %p\n", source, target ); target = IoGetAttachedDevice( target ); target->AttachedDevice = source; + source->DeviceObjectExtension->AttachedTo = target; source->StackSize = target->StackSize + 1; return target; } @@ -1679,6 +1682,8 @@ NTSTATUS WINAPI IoCreateDevice( DRIVER_OBJECT *driver, ULONG ext_size, device->DeviceExtension = wine_device + 1; device->DeviceType = type; device->StackSize = 1; + device->DeviceObjectExtension = &wine_device->devobj_ext; + device->DeviceObjectExtension->DeviceObject = device; if (characteristics & FILE_AUTOGENERATED_DEVICE_NAME) { diff --git a/dlls/ntoskrnl.exe/ntoskrnl_private.h b/dlls/ntoskrnl.exe/ntoskrnl_private.h index 5f3fa69ecf3..760ceb24751 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl_private.h +++ b/dlls/ntoskrnl.exe/ntoskrnl_private.h @@ -119,6 +119,7 @@ static const WCHAR servicesW[] = {'\\','R','e','g','i','s','t','r','y', struct wine_device { DEVICE_OBJECT device_obj; + DEVOBJ_EXTENSION devobj_ext; DEVICE_RELATIONS *children; HKEY dyn_data_key; }; diff --git a/dlls/ntoskrnl.exe/tests/driver.c b/dlls/ntoskrnl.exe/tests/driver.c index 882af32eaa7..6cf88cdf203 100644 --- a/dlls/ntoskrnl.exe/tests/driver.c +++ b/dlls/ntoskrnl.exe/tests/driver.c @@ -1861,24 +1861,21 @@ static void test_IoAttachDeviceToDeviceStack(void) status = IoCreateDevice(driver_obj, 0, NULL, FILE_DEVICE_UNKNOWN, FILE_DEVICE_SECURE_OPEN, FALSE, &dev1); ok(status == STATUS_SUCCESS, "IoCreateDevice failed\n"); - todo_wine ok(!!dev1->DeviceObjectExtension, "Got NULL DeviceObjectExtension.\n"); + ok(!!dev1->DeviceObjectExtension, "Got NULL DeviceObjectExtension.\n"); ext1 = dev1->DeviceObjectExtension; status = IoCreateDevice(driver_obj, 0, NULL, FILE_DEVICE_UNKNOWN, FILE_DEVICE_SECURE_OPEN, FALSE, &dev2); ok(status == STATUS_SUCCESS, "IoCreateDevice failed\n"); - todo_wine ok(!!dev2->DeviceObjectExtension, "Got NULL DeviceObjectExtension.\n"); + ok(!!dev2->DeviceObjectExtension, "Got NULL DeviceObjectExtension.\n"); ext2 = dev2->DeviceObjectExtension; status = IoCreateDevice(driver_obj, 0, NULL, FILE_DEVICE_UNKNOWN, FILE_DEVICE_SECURE_OPEN, FALSE, &dev3); ok(status == STATUS_SUCCESS, "IoCreateDevice failed\n"); - todo_wine ok(!!dev3->DeviceObjectExtension, "Got NULL DeviceObjectExtension.\n"); + ok(!!dev3->DeviceObjectExtension, "Got NULL DeviceObjectExtension.\n"); ext3 = dev3->DeviceObjectExtension; - if (ext1) - { - ok(!ext1->AttachedTo, "Unexpected AttachedTo %p.\n", ext1->AttachedTo); - ok(!ext2->AttachedTo, "Unexpected AttachedTo %p.\n", ext2->AttachedTo); - ok(!ext3->AttachedTo, "Unexpected AttachedTo %p.\n", ext3->AttachedTo); - } + ok(!ext1->AttachedTo, "Unexpected AttachedTo %p.\n", ext1->AttachedTo); + ok(!ext2->AttachedTo, "Unexpected AttachedTo %p.\n", ext2->AttachedTo); + ok(!ext3->AttachedTo, "Unexpected AttachedTo %p.\n", ext3->AttachedTo); /* TODO: initialize devices properly */ dev1->Flags &= ~DO_DEVICE_INITIALIZING; @@ -1888,11 +1885,9 @@ static void test_IoAttachDeviceToDeviceStack(void) ok(ret == dev1, "IoAttachDeviceToDeviceStack returned %p, expected %p\n", ret, dev1); ok(dev1->AttachedDevice == dev2, "dev1->AttachedDevice = %p, expected %p\n", dev1->AttachedDevice, dev2); - if (ext1) - ok(!ext1->AttachedTo, "Unexpected AttachedTo %p.\n", ext1->AttachedTo); + ok(!ext1->AttachedTo, "Unexpected AttachedTo %p.\n", ext1->AttachedTo); ok(!dev2->AttachedDevice, "dev2->AttachedDevice = %p\n", dev2->AttachedDevice); - if (ext2) - ok(ext2->AttachedTo == dev1, "Unexpected AttachedTo %p.\n", ext2->AttachedTo); + ok(ext2->AttachedTo == dev1, "Unexpected AttachedTo %p.\n", ext2->AttachedTo); ok(dev1->StackSize == 1, "dev1->StackSize = %d\n", dev1->StackSize); ok(dev2->StackSize == 2, "dev2->StackSize = %d\n", dev2->StackSize); @@ -1900,15 +1895,12 @@ static void test_IoAttachDeviceToDeviceStack(void) ok(ret == dev2, "IoAttachDeviceToDeviceStack returned %p, expected %p\n", ret, dev2); ok(dev1->AttachedDevice == dev2, "dev1->AttachedDevice = %p, expected %p\n", dev1->AttachedDevice, dev2); - if (ext1) - ok(!ext1->AttachedTo, "Unexpected AttachedTo %p.\n", ext1->AttachedTo); + ok(!ext1->AttachedTo, "Unexpected AttachedTo %p.\n", ext1->AttachedTo); ok(dev2->AttachedDevice == dev3, "dev2->AttachedDevice = %p, expected %p\n", dev2->AttachedDevice, dev3); - if (ext2) - ok(ext2->AttachedTo == dev1, "Unexpected AttachedTo %p.\n", ext2->AttachedTo); + ok(ext2->AttachedTo == dev1, "Unexpected AttachedTo %p.\n", ext2->AttachedTo); ok(!dev3->AttachedDevice, "dev3->AttachedDevice = %p\n", dev3->AttachedDevice); - if (ext3) - ok(ext3->AttachedTo == dev2, "Unexpected AttachedTo %p.\n", ext3->AttachedTo); + ok(ext3->AttachedTo == dev2, "Unexpected AttachedTo %p.\n", ext3->AttachedTo); ok(dev1->StackSize == 1, "dev1->StackSize = %d\n", dev1->StackSize); ok(dev2->StackSize == 2, "dev2->StackSize = %d\n", dev2->StackSize); ok(dev3->StackSize == 3, "dev3->StackSize = %d\n", dev3->StackSize); @@ -1916,13 +1908,11 @@ static void test_IoAttachDeviceToDeviceStack(void) IoDetachDevice(dev1); ok(!dev1->AttachedDevice, "dev1->AttachedDevice = %p\n", dev1->AttachedDevice); ok(dev2->AttachedDevice == dev3, "dev2->AttachedDevice = %p\n", dev2->AttachedDevice); - if (ext2) - ok(!ext2->AttachedTo, "Unexpected AttachedTo %p.\n", ext2->AttachedTo); + ok(!ext2->AttachedTo, "Unexpected AttachedTo %p.\n", ext2->AttachedTo); IoDetachDevice(dev2); ok(!dev2->AttachedDevice, "dev2->AttachedDevice = %p\n", dev2->AttachedDevice); - if (ext3) - ok(!ext3->AttachedTo, "Unexpected AttachedTo %p.\n", ext3->AttachedTo); + ok(!ext3->AttachedTo, "Unexpected AttachedTo %p.\n", ext3->AttachedTo); ok(dev1->StackSize == 1, "dev1->StackSize = %d\n", dev1->StackSize); ok(dev2->StackSize == 2, "dev2->StackSize = %d\n", dev2->StackSize); ok(dev3->StackSize == 3, "dev3->StackSize = %d\n", dev3->StackSize); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11263
From: Connor McAdams <cmcadams@codeweavers.com> Signed-off-by: Connor McAdams <cmcadams@codeweavers.com> --- dlls/ntoskrnl.exe/pnp.c | 1 + dlls/ntoskrnl.exe/tests/driver_pnp.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 11a578a7545..316d21ef8e4 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -372,6 +372,7 @@ static void create_dyn_data_key( DEVICE_OBJECT *device ) * send IRPs to start the device. */ static void start_device( DEVICE_OBJECT *device, HDEVINFO set, SP_DEVINFO_DATA *sp_device ) { + device->Flags |= DO_BUS_ENUMERATED_DEVICE; load_function_driver( device, set, sp_device ); if (device->DriverObject) send_pnp_irp( device, IRP_MN_START_DEVICE ); diff --git a/dlls/ntoskrnl.exe/tests/driver_pnp.c b/dlls/ntoskrnl.exe/tests/driver_pnp.c index d166a1ae4b9..e3019944604 100644 --- a/dlls/ntoskrnl.exe/tests/driver_pnp.c +++ b/dlls/ntoskrnl.exe/tests/driver_pnp.c @@ -861,7 +861,7 @@ static void test_enumerator_name(void) memset(buffer, 0, sizeof(buffer)); status = IoGetDeviceProperty(bus_pdo, DevicePropertyEnumeratorName, sizeof(buffer), buffer, &req_size); ok(status == STATUS_SUCCESS, "IoGetDeviceProperty failed: %#lx\n", status); - todo_wine ok(!!(bus_pdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected bus_pdo flags %#lx.\n", bus_pdo->Flags); + ok(!!(bus_pdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected bus_pdo flags %#lx.\n", bus_pdo->Flags); ok(req_size == sizeof(root), "unexpected size %lu\n", req_size); if (status == STATUS_SUCCESS) ok(!wcscmp(root, buffer), "unexpected property value '%ls'\n", buffer); @@ -900,7 +900,7 @@ static void test_device_registry_key(void) status = IoOpenDeviceRegistryKey(bus_pdo, PLUGPLAY_REGKEY_DEVICE, KEY_ALL_ACCESS, &hkey); ok(status == STATUS_SUCCESS, "IoOpenDeviceRegistryKey failed: %#lx\n", status); - todo_wine ok(!!(bus_pdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected bus_pdo flags %#lx.\n", bus_pdo->Flags); + ok(!!(bus_pdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected bus_pdo flags %#lx.\n", bus_pdo->Flags); if (status == STATUS_SUCCESS) { RtlInitUnicodeString(&name_str, foobar); @@ -1173,7 +1173,7 @@ static NTSTATUS WINAPI driver_add_device(DRIVER_OBJECT *driver, DEVICE_OBJECT *p DEVOBJ_EXTENSION *fdo_ext; NTSTATUS ret; - todo_wine ok(!!(pdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected pdo flags %#lx.\n", pdo->Flags); + ok(!!(pdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected pdo flags %#lx.\n", pdo->Flags); if ((ret = IoCreateDevice(driver, 0, NULL, FILE_DEVICE_BUS_EXTENDER, 0, FALSE, &bus_fdo))) return ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11263
From: Connor McAdams <cmcadams@codeweavers.com> Signed-off-by: Connor McAdams <cmcadams@codeweavers.com> --- dlls/ntoskrnl.exe/pnp.c | 18 ++++++++++++++++++ dlls/ntoskrnl.exe/tests/driver_pnp.c | 4 ++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 316d21ef8e4..c2de691d982 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -669,6 +669,9 @@ NTSTATUS WINAPI IoGetDevicePropertyData( DEVICE_OBJECT *device, const DEVPROPKEY device, debugstr_propkey( property_key ), lcid, flags, size, data, required_size, property_type ); + if (!(device->Flags & DO_BUS_ENUMERATED_DEVICE)) + ERR( "Passed in non-PDO device, this would crash on native.\n" ); + if (lcid == LOCALE_SYSTEM_DEFAULT || lcid == LOCALE_USER_DEFAULT) return STATUS_INVALID_PARAMETER; if (lcid != LOCALE_NEUTRAL) FIXME( "Only LOCALE_NEUTRAL is supported\n" ); @@ -717,6 +720,12 @@ NTSTATUS WINAPI IoGetDeviceProperty( DEVICE_OBJECT *device, DEVICE_REGISTRY_PROP TRACE("device %p, property %u, length %lu, buffer %p, needed %p.\n", device, property, length, buffer, needed); + if (!(device->Flags & DO_BUS_ENUMERATED_DEVICE)) + { + WARN( "Passed in non-PDO device.\n" ); + return STATUS_INVALID_DEVICE_REQUEST; + } + switch (property) { case DevicePropertyEnumeratorName: @@ -1164,6 +1173,9 @@ NTSTATUS WINAPI IoSetDevicePropertyData( DEVICE_OBJECT *device, const DEVPROPKEY if (lcid != LOCALE_NEUTRAL) FIXME( "only LOCALE_NEUTRAL is supported\n" ); + if (!(device->Flags & DO_BUS_ENUMERATED_DEVICE)) + ERR( "Passed in non-PDO device, this would crash on native.\n" ); + if ((status = get_device_instance_id( device, device_instance_id ))) return status; if ((set = SetupDiCreateDeviceInfoList( &GUID_NULL, NULL )) == INVALID_HANDLE_VALUE) @@ -1344,6 +1356,12 @@ NTSTATUS WINAPI IoOpenDeviceRegistryKey( DEVICE_OBJECT *device, ULONG type, ACCE TRACE("device %p, type %#lx, access %#lx, key %p.\n", device, type, access, key); + if (!(device->Flags & DO_BUS_ENUMERATED_DEVICE)) + { + WARN( "Passed in non-PDO device.\n" ); + return STATUS_INVALID_PARAMETER; + } + if ((status = get_device_instance_id( device, device_instance_id ))) { ERR("Failed to get device instance ID, error %#lx.\n", status); diff --git a/dlls/ntoskrnl.exe/tests/driver_pnp.c b/dlls/ntoskrnl.exe/tests/driver_pnp.c index e3019944604..70295bdc09b 100644 --- a/dlls/ntoskrnl.exe/tests/driver_pnp.c +++ b/dlls/ntoskrnl.exe/tests/driver_pnp.c @@ -854,7 +854,7 @@ static void test_enumerator_name(void) NTSTATUS status; status = IoGetDeviceProperty(bus_fdo, DevicePropertyEnumeratorName, sizeof(buffer), buffer, &req_size); - todo_wine ok(status == STATUS_INVALID_DEVICE_REQUEST, "got unexpected status %#lx\n", status); + ok(status == STATUS_INVALID_DEVICE_REQUEST, "got unexpected status %#lx\n", status); ok(!(bus_fdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected bus_fdo flags %#lx.\n", bus_fdo->Flags); req_size = 0; @@ -895,7 +895,7 @@ static void test_device_registry_key(void) DWORD size; status = IoOpenDeviceRegistryKey(bus_fdo, PLUGPLAY_REGKEY_DEVICE, KEY_ALL_ACCESS, &hkey); - todo_wine ok(status == STATUS_INVALID_PARAMETER, "got unexpected status %#lx\n", status); + ok(status == STATUS_INVALID_PARAMETER, "got unexpected status %#lx\n", status); ok(!(bus_fdo->Flags & DO_BUS_ENUMERATED_DEVICE), "Unexpected bus_fdo flags %#lx.\n", bus_fdo->Flags); status = IoOpenDeviceRegistryKey(bus_pdo, PLUGPLAY_REGKEY_DEVICE, KEY_ALL_ACCESS, &hkey); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11263
From: Connor McAdams <cmcadams@codeweavers.com> Signed-off-by: Connor McAdams <cmcadams@codeweavers.com> --- dlls/ntoskrnl.exe/ntoskrnl_private.h | 5 ++++ dlls/ntoskrnl.exe/pnp.c | 45 ++++++++++++++++++---------- dlls/ntoskrnl.exe/tests/driver_pnp.c | 10 +++---- 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl_private.h b/dlls/ntoskrnl.exe/ntoskrnl_private.h index 760ceb24751..79a124671b1 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl_private.h +++ b/dlls/ntoskrnl.exe/ntoskrnl_private.h @@ -29,8 +29,10 @@ #include "winbase.h" #include "winsvc.h" #include "winternl.h" +#include "winreg.h" #include "ddk/ntifs.h" #include "ddk/wdm.h" +#include "cfgmgr32.h" #include "wine/asm.h" #include "wine/debug.h" @@ -122,5 +124,8 @@ struct wine_device DEVOBJ_EXTENSION devobj_ext; DEVICE_RELATIONS *children; HKEY dyn_data_key; + + /* Combination of device_id and instance_id. Only set on PDO devices. */ + WCHAR device_instance_id[MAX_DEVICE_ID_LEN]; }; #endif diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index c2de691d982..4aa2b837c22 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -149,29 +149,43 @@ static NTSTATUS send_pnp_irp( DEVICE_OBJECT *device, UCHAR minor ) static NTSTATUS get_device_instance_id( DEVICE_OBJECT *device, WCHAR *buffer ) { - static const WCHAR backslashW[] = {'\\',0}; + struct wine_device *pdo_dev; NTSTATUS status; WCHAR *id; - if ((status = get_device_id( device, BusQueryDeviceID, &id ))) + while (device->DeviceObjectExtension->AttachedTo) + device = device->DeviceObjectExtension->AttachedTo; + + if (!(device->Flags & DO_BUS_ENUMERATED_DEVICE)) { - ERR("Failed to get device ID, status %#lx.\n", status); - return status; + ERR( "Lowest device in stack is not a PDO.\n" ); + return STATUS_INVALID_DEVICE_REQUEST; } - lstrcpyW( buffer, id ); - ExFreePool( id ); - - if ((status = get_device_id( device, BusQueryInstanceID, &id ))) + pdo_dev = CONTAINING_RECORD(device, struct wine_device, device_obj); + if (!wcslen( pdo_dev->device_instance_id )) { - ERR("Failed to get instance ID, status %#lx.\n", status); - return status; - } + if ((status = get_device_id( device, BusQueryDeviceID, &id ))) + { + ERR("Failed to get device ID, status %#lx.\n", status); + return status; + } - lstrcatW( buffer, backslashW ); - lstrcatW( buffer, id ); - ExFreePool( id ); + wcscpy( pdo_dev->device_instance_id, id ); + wcscat( pdo_dev->device_instance_id, L"\\" ); + ExFreePool( id ); + if ((status = get_device_id( device, BusQueryInstanceID, &id ))) + { + ERR("Failed to get instance ID, status %#lx.\n", status); + pdo_dev->device_instance_id[0] = 0; + return status; + } + wcscat( pdo_dev->device_instance_id, id ); + ExFreePool( id ); + } + + wcscpy( buffer, pdo_dev->device_instance_id ); TRACE("Returning ID %s.\n", debugstr_w(buffer)); return STATUS_SUCCESS; @@ -372,7 +386,6 @@ static void create_dyn_data_key( DEVICE_OBJECT *device ) * send IRPs to start the device. */ static void start_device( DEVICE_OBJECT *device, HDEVINFO set, SP_DEVINFO_DATA *sp_device ) { - device->Flags |= DO_BUS_ENUMERATED_DEVICE; load_function_driver( device, set, sp_device ); if (device->DriverObject) send_pnp_irp( device, IRP_MN_START_DEVICE ); @@ -393,6 +406,7 @@ static void enumerate_new_device( DEVICE_OBJECT *device, HDEVINFO set, DEVICE_OB HKEY key; WCHAR *id; + device->Flags |= DO_BUS_ENUMERATED_DEVICE; if (get_device_instance_id( device, device_instance_id )) return; @@ -1642,6 +1656,7 @@ void CDECL wine_enumerate_root_devices( const WCHAR *driver_name ) wcscpy( pnp_device->id, id ); pnp_device->device = device; list_add_tail( &new_list, &pnp_device->entry ); + device->Flags |= DO_BUS_ENUMERATED_DEVICE; start_device( device, set, &sp_device ); } diff --git a/dlls/ntoskrnl.exe/tests/driver_pnp.c b/dlls/ntoskrnl.exe/tests/driver_pnp.c index 70295bdc09b..cd7f30ef82d 100644 --- a/dlls/ntoskrnl.exe/tests/driver_pnp.c +++ b/dlls/ntoskrnl.exe/tests/driver_pnp.c @@ -324,7 +324,7 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device_obj, IRP *irp) query_id_count = 0; status = IoRegisterDeviceInterface(device_obj, &child_class, NULL, &device->child_symlink); - todo_wine ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); + ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); ok(!status, "Failed to register interface, status %#lx.\n", status); ok(device->child_symlink.Length == sizeof(expect_symlink) - sizeof(WCHAR), "Got length %u.\n", device->child_symlink.Length); @@ -768,12 +768,12 @@ static void test_child_device_properties(DEVICE_OBJECT *device) query_id_count = 0; status = IoSetDevicePropertyData(device, key, LOCALE_NEUTRAL, 0, type, size, &value); - todo_wine ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); + ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); ok(status == STATUS_SUCCESS, "failed to set device property, status %#lx\n", status); query_id_count = 0; status = IoGetDevicePropertyData(device, key, LOCALE_NEUTRAL, 0, size, &stored_value, &req_size, &stored_type); - todo_wine ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); + ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); ok(status == STATUS_SUCCESS, "failed to get device property, status %#lx\n", status); ok(req_size == size, "expected required size %lu, got %lu\n", req_size, size); ok(stored_type == type, "expected DEVPROPTYPE value %#lx, got %#lx\n", type, stored_type); @@ -781,7 +781,7 @@ static void test_child_device_properties(DEVICE_OBJECT *device) query_id_count = 0; status = IoSetDevicePropertyData(device, key, LOCALE_NEUTRAL, 0, type, 0, NULL); - todo_wine ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); + ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); ok(status == STATUS_SUCCESS, "failed to delete device property, status %#lx\n", status); } @@ -938,7 +938,7 @@ static void test_child_device_registry_key(DEVICE_OBJECT *device) query_id_count = 0; status = IoOpenDeviceRegistryKey(device, PLUGPLAY_REGKEY_DEVICE, KEY_ALL_ACCESS, &hkey); - todo_wine ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); + ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); ok(status == STATUS_SUCCESS, "IoOpenDeviceRegistryKey failed: %#lx\n", status); if (status == STATUS_SUCCESS) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11263
From: Connor McAdams <cmcadams@codeweavers.com> Signed-off-by: Connor McAdams <cmcadams@codeweavers.com> --- dlls/ntoskrnl.exe/pnp.c | 21 +++++++++++++-------- dlls/ntoskrnl.exe/tests/driver_pnp.c | 2 +- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 4aa2b837c22..775394c4948 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -744,25 +744,30 @@ NTSTATUS WINAPI IoGetDeviceProperty( DEVICE_OBJECT *device, DEVICE_REGISTRY_PROP { case DevicePropertyEnumeratorName: { - WCHAR *id, *ptr; + WCHAR *ptr; - status = get_device_id( device, BusQueryDeviceID, &id ); + status = get_device_instance_id( device, device_instance_id ); if (status != STATUS_SUCCESS) { ERR("Failed to get instance ID, status %#lx.\n", status); - break; + return status; } - ptr = wcschr( id, '\\' ); - if (ptr) *ptr = 0; + if (!(ptr = wcschr( device_instance_id, '\\' ))) + { + ERR( "Instance ID %s has no enumerator separator.\n", debugstr_w(device_instance_id) ); + return STATUS_UNSUCCESSFUL; + } - *needed = sizeof(WCHAR) * (lstrlenW(id) + 1); + *needed = ((ptr - device_instance_id) + 1) * sizeof(WCHAR); if (length >= *needed) - memcpy( buffer, id, *needed ); + { + memcpy( buffer, device_instance_id, *needed - sizeof(WCHAR) ); + ((WCHAR *)buffer)[((ptr - device_instance_id) + 1)] = 0; + } else status = STATUS_BUFFER_TOO_SMALL; - ExFreePool( id ); return status; } case DevicePropertyPhysicalDeviceObjectName: diff --git a/dlls/ntoskrnl.exe/tests/driver_pnp.c b/dlls/ntoskrnl.exe/tests/driver_pnp.c index cd7f30ef82d..ea7306a8900 100644 --- a/dlls/ntoskrnl.exe/tests/driver_pnp.c +++ b/dlls/ntoskrnl.exe/tests/driver_pnp.c @@ -876,7 +876,7 @@ static void test_child_enumerator_name(DEVICE_OBJECT *device) query_id_count = 0; status = IoGetDeviceProperty(device, DevicePropertyEnumeratorName, sizeof(buffer), buffer, &req_size); - todo_wine ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); + ok(query_id_count == 0, "expected no IRP_MN_QUERY_ID\n"); ok(status == STATUS_SUCCESS, "IoGetDeviceProperty failed: %#lx\n", status); ok(req_size == sizeof(wine), "unexpected size %lu\n", req_size); if (status == STATUS_SUCCESS) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11263
I have a hard time seeing how they're helpful. I'd at least make it an ERR without an early return.
Fair enough, I've kept the `ERR` but got rid of the return.
Making it a separate structure seems unnecessary to me. I would just store it directly and add a comment noting that those fields are PDO specific.
Okay, done in the current revision. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11263#note_144321
This is causing an occasional racy issue with winebth.sys, winebth.sys is causing `IRP_MN_QUERY_DEVICE_RELATIONS` to be sent prior to `IRP_MN_START_DEVICE`, which results in `get_device_instance_id()` being called prior to the children's parent device having the `DO_BUS_ENUMERATED_DEVICE` flag set. This would also cause issues with my later patches, so this will probably need to be fixed prior to merging this. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11263#note_144330
participants (2)
-
Connor McAdams -
Connor McAdams (＠cmcadams)