|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [XENIFACE PATCH 1/2] Fix Registry Isolation issues with Server 2025
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@xxxxxxxxx>
---
src/xeniface/driver.c | 146 +++++++++++++++++++++++++++++++---------
src/xeniface/driver.h | 17 ++---
src/xeniface/fdo.c | 94 ++++++++++----------------
src/xeniface/registry.c | 81 +++++++++++++++++++++-
src/xeniface/registry.h | 14 +++-
src/xeniface/wmi.c | 7 +-
6 files changed, 258 insertions(+), 101 deletions(-)
diff --git a/src/xeniface/driver.c b/src/xeniface/driver.c
index 6c3032b..3b5b16d 100644
--- a/src/xeniface/driver.c
+++ b/src/xeniface/driver.c
@@ -36,32 +36,99 @@
#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;
}
diff --git a/src/xeniface/driver.h b/src/xeniface/driver.h
index 27cd453..9007d97 100644
--- a/src/xeniface/driver.h
+++ b/src/xeniface/driver.h
@@ -46,19 +46,20 @@
#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;
diff --git a/src/xeniface/fdo.c b/src/xeniface/fdo.c
index 29f8338..a2cb713 100644
--- a/src/xeniface/fdo.c
+++ b/src/xeniface/fdo.c
@@ -61,85 +61,63 @@
#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);
diff --git a/src/xeniface/registry.c b/src/xeniface/registry.c
index 8f84818..1ab85c9 100644
--- a/src/xeniface/registry.c
+++ b/src/xeniface/registry.c
@@ -38,8 +38,13 @@
#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,
diff --git a/src/xeniface/registry.h b/src/xeniface/registry.h
index 1cf8671..42ee9c8 100644
--- a/src/xeniface/registry.h
+++ b/src/xeniface/registry.h
@@ -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,
diff --git a/src/xeniface/wmi.c b/src/xeniface/wmi.c
index 97ef2c3..95b4bb1 100644
--- a/src/xeniface/wmi.c
+++ b/src/xeniface/wmi.c
@@ -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 **)®info,
WMI_BUFFER, entries * sizeof(WMIREGGUID), (UCHAR
**)&guiddata,
WMI_STRING, mofnamesz, &mofnameptr,
- WMI_STRING, DriverParameters.RegistryPath.Length +
sizeof(USHORT), ®path,
+ WMI_STRING, RegistryPath->Length + sizeof(USHORT),
®path,
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;
--
2.44.0.windows.1
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |