[ros-diffs] [hbelusca] 74735: [NTOS]: Better clean up behind ourselves on failure paths (aka. closing handles). These functions are clearly not written with correct cleanup-on-failure concept in mind...

Author: hbelusca
Date: Thu Jun  1 18:37:48 2017
New Revision: 74735

URL: http://svn.reactos.org/svn/reactos?rev=74735&view=rev
Log:
[NTOS]: Better clean up behind ourselves on failure paths (aka. closing handles). These
functions are clearly not written with correct cleanup-on-failure concept in mind...

Modified:
    trunk/reactos/ntoskrnl/config/i386/cmhardwr.c
    trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c

Modified: trunk/reactos/ntoskrnl/config/i386/cmhardwr.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/i386/cmhar…
==============================================================================
--- trunk/reactos/ntoskrnl/config/i386/cmhardwr.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/config/i386/cmhardwr.c	[iso-8859-1] Thu Jun  1 18:37:48 2017
@@ -382,7 +382,8 @@
                                NULL,
                                NULL);
     Status = NtOpenKey(&SystemHandle, KEY_READ | KEY_WRITE, &ObjectAttributes);
-    if (!NT_SUCCESS(Status)) return Status;
+    if (!NT_SUCCESS(Status))
+        return Status;
 
     /* Create the BIOS Information key */
     RtlInitUnicodeString(&KeyName,
@@ -406,7 +407,10 @@
             BiosHandle = NULL;
     }
     else if (!NT_SUCCESS(Status))
+    {
+        NtClose(SystemHandle);
         return Status;
+    }
 
     /* Create the CPU Key, and check if it already existed */
     RtlInitUnicodeString(&KeyName, L"CentralProcessor");
@@ -431,7 +435,11 @@
         CmpConfigurationData = ExAllocatePoolWithTag(PagedPool,
                                                      CmpConfigurationAreaSize,
                                                      TAG_CM);
-        if (!CmpConfigurationData) return STATUS_INSUFFICIENT_RESOURCES;
+        if (!CmpConfigurationData)
+        {
+            // FIXME: Cleanup stuff!!
+            return STATUS_INSUFFICIENT_RESOURCES;
+        }
 
         /* Loop all CPUs */
         for (i = 0; i < KeNumberProcessors; i++)
@@ -440,7 +448,7 @@
             Prcb = KiProcessorBlock[i];
 
             /* Setup the Configuration Entry for the Processor */
-            RtlZeroMemory(&ConfigData, sizeof (ConfigData));
+            RtlZeroMemory(&ConfigData, sizeof(ConfigData));
             ConfigData.ComponentEntry.Class = ProcessorClass;
             ConfigData.ComponentEntry.Type = CentralProcessor;
             ConfigData.ComponentEntry.Key = i;
@@ -477,7 +485,12 @@
                                                InterfaceTypeUndefined,
                                                0xFFFFFFFF,
                                                IndexTable);
-            if (!NT_SUCCESS(Status)) return(Status);
+            if (!NT_SUCCESS(Status))
+            {
+                NtClose(BiosHandle);
+                NtClose(SystemHandle);
+                return Status;
+            }
 
             /* Check if we have an FPU */
             if (KeI386NpxPresent)
@@ -505,8 +518,10 @@
                                                    IndexTable);
                 if (!NT_SUCCESS(Status))
                 {
-                    /* Failed, close the CPU handle and return */
+                    /* We failed, close all the opened handles and return */
                     NtClose(KeyHandle);
+                    NtClose(BiosHandle);
+                    NtClose(SystemHandle);
                     return Status;
                 }
 
@@ -672,7 +687,15 @@
     Status = ZwOpenSection(&SectionHandle,
                            SECTION_ALL_ACCESS,
                            &ObjectAttributes);
-    if (!NT_SUCCESS(Status)) goto Quickie;
+    if (!NT_SUCCESS(Status))
+    {
+        /* We failed, close all the opened handles and return */
+        // NtClose(KeyHandle);
+        NtClose(BiosHandle);
+        NtClose(SystemHandle);
+        /* 'Quickie' closes KeyHandle */
+        goto Quickie;
+    }
 
     /* Map the first 1KB of memory to get the IVT */
     ViewSize = PAGE_SIZE;
@@ -939,7 +962,7 @@
     if (BiosVersion) ExFreePoolWithTag(BiosVersion, TAG_CM);
 
 Quickie:
-    /* Close the procesor handle */
+    /* Close the processor handle */
     NtClose(KeyHandle);
     return STATUS_SUCCESS;
 }

Modified: trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/powerpc/cm…
==============================================================================
--- trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c	[iso-8859-1] Thu Jun  1 18:37:48
2017
@@ -95,7 +95,7 @@
                 }
             }
 
-            /* Add slashes were we previously had NULLs */
+            /* Add slashes where we previously had NULLs */
             CurrentDate[4] = CurrentDate[7] = '/';
 
             /* Check which date is newer */
@@ -244,7 +244,7 @@
     PCHAR PartialString = NULL, BiosVersion;
     CHAR CpuString[48];
     PVOID BaseAddress = NULL;
-    LARGE_INTEGER ViewBase = {{0}};
+    LARGE_INTEGER ViewBase = {{0, 0}};
     ULONG_PTR VideoRomBase;
     PCHAR CurrentVersion;
     extern UNICODE_STRING KeRosProcessorName, KeRosBiosDate, KeRosBiosVersion;
@@ -287,7 +287,8 @@
                                NULL,
                                NULL);
     Status = NtOpenKey(&SystemHandle, KEY_READ | KEY_WRITE, &ObjectAttributes);
-    if (!NT_SUCCESS(Status)) return Status;
+    if (!NT_SUCCESS(Status))
+        return Status;
 
     /* Create the BIOS Information key */
     RtlInitUnicodeString(&KeyName,
@@ -311,7 +312,10 @@
             BiosHandle = NULL;
     }
     else if (!NT_SUCCESS(Status))
+    {
+        NtClose(SystemHandle);
         return Status;
+    }
 
     /* Create the CPU Key, and check if it already existed */
     RtlInitUnicodeString(&KeyName, L"CentralProcessor");
@@ -336,7 +340,11 @@
         CmpConfigurationData = ExAllocatePoolWithTag(PagedPool,
                                                      CmpConfigurationAreaSize,
                                                      TAG_CM);
-        if (!CmpConfigurationData) return STATUS_INSUFFICIENT_RESOURCES;
+        if (!CmpConfigurationData)
+        {
+            // FIXME: Cleanup stuff!!
+            return STATUS_INSUFFICIENT_RESOURCES;
+        }
 
         /* Loop all CPUs */
         for (i = 0; i < KeNumberProcessors; i++)
@@ -345,7 +353,7 @@
             Prcb = KiProcessorBlock[i];
 
             /* Setup the Configuration Entry for the Processor */
-            RtlZeroMemory(&ConfigData, sizeof (ConfigData));
+            RtlZeroMemory(&ConfigData, sizeof(ConfigData));
             ConfigData.ComponentEntry.Class = ProcessorClass;
             ConfigData.ComponentEntry.Type = CentralProcessor;
             ConfigData.ComponentEntry.Key = i;
@@ -382,7 +390,12 @@
                                                InterfaceTypeUndefined,
                                                0xFFFFFFFF,
                                                IndexTable);
-            if (!NT_SUCCESS(Status)) return(Status);
+            if (!NT_SUCCESS(Status))
+            {
+                NtClose(BiosHandle);
+                NtClose(SystemHandle);
+                return Status;
+            }
 
             {
                 /* Setup the Configuration Entry for the FPU */
@@ -408,8 +421,10 @@
                                                    IndexTable);
                 if (!NT_SUCCESS(Status))
                 {
-                    /* Failed, close the CPU handle and return */
+                    /* We failed, close all the opened handles and return */
                     NtClose(KeyHandle);
+                    NtClose(BiosHandle);
+                    NtClose(SystemHandle);
                     return Status;
                 }
 
@@ -428,7 +443,7 @@
                     //Ki386Cpuid(0x80000000, &ExtendedId, &Dummy, &Dummy,
&Dummy);
                     if (ExtendedId >= 0x80000004)
                     {
-                        /* Do all the CPUIDs requred to get the full name */
+                        /* Do all the CPUIDs required to get the full name */
                         PartialString = CpuString;
                         for (ExtendedId = 2; ExtendedId <= 4; ExtendedId++)
                         {
@@ -559,7 +574,15 @@
     Status = ZwOpenSection(&SectionHandle,
                            SECTION_ALL_ACCESS,
                            &ObjectAttributes);
-    if (!NT_SUCCESS(Status)) goto Quickie;
+    if (!NT_SUCCESS(Status))
+    {
+        /* We failed, close all the opened handles and return */
+        // NtClose(KeyHandle);
+        NtClose(BiosHandle);
+        NtClose(SystemHandle);
+        /* 'Quickie' closes KeyHandle */
+        goto Quickie;
+    }
 
     /* Map the first 1KB of memory to get the IVT */
     ViewSize = PAGE_SIZE;