]> xenbits.xensource.com Git - pvdrivers/win/xeniface.git/commitdiff
Fix Registry Isolation issues with Server 2025
authorOwen Smith <owen.smith@cloud.com>
Tue, 9 Jul 2024 10:41:21 +0000 (11:41 +0100)
committerPaul Durrant <pdurrant@amazon.com>
Wed, 24 Jul 2024 09:31:29 +0000 (10:31 +0100)
Use MmGetSystemRoutineAddress to dynamically get IoOpenDriverRegistryKey,
which is not present on Server 2016.
Where possible, use IoOpenDriverRegistryKey to avoid opening absolute registry
paths, which is a driver verifier violation on Server 2025 WHQL testing.

Also refactors all registry access to use functions in registry.h and cleans up
driver.c to be more inline with other drivers.

Signed-off-by: Owen Smith <owen.smith@cloud.com>
src/xeniface/driver.c
src/xeniface/driver.h
src/xeniface/fdo.c
src/xeniface/registry.c
src/xeniface/registry.h
src/xeniface/wmi.c

index 6c3032b496616c047f64d279a42a04a12c02b233..3b5b16def78271080d2fd57250d858e9d7a2f55a 100644 (file)
 
 #include "fdo.h"
 #include "driver.h"
+#include "registry.h"
 
 #include "assert.h"
 #include "wmi.h"
 #include "util.h"
 
-PDRIVER_OBJECT      DriverObject;
+typedef struct _XENIFACE_DRIVER {
+    PDRIVER_OBJECT      DriverObject;
+    HANDLE              ParametersKey;
+} XENIFACE_DRIVER, *PXENIFACE_DRIVER;
 
-DRIVER_UNLOAD       DriverUnload;
+static XENIFACE_DRIVER  Driver;
+
+static FORCEINLINE VOID
+__DriverSetDriverObject(
+    IN  PDRIVER_OBJECT  DriverObject
+    )
+{
+    Driver.DriverObject = DriverObject;
+}
+
+static FORCEINLINE PDRIVER_OBJECT
+__DriverGetDriverObject(
+    VOID
+    )
+{
+    return Driver.DriverObject;
+}
+
+PDRIVER_OBJECT
+DriverGetDriverObject(
+    VOID
+    )
+{
+    return __DriverGetDriverObject();
+}
+
+static FORCEINLINE VOID
+__DriverSetParametersKey(
+    IN  HANDLE  Key
+    )
+{
+    Driver.ParametersKey = Key;
+}
+
+static FORCEINLINE HANDLE
+__DriverGetParametersKey(
+    VOID
+    )
+{
+    return Driver.ParametersKey;
+}
+
+HANDLE
+DriverGetParametersKey(
+    VOID
+    )
+{
+    return __DriverGetParametersKey();
+}
 
-XENIFACE_PARAMETERS DriverParameters;
+DRIVER_UNLOAD       DriverUnload;
 
 VOID
 DriverUnload(
-    IN  PDRIVER_OBJECT  _DriverObject
+    IN  PDRIVER_OBJECT  DriverObject
     )
 {
-    ASSERT3P(_DriverObject, ==, DriverObject);
+    HANDLE              ParametersKey;
+
+    ASSERT3P(DriverObject, ==, __DriverGetDriverObject());
 
     Trace("====>\n");
 
-    if (DriverParameters.RegistryPath.Buffer != NULL) {
-        __FreePoolWithTag(DriverParameters.RegistryPath.Buffer,
-                          XENIFACE_POOL_TAG);
-    }
+    ParametersKey = __DriverGetParametersKey();
+    __DriverSetParametersKey(NULL);
+
+    RegistryCloseKey(ParametersKey);
+
+    RegistryTeardown();
+
+    Info("XENIFACE %d.%d.%d (%d) (%02d.%02d.%04d)\n",
+         MAJOR_VERSION,
+         MINOR_VERSION,
+         MICRO_VERSION,
+         BUILD_NUMBER,
+         DAY,
+         MONTH,
+         YEAR);
+
+    __DriverSetDriverObject(NULL);
 
-    DriverObject = NULL;
+    ASSERT(IsZeroMemory(&Driver, sizeof (XENIFACE_DRIVER)));
 
     Trace("<====\n");
 }
@@ -70,13 +137,13 @@ DRIVER_ADD_DEVICE   AddDevice;
 
 NTSTATUS
 AddDevice(
-    IN  PDRIVER_OBJECT  _DriverObject,
+    IN  PDRIVER_OBJECT  DriverObject,
     IN  PDEVICE_OBJECT  DeviceObject
     )
 {
     NTSTATUS            status;
 
-    ASSERT3P(_DriverObject, ==, DriverObject);
+    ASSERT3P(DriverObject, ==, __DriverGetDriverObject());
 
     status = FdoCreate(DeviceObject);
     if (!NT_SUCCESS(status))
@@ -147,40 +214,45 @@ done:
 
 DRIVER_INITIALIZE   DriverEntry;
 
-
 NTSTATUS
 DriverEntry(
-    IN  PDRIVER_OBJECT  _DriverObject,
+    IN  PDRIVER_OBJECT  DriverObject,
     IN  PUNICODE_STRING RegistryPath
     )
 {
+    HANDLE              ParametersKey;
     ULONG               Index;
-    NTSTATUS status = STATUS_UNSUCCESSFUL;
-    ASSERT3P(DriverObject, ==, NULL);
+    NTSTATUS            status;
+
+    ASSERT3P(__DriverGetDriverObject(), ==, NULL);
 
     ExInitializeDriverRuntime(DrvRtPoolNxOptIn);
     WdmlibProcgrpInitialize();
 
     Trace("====>\n");
 
-    Info("%s (%s)\n",
-         MAJOR_VERSION_STR "." MINOR_VERSION_STR "." MICRO_VERSION_STR "." BUILD_NUMBER_STR,
-         DAY_STR "/" MONTH_STR "/" YEAR_STR);
-
-    DriverParameters.RegistryPath.MaximumLength = RegistryPath->Length + sizeof(UNICODE_NULL);
-    DriverParameters.RegistryPath.Length = RegistryPath->Length;
-    DriverParameters.RegistryPath.Buffer = __AllocatePoolWithTag(PagedPool,
-                                                DriverParameters.RegistryPath.MaximumLength,
-                                                XENIFACE_POOL_TAG);
-    if (NULL == DriverParameters.RegistryPath.Buffer) {
-        status = STATUS_INSUFFICIENT_RESOURCES;
+    __DriverSetDriverObject(DriverObject);
+
+    DriverObject->DriverUnload = DriverUnload;
+
+    Info("XENIFACE %d.%d.%d (%d) (%02d.%02d.%04d)\n",
+         MAJOR_VERSION,
+         MINOR_VERSION,
+         MICRO_VERSION,
+         BUILD_NUMBER,
+         DAY,
+         MONTH,
+         YEAR);
+
+    status = RegistryInitialize(DriverObject, RegistryPath);
+    if (!NT_SUCCESS(status))
         goto fail1;
-    }
-    RtlCopyUnicodeString(&DriverParameters.RegistryPath, RegistryPath);
 
+    status = RegistryOpenParametersKey(KEY_READ, &ParametersKey);
+    if (!NT_SUCCESS(status))
+        goto fail2;
 
-    DriverObject = _DriverObject;
-    DriverObject->DriverUnload = DriverUnload;
+    __DriverSetParametersKey(ParametersKey);
 
     DriverObject->DriverExtension->AddDevice = AddDevice;
 
@@ -193,7 +265,19 @@ DriverEntry(
     Trace("<====\n");
 
     return STATUS_SUCCESS;
+
+
+fail2:
+    Error("fail2\n");
+
+    RegistryTeardown();
+
 fail1:
     Error("fail1 (%08x)\n", status);
+
+    __DriverSetDriverObject(NULL);
+
+    ASSERT(IsZeroMemory(&Driver, sizeof (XENIFACE_DRIVER)));
+
     return status;
 }
index 27cd453dbbd9bc9a6e34206f0a86258e4fec2cbd..9007d975480425e7687956ebba8d6674080b3d7f 100644 (file)
 
 #include <wmilib.h>
 #include <ntifs.h>
-extern PDRIVER_OBJECT   DriverObject;
-
 
 #define MAX_DEVICE_ID_LEN   200
 
-typedef struct _XENIFACE_PARAMETERS {
-    UNICODE_STRING RegistryPath;
-
-} XENIFACE_PARAMETERS, *PXENIFACE_PARAMETERS;
-
 #define XENIFACE_POOL_TAG (ULONG) 'XIfc'
 
-extern XENIFACE_PARAMETERS DriverParameters;
+extern PDRIVER_OBJECT
+DriverGetDriverObject(
+    VOID
+    );
+
+extern HANDLE
+DriverGetParametersKey(
+    VOID
+    );
 
 typedef struct _XENIFACE_DX {
     PDEVICE_OBJECT      DeviceObject;
index 29f83387ab630d00de50fe9b638801e45501d108..a2cb713f58badeb4f1a72cdc8a7adff89229ac07 100644 (file)
 
 #define MAXNAMELEN  128
 
-
-static void
+static NTSTATUS
 FdoInitialiseXSRegistryEntries(
     IN PXENIFACE_FDO        Fdo
     )
 {
-    OBJECT_ATTRIBUTES Attributes;
-    HANDLE RegHandle;
-    UNICODE_STRING UnicodeValueName;
-    UNICODE_STRING UnicodeValue;
-    ANSI_STRING AnsiValue;
-    char *value;
-    NTSTATUS status;
+    ANSI_STRING             Ansi[2];
+    HANDLE                  Key;
+    PCHAR                   Value;
+    NTSTATUS                status;
+
     NT_ASSERT(KeGetCurrentIrql() == PASSIVE_LEVEL);
+
     status = XENBUS_STORE(Read,
                           &Fdo->StoreInterface,
                           NULL,
                           NULL,
                           "/mh/boot-time/management-mac-address",
-                          &value);
-    if (!NT_SUCCESS(status)){
-        Error("no such xenstore key\n");
-        goto failXS;
-    }
-
-    InitializeObjectAttributes(&Attributes, &DriverParameters.RegistryPath,
-                                OBJ_CASE_INSENSITIVE | OBJ_KERNEL_HANDLE,
-                                NULL,
-                                NULL);
-
-    status = ZwOpenKey(&RegHandle, KEY_WRITE, &Attributes);
+                          &Value);
+    if (!NT_SUCCESS(status))
+        goto fail1;
 
-    if (!NT_SUCCESS(status)) {
-        Error("no such registry key %s\n", DriverParameters.RegistryPath);
-        goto failReg;
-    }
+    status = RegistryOpenParametersKey(KEY_WRITE, &Key);
+    if (!NT_SUCCESS(status))
+        goto fail2;
 
-    RtlInitUnicodeString(&UnicodeValueName, L"MgmtMacAddr");
-    RtlInitUnicodeString(&UnicodeValue, NULL);
-    RtlInitAnsiString(&AnsiValue, value);
+    RtlInitAnsiString(&Ansi[0], Value);
+    RtlZeroMemory(&Ansi[1], sizeof(ANSI_STRING));
 
-    Info("About to convert unicode string\n");
-    status = RtlAnsiStringToUnicodeString(&UnicodeValue, &AnsiValue, TRUE);
-    if (!NT_SUCCESS(status)) {
-        Error("Can't convert string\n");
-        goto failReg;
-    }
+    status = RegistryUpdateSzValue(Key,
+                                   "MgmtMacAddr",
+                                   REG_SZ,
+                                   &Ansi[0]);
+    if (!NT_SUCCESS(status))
+        goto fail3;
 
-    Info("About to write unicode string\n");
-    status = ZwSetValueKey(RegHandle, &UnicodeValueName, 0, REG_SZ, UnicodeValue.Buffer, UnicodeValue.Length+sizeof(WCHAR));
-    if (!NT_SUCCESS(status)) {
-        Error("Can't write key\n");
-        goto failWrite;
-    }
+    RegistryCloseKey(Key);
 
-    ZwClose(RegHandle);
+    XENBUS_STORE(Free, &Fdo->StoreInterface, Value);
 
-    RtlFreeUnicodeString(&UnicodeValue);
-    XENBUS_STORE(Free, &Fdo->StoreInterface, value);
+    return STATUS_SUCCESS;
 
-    return;
+fail3:
+    Error("fail3\n");
 
-failWrite:
+    RegistryCloseKey(Key);
 
-    Error("Fail : Write\n");
-    ZwClose(RegHandle);
-    RtlFreeUnicodeString(&UnicodeValue);
+fail2:
+    Error("fail2\n");
 
-failReg:
+    XENBUS_STORE(Free, &Fdo->StoreInterface, Value);
 
-    Error("Fail : Reg\n");
-    XENBUS_STORE(Free, &Fdo->StoreInterface, value);
+fail1:
+    Error("fail1 %08x\n", status);
 
-failXS:
-    Error("Failed to initialise registry (%08x)\n", status);
-    return;
+    return status;
 }
 
-
 #define REGISTRY_WRITE_EVENT 0
 #define REGISTRY_THREAD_END_EVENT 1
 #define REGISTRY_EVENTS 2
@@ -163,7 +141,7 @@ static NTSTATUS FdoRegistryThreadHandler(IN  PXENIFACE_THREAD  Self,
         if ((status>=STATUS_WAIT_0) && (status < STATUS_WAIT_0+REGISTRY_EVENTS)) {
             if (status == STATUS_WAIT_0+REGISTRY_WRITE_EVENT) {
                 Info("WriteRegistry\n");
-                FdoInitialiseXSRegistryEntries(Fdo);
+                (VOID) FdoInitialiseXSRegistryEntries(Fdo);
                 KeClearEvent(threadevents[REGISTRY_WRITE_EVENT]);
             }
             if (status == STATUS_WAIT_0+REGISTRY_THREAD_END_EVENT) {
@@ -2475,7 +2453,7 @@ FdoCreate(
     NTSTATUS            status;
 
 #pragma prefast(suppress:28197) // Possibly leaking memory 'FunctionDeviceObject'
-    status = IoCreateDevice(DriverObject,
+    status = IoCreateDevice(DriverGetDriverObject(),
                             sizeof (XENIFACE_DX),
                             NULL,
                             FILE_DEVICE_UNKNOWN,
@@ -2585,7 +2563,7 @@ FdoCreate(
     InitializeListHead(&Dx->ListEntry);
     Fdo->References = 1;
 
-    FdoInitialiseXSRegistryEntries(Fdo);
+    (VOID) FdoInitialiseXSRegistryEntries(Fdo);
 
     KeInitializeEvent(&Fdo->registryWriteEvent, NotificationEvent, FALSE);
 
index 8f848184344af65fc71fc17d8e10e3616c0f1fa4..1ab85c923a64becfdd9ff7fa51596098694a884c 100644 (file)
 
 #define REGISTRY_TAG 'GERX'
 
+static PDRIVER_OBJECT   RegistryDriverObject;
 static UNICODE_STRING   RegistryPath;
 
+typedef NTSTATUS(*IOOPENDRIVERREGISTRYKEY)(PDRIVER_OBJECT, DRIVER_REGKEY_TYPE, ACCESS_MASK, ULONG, PHANDLE);
+
+static IOOPENDRIVERREGISTRYKEY __IoOpenDriverRegistryKey;
+
 static FORCEINLINE PVOID
 __RegistryAllocate(
     IN  ULONG   Length
@@ -58,9 +63,12 @@ __RegistryFree(
 
 NTSTATUS
 RegistryInitialize(
-    IN PUNICODE_STRING  Path
+    IN  PDRIVER_OBJECT  DriverObject,
+    IN  PUNICODE_STRING Path
     )
 {
+    UNICODE_STRING      Unicode;
+    PVOID               Func;
     NTSTATUS            status;
 
     ASSERT3P(RegistryPath.Buffer, ==, NULL);
@@ -69,6 +77,16 @@ RegistryInitialize(
     if (!NT_SUCCESS(status))
         goto fail1;
 
+    ASSERT3P(RegistryDriverObject, ==, NULL);
+    RegistryDriverObject = DriverObject;
+
+    ASSERT3P(__IoOpenDriverRegistryKey, ==, NULL);
+    RtlInitUnicodeString(&Unicode, L"IoOpenDriverRegistryKey");
+
+    Func = MmGetSystemRoutineAddress(&Unicode);
+    if (Func != NULL)
+        __IoOpenDriverRegistryKey = (IOOPENDRIVERREGISTRYKEY)Func;
+
     return STATUS_SUCCESS;
 
 fail1:
@@ -82,11 +100,24 @@ RegistryTeardown(
     VOID
     )
 {
+    __IoOpenDriverRegistryKey = NULL;
+
+    RegistryDriverObject = NULL;
+
     RtlFreeUnicodeString(&RegistryPath);
     RegistryPath.Buffer = NULL;
     RegistryPath.MaximumLength = RegistryPath.Length = 0;
 }
 
+PUNICODE_STRING
+RegistryGetPath(
+    VOID
+    )
+{
+    return &RegistryPath;
+}
+
+
 NTSTATUS
 RegistryOpenKey(
     IN  HANDLE          Parent,
@@ -266,6 +297,54 @@ RegistryCreateServiceKey(
     return RegistryCreateKey(NULL, &RegistryPath, REG_OPTION_NON_VOLATILE, Key);
 }
 
+NTSTATUS
+RegistryOpenParametersKey(
+    IN  ACCESS_MASK     DesiredAccess,
+    OUT PHANDLE         Key
+    )
+{
+    HANDLE              ServiceKey;
+    NTSTATUS            status;
+
+    if (__IoOpenDriverRegistryKey != NULL) {
+        status = __IoOpenDriverRegistryKey(RegistryDriverObject,
+                                           DriverRegKeyParameters,
+                                           DesiredAccess,
+                                           0,
+                                           Key);
+        if (!NT_SUCCESS(status))
+            goto fail1;
+
+        goto done;
+    }
+
+    status = RegistryOpenKey(NULL, &RegistryPath, DesiredAccess, &ServiceKey);
+    if (!NT_SUCCESS(status))
+        goto fail2;
+
+    status = RegistryOpenSubKey(ServiceKey, "Parameters", DesiredAccess, Key);
+    if (!NT_SUCCESS(status))
+        goto fail3;
+
+    RegistryCloseKey(ServiceKey);
+
+done:
+    return STATUS_SUCCESS;
+
+fail3:
+    Error("fail3\n");
+
+    RegistryCloseKey(ServiceKey);
+
+fail2:
+    Error("fail2\n");
+
+fail1:
+    Error("fail1 %08x\n", status);
+
+    return status;
+}
+
 NTSTATUS
 RegistryOpenSoftwareKey(
     IN  PDEVICE_OBJECT  DeviceObject,
index 1cf86712c6f9a3169afe79dda6be4c61e4dcd36b..42ee9c8e85065303ad7d564befc7c523ed637805 100644 (file)
@@ -37,7 +37,8 @@
 
 extern NTSTATUS
 RegistryInitialize(
-    IN PUNICODE_STRING  Path
+    IN  PDRIVER_OBJECT  DriverObject,
+    IN  PUNICODE_STRING Path
     );
 
 extern VOID
@@ -45,6 +46,11 @@ RegistryTeardown(
     VOID
     );
 
+extern PUNICODE_STRING
+RegistryGetPath(
+    VOID
+    );
+
 extern NTSTATUS
 RegistryOpenKey(
     IN  HANDLE          Parent,
@@ -72,6 +78,12 @@ RegistryCreateServiceKey(
     OUT PHANDLE     Key
     );
 
+extern NTSTATUS
+RegistryOpenParametersKey(
+    IN  ACCESS_MASK     DesiredAccess,
+    OUT PHANDLE         Key
+    );
+
 extern NTSTATUS
 RegistryOpenSoftwareKey(
     IN  PDEVICE_OBJECT  DeviceObject,
index 97ef2c3cf0c27b107280afb6c2dc1d661dc4de46..95b4bb1ceaaa463795370f0a594a2e5165617b16 100644 (file)
@@ -41,6 +41,7 @@
 #include <ntstrsafe.h>
 #include "wmi.h"
 #include "driver.h"
+#include "registry.h"
 #include "store_interface.h"
 #include "suspend_interface.h"
 #include "log.h"
@@ -2999,6 +3000,7 @@ WmiRegInfo(
     UCHAR*                  mofnameptr;
     UCHAR*                  regpath;
     ULONG                   RequiredSize;
+    PUNICODE_STRING         RegistryPath;
 
     const int entries = 4;
     const static UNICODE_STRING mofname = RTL_CONSTANT_STRING(L"XENIFACEMOF");
@@ -3010,13 +3012,14 @@ WmiRegInfo(
     else
         mofnamesz = 0;
 
+    RegistryPath = RegistryGetPath();
     if (!AccessWmiBuffer(Stack->Parameters.WMI.Buffer, FALSE,
                          &RequiredSize,
                          Stack->Parameters.WMI.BufferSize,
                          WMI_BUFFER, sizeof(WMIREGINFO), (UCHAR **)&reginfo,
                          WMI_BUFFER, entries * sizeof(WMIREGGUID), (UCHAR **)&guiddata,
                          WMI_STRING, mofnamesz, &mofnameptr,
-                         WMI_STRING, DriverParameters.RegistryPath.Length + sizeof(USHORT), &regpath,
+                         WMI_STRING, RegistryPath->Length + sizeof(USHORT), &regpath,
                          WMI_DONE)) {
         reginfo->BufferSize = RequiredSize;
         *BytesWritten = sizeof(ULONG);
@@ -3027,7 +3030,7 @@ WmiRegInfo(
         reginfo->MofResourceName = (ULONG)((ULONG_PTR)mofnameptr - (ULONG_PTR)reginfo);
         WriteCountedUnicodeString(&mofname, mofnameptr);
         reginfo->RegistryPath = (ULONG)((ULONG_PTR)regpath - (ULONG_PTR)reginfo);
-        WriteCountedUnicodeString(&DriverParameters.RegistryPath, regpath);
+        WriteCountedUnicodeString(RegistryPath, regpath);
     }
 
     reginfo->BufferSize = RequiredSize;