[ros-diffs] [reactos] 01/02: [NTOS:IO] Fix basicInfo handling in IopGetDriverNames().

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=e09d1dec7ae938e27ca30…

commit e09d1dec7ae938e27ca3013d2ffefae6bf78d230
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Thu Jun 10 22:31:45 2021 +0200
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Wed Jun 16 22:18:02 2021 +0200

    [NTOS:IO] Fix basicInfo handling in IopGetDriverNames().
    
    - Fix CID 1477246: Uninitialized pointer read (UNINIT) (happens in
      the last ExFreePoolWithTag(basicInfo, TAG_IO) call when the
      "(!NT_SUCCESS(status) || ServiceName != NULL)" case is not taken).
    
    - Centralize all the ExFreePoolWithTag(basicInfo, TAG_IO) cleanups
      at the end of the function.
    
    - Both cases "(driverName.Buffer == NULL)" and "(ServiceName !=
NULL)"
      can only be taken when basicInfo != NULL, so assert on this fact.
---
 ntoskrnl/io/iomgr/driver.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/ntoskrnl/io/iomgr/driver.c b/ntoskrnl/io/iomgr/driver.c
index 6d82bfc536e..f48a85a9705 100644
--- a/ntoskrnl/io/iomgr/driver.c
+++ b/ntoskrnl/io/iomgr/driver.c
@@ -163,7 +163,7 @@ IopGetDriverNames(
     /* Check whether we need to get ServiceName as well, either to construct
      * the driver name (because we could not use "ObjectName"), or because
      * it is requested by the caller. */
-    PKEY_BASIC_INFORMATION basicInfo;
+    PKEY_BASIC_INFORMATION basicInfo = NULL;
     if (!NT_SUCCESS(status) || ServiceName != NULL)
     {
         /* Retrieve the necessary buffer size */
@@ -197,19 +197,20 @@ IopGetDriverNames(
      * it will be either "\Driver\<ServiceName>" or
"\FileSystem\<ServiceName>" */
     if (driverName.Buffer == NULL)
     {
+        ASSERT(basicInfo); // Container for serviceName
+
         /* Retrieve the driver type */
         ULONG driverType;
         status = IopGetRegistryValue(ServiceHandle, L"Type", &kvInfo);
         if (!NT_SUCCESS(status))
         {
-            ExFreePoolWithTag(basicInfo, TAG_IO);
-            return status;
+            goto Cleanup;
         }
         if (kvInfo->Type != REG_DWORD || kvInfo->DataLength != sizeof(ULONG))
         {
             ExFreePool(kvInfo);
-            ExFreePoolWithTag(basicInfo, TAG_IO); // container for serviceName
-            return STATUS_ILL_FORMED_SERVICE_ENTRY;
+            status = STATUS_ILL_FORMED_SERVICE_ENTRY;
+            goto Cleanup;
         }
         driverType = *(PULONG)((ULONG_PTR)kvInfo + kvInfo->DataOffset);
         ExFreePool(kvInfo);
@@ -227,8 +228,8 @@ IopGetDriverNames(
         driverName.Buffer = ExAllocatePoolWithTag(NonPagedPool, driverName.MaximumLength,
TAG_IO);
         if (!driverName.Buffer)
         {
-            ExFreePoolWithTag(basicInfo, TAG_IO); // container for serviceName
-            return STATUS_INSUFFICIENT_RESOURCES;
+            status = STATUS_INSUFFICIENT_RESOURCES;
+            goto Cleanup;
         }
 
         if (driverType == SERVICE_RECOGNIZER_DRIVER || driverType ==
SERVICE_FILE_SYSTEM_DRIVER)
@@ -241,24 +242,30 @@ IopGetDriverNames(
 
     if (ServiceName != NULL)
     {
+        ASSERT(basicInfo); // Container for serviceName
+
         /* Allocate a copy for the caller */
         PWCHAR buf = ExAllocatePoolWithTag(PagedPool, serviceName.Length, TAG_IO);
         if (!buf)
         {
-            ExFreePoolWithTag(basicInfo, TAG_IO); // container for serviceName
             ExFreePoolWithTag(driverName.Buffer, TAG_IO);
-            return STATUS_INSUFFICIENT_RESOURCES;
+            status = STATUS_INSUFFICIENT_RESOURCES;
+            goto Cleanup;
         }
         RtlMoveMemory(buf, serviceName.Buffer, serviceName.Length);
         ServiceName->MaximumLength = serviceName.Length;
         ServiceName->Length = serviceName.Length;
         ServiceName->Buffer = buf;
     }
-    ExFreePoolWithTag(basicInfo, TAG_IO); // container for ServiceName
 
     *DriverName = driverName;
+    status = STATUS_SUCCESS;
 
-    return STATUS_SUCCESS;
+Cleanup:
+    if (basicInfo)
+        ExFreePoolWithTag(basicInfo, TAG_IO);
+
+    return status;
 }
 
 /*