[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 3/4] SDV: ZwRegistryOpen rule violations


  • To: <win-pv-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Owen Smith <owen.smith@xxxxxxxxxx>
  • Date: Mon, 7 Feb 2022 13:15:02 +0000
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Owen Smith <owen.smith@xxxxxxxxxx>
  • Delivery-date: Mon, 07 Feb 2022 13:15:20 +0000
  • Ironport-data: A9a23:Gff0mqB8+AacqhVW/wnkw5YqxClBgxIJ4kV8jS/XYbTApDkk3jdVm GpNUGrTM/iJNDOhKoonb4+/oE0HucOAx4cwQQY4rX1jcSlH+JHPbTi7wuYcHM8wwunrFh8PA xA2M4GYRCwMo/u1Si6FatANl1ElvU2zbue6WL6s1hxZH1c+En970U07wobVv6Yz6TSHK1LV0 T/Ni5W31G+Ng1aY5UpNtspvADs21BjDkGtwUm4WPJinj3eH/5UhN7oNJLnZEpfNatI88thW5 Qr05OrREmvxp3/BAz4++1rxWhVirrX6ZWBihpfKMkQLb9crSiEai84G2PQghUh/1QjQuMJ89 +R2vr+VQhklN6zIx/kxekwNe81+FfUuFL7vJHG+tYqYzlHccmuqyPJrZK00FdRGoKAtWzgIr KFGbmBWBvyAr7veLLaTTfZhgII4Kcj3PKsUu216zCGfBvEjKXzGa/uXtYIAgm1v7ixINfTDV fEpdDlRVRT/RAxTeXpJM5l9od790xETdBUH8QnI9MLb+VP70w13laDzddbYZNGObcFUhVqD4 HLL+XzjBRMXP8DZziCKmk9AnceWw3m9AthLUuTlqLg62zV/21D/FjVIaAKcs/SGu3W1ePdUN UAd+y5xsIQboRnDosbGYzW0p3uNvxg5UtVWEvEn5Azl9pc48zp1FUBfEGcfNYVOWNseAGVzi wTXx4+B6SlH7eXNIU9x4It4ut9b1cI9CWYZLREJQgIeizUIiNFi10mfJjqP/UPcszEUJd0S6 23QxMTdr+9K5SLu60lc1QqW6w9AXrCTEmYICvz/BwpJFD9Rao+/fJCP4lPG9/tGJ4vxZgDf4 CRdypHHsrFUVMrleMmxrAIlRuDB2hp4GGeE3Q4H82cJq1xBBEJPjagPuWojdS+Fw+4PeCPzY V+7hO+izMQ7AZdeVocuO9jZI51zlcDITI25PtiJPosmSsUgL2evoXAxDWbOhDqFuBZ3zskC1 WKzLJ/E4YAyUv88klJbho41jNcW+8zJ7TmPHMCgkUX7jub2ibz8Ye5tDWZip9sRtMusyDg5O f4GXydT4xkAAuD4fAfN9osfcQIDIXQhXMikoM1LbO+TZAFhHTh5WfPWxLogfa1jnrhUybiUr i3sBBcAxQqtn2DDJCWLdmtnNOHlU6FgoC9pJicrJ1uphSQuON798KcFepIrVrA77+g/n+VsR vwIdpzYUPRCQzjK4RoHapz5oNAwfRinn1vWbSGkfCI+b9hrQAmQoo3oeQ7m9S8vCCurtJRh/ +38h12DGZdaHlZsFsfbbv6r3midh3lFlbIgRVbML/lSZF7orNpgJRvug6JlOMoLMxjCmGeXj l7EHRcCqODRiIYp692V17ucpoKkHuYiTEpXG27XseS/OSXApzfxxIZBVKCDfCzHVXOy86KnP L0Hw/b5OfwBvVBLr4sjTOo7kfNgv4Pi9+1A0wBpPHTXdFD6WLpvL06P0dRLqqAQlKRSvhG7W x7X99RXUVlT1BgJzLLFyNIZU9m+
  • Ironport-hdrordr: A9a23:wgOt36yYKvtnZqXxpalrKrPwKL1zdoMgy1knxilNoHtuA6ulfq GV7ZAmPHrP4wr5N0tNpTntAsa9qBDnlaKdg7N+AV7KZmCP0gaVxepZjLfK8nnNHDD/6/4Y9Y oISdkaNDQoNykYsS8t2njbL+od
  • Ironport-sdr: G29TIb+fd+iDw4bWXqmC7NXSI+1H+npPxCvmRjacdgxcQ/q3612lQxP8XxnNfCoHIbLGoahvM9 p2aSYfTDLGE9sOTc+kK8eIIDxAZVUqN2hxm6CAaOjwZGIo1wvXIwX29JhNHiZrq/3YeJA0A0kr AHAfmrhTSPjCqZ2bo/URFVPf2zmAsmr0KlVoRw3GcYP32vFLwb/B1O/t4rKpW4kxkmn0nyJCI8 49V//GhJNW+lMhKD5SWHoagnCL7yEsssw0ldfZ2flOTCuLEpDRj1+L8AG6fnTUhtjP5yKeh8fD MjlC+PF3bFdJ8OvloFIwpvG6
  • List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>

Dont hold the ParametersKey open, SDV treats this as a mismatched
ZwRegistryOpen and ZwClose pair.
Open the registry key when required, and close it once its no longer
required.

Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
---
 src/xenfilt/driver.c | 103 +++++++++++++++++++++----------------------
 1 file changed, 50 insertions(+), 53 deletions(-)

diff --git a/src/xenfilt/driver.c b/src/xenfilt/driver.c
index f131282..ce9e0cd 100644
--- a/src/xenfilt/driver.c
+++ b/src/xenfilt/driver.c
@@ -47,7 +47,6 @@
 
 typedef struct _XENFILT_DRIVER {
     PDRIVER_OBJECT              DriverObject;
-    HANDLE                      ParametersKey;
 
     MUTEX                       Mutex;
     LIST_ENTRY                  List;
@@ -113,28 +112,39 @@ DriverGetDriverObject(
     return __DriverGetDriverObject();
 }
 
-static FORCEINLINE VOID
-__DriverSetParametersKey(
-    IN  HANDLE  Key
+static FORCEINLINE NTSTATUS
+__DriverOpenParametersKey(
+    OUT PHANDLE     ParametersKey
     )
 {
-    Driver.ParametersKey = Key;
-}
+    HANDLE          ServiceKey;
+    NTSTATUS        status;
 
-static FORCEINLINE HANDLE
-__DriverGetParametersKey(
-    VOID
-    )
-{
-    return Driver.ParametersKey;
+    status = RegistryOpenServiceKey(KEY_READ, &ServiceKey);
+    if (!NT_SUCCESS(status))
+        goto fail1;
+
+    status = RegistryOpenSubKey(ServiceKey, "Parameters", KEY_READ, 
ParametersKey);
+    if (!NT_SUCCESS(status))
+        goto fail2;
+
+    RegistryCloseKey(ServiceKey);
+
+    return STATUS_SUCCESS;
+
+fail2:
+    RegistryCloseKey(ServiceKey);
+
+fail1:
+    return status;
 }
 
-HANDLE
-DriverGetParametersKey(
-    VOID
+NTSTATUS
+DriverOpenParametersKey(
+    OUT PHANDLE     ParametersKey
     )
 {
-    return __DriverGetParametersKey();
+    return __DriverOpenParametersKey(ParametersKey);
 }
 
 static FORCEINLINE VOID
@@ -244,7 +254,9 @@ __DriverGetActive(
 
     ASSERT3U(KeGetCurrentIrql(), ==, PASSIVE_LEVEL);
 
-    ParametersKey = __DriverGetParametersKey();
+    status = __DriverOpenParametersKey(&ParametersKey);
+    if (!NT_SUCCESS(status))
+        goto fail1;
 
     status = RtlStringCbPrintfA(Name, MAXNAMELEN, "Active%s", Key);
     ASSERT(NT_SUCCESS(status));
@@ -254,14 +266,14 @@ __DriverGetActive(
                                   NULL,
                                   &Ansi);
     if (!NT_SUCCESS(status))
-        goto fail1;
+        goto fail2;
 
     Length = Ansi[0].Length + sizeof (CHAR);
     *Value = __AllocatePoolWithTag(NonPagedPool, Length, 'TLIF');
 
     status = STATUS_NO_MEMORY;
     if (*Value == NULL)
-        goto fail2;
+        goto fail3;
 
     status = RtlStringCbPrintfA(*Value,
                                 Length,
@@ -271,13 +283,20 @@ __DriverGetActive(
 
     RegistryFreeSzValue(Ansi);
 
+    RegistryCloseKey(ParametersKey);
+
     Trace("<====\n");
 
     return STATUS_SUCCESS;
 
+fail3:
+    Error("fail3\n");
+
 fail2:
     Error("fail2\n");
 
+    RegistryCloseKey(ParametersKey);
+
 fail1:
     if (status != STATUS_OBJECT_NAME_NOT_FOUND)
         Error("fail1 (%08x)\n", status);
@@ -409,8 +428,6 @@ DriverUnload(
     IN  PDRIVER_OBJECT  DriverObject
     )
 {
-    HANDLE              ParametersKey;
-
     ASSERT3P(DriverObject, ==, __DriverGetDriverObject());
 
     Trace("====>\n");
@@ -428,10 +445,6 @@ DriverUnload(
     EmulatedTeardown(Driver.EmulatedContext);
     Driver.EmulatedContext = NULL;
 
-    ParametersKey = __DriverGetParametersKey();
-    __DriverSetParametersKey(NULL);
-    RegistryCloseKey(ParametersKey);
-
     RegistryTeardown();
 
     Info("XENFILT %d.%d.%d (%d) (%02d.%02d.%04d)\n",
@@ -744,8 +757,11 @@ DriverGetEmulatedType(
     HANDLE                          ParametersKey;
     XENFILT_EMULATED_OBJECT_TYPE    Type;
     ULONG                           Index;
+    NTSTATUS                        status;
 
-    ParametersKey = __DriverGetParametersKey();
+    status = __DriverOpenParametersKey(&ParametersKey);
+    if (!NT_SUCCESS(status))
+        goto fail1;
 
     Type = XENFILT_EMULATED_OBJECT_TYPE_UNKNOWN;
     Index = 0;
@@ -753,7 +769,6 @@ DriverGetEmulatedType(
     do {
         ULONG           Length;
         PANSI_STRING    Ansi;
-        NTSTATUS        status;
 
         Length = (ULONG)strlen(&Id[Index]);
         if (Length == 0)
@@ -779,7 +794,14 @@ DriverGetEmulatedType(
         Index += Length + 1;
     } while (Type == XENFILT_EMULATED_OBJECT_TYPE_UNKNOWN);
 
+    RegistryCloseKey(ParametersKey);
+
     return Type;
+
+fail1:
+    Error("fail1 %08x\n", status);
+
+    return XENFILT_EMULATED_OBJECT_TYPE_UNKNOWN;
 }
 
 DRIVER_ADD_DEVICE   DriverAddDevice;
@@ -886,8 +908,6 @@ DriverEntry(
     IN  PUNICODE_STRING         RegistryPath
     )
 {
-    HANDLE                      ServiceKey;
-    HANDLE                      ParametersKey;
     PXENFILT_EMULATED_CONTEXT   EmulatedContext;
     ULONG                       Index;
     NTSTATUS                    status;
@@ -926,19 +946,9 @@ DriverEntry(
     if (!NT_SUCCESS(status))
         goto fail1;
 
-    status = RegistryOpenServiceKey(KEY_READ, &ServiceKey);
-    if (!NT_SUCCESS(status))
-        goto fail2;
-
-    status = RegistryOpenSubKey(ServiceKey, "Parameters", KEY_READ, 
&ParametersKey);
-    if (!NT_SUCCESS(status))
-        goto fail3;
-
-    __DriverSetParametersKey(ParametersKey);
-
     status = EmulatedInitialize(&EmulatedContext);
     if (!NT_SUCCESS(status))
-        goto fail4;
+        goto fail2;
 
     __DriverSetEmulatedContext(EmulatedContext);
 
@@ -948,8 +958,6 @@ DriverEntry(
                                   sizeof (Driver.EmulatedInterface));
     ASSERT(NT_SUCCESS(status));
 
-    RegistryCloseKey(ServiceKey);
-
     DriverObject->DriverExtension->AddDevice = DriverAddDevice;
 
     for (Index = 0; Index <= IRP_MJ_MAXIMUM_FUNCTION; Index++) {
@@ -966,17 +974,6 @@ done:
     Trace("<====\n");
     return STATUS_SUCCESS;
 
-fail4:
-    Error("fail4\n");
-
-    __DriverSetParametersKey(NULL);
-    RegistryCloseKey(ParametersKey);
-
-fail3:
-    Error("fail3\n");
-
-    RegistryCloseKey(ServiceKey);
-
 fail2:
     Error("fail2\n");
 
-- 
2.33.0.windows.2




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.