From 4b4ffa92f564a334ad919bdda2404561aa1ab10f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?=
 <hermes.belusca-maito@reactos.org>
Date: Thu, 10 Jun 2021 19:44:59 +0200
Subject: [PATCH] [NTOS:IO] Modify when 'PartitionBuffer' and how 'DriveLayout'
 are freed in IopCreateArcNamesDisk().

- Manage the lifetime of the temporary 'PartitionBuffer' buffer where
  it is locally used only, and free it as soon as possible, just after
  calculating the sector checksum. No need to then free it outside of
  the main for-loop.

- When the 'DriveLayout' buffer is freed, ensure the pointer is NULL-ed
  (and assert this at the top of the main for-loop), since it can also
  be freed at cleanup outside this for-loop, and in this case a NULL
  check is performed.
  This will avoid the scenario of possibly double-freeing a pointer,
  in the case the 'DriveLayout' was previously freed (when e.g. reading
  the sector for checksum calculation failed), then the for-loop goes to
  the next disk and stops early.
---
 ntoskrnl/io/iomgr/arcname.c | 42 +++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/ntoskrnl/io/iomgr/arcname.c b/ntoskrnl/io/iomgr/arcname.c
index 3be5930ec69..937b5cfb0c5 100644
--- a/ntoskrnl/io/iomgr/arcname.c
+++ b/ntoskrnl/io/iomgr/arcname.c
@@ -478,6 +478,8 @@ IopCreateArcNamesDisk(IN PLOADER_PARAMETER_BLOCK LoaderBlock,
     /* Start browsing disks */
     for (DiskNumber = 0; DiskNumber < DiskCount; DiskNumber++)
     {
+        ASSERT(DriveLayout == NULL);
+
         /* Check if we have an enabled disk */
         if (lSymbolicLinkList && *lSymbolicLinkList != UNICODE_NULL)
         {
@@ -652,6 +654,7 @@ IopCreateArcNamesDisk(IN PLOADER_PARAMETER_BLOCK LoaderBlock,
                                            &IoStatusBlock);
         if (!Irp)
         {
+            ExFreePoolWithTag(PartitionBuffer, TAG_IO);
             ObDereferenceObject(FileObject);
             Status = STATUS_INSUFFICIENT_RESOURCES;
             goto Cleanup;
@@ -665,22 +668,28 @@ IopCreateArcNamesDisk(IN PLOADER_PARAMETER_BLOCK LoaderBlock,
             KeWaitForSingleObject(&Event, Executive, KernelMode, FALSE, NULL);
             Status = IoStatusBlock.Status;
         }
+
+        /* If reading succeeded, calculate checksum by adding data */
+        if (NT_SUCCESS(Status))
+        {
+            for (i = 0, CheckSum = 0; i < 512 / sizeof(ULONG); i++)
+            {
+                CheckSum += PartitionBuffer[i];
+            }
+        }
+
+        /* Release now unnecessary resources */
+        ExFreePoolWithTag(PartitionBuffer, TAG_IO);
+        ObDereferenceObject(FileObject);
+
+        /* If we failed, release drive layout before going to next disk */
         if (!NT_SUCCESS(Status))
         {
             ExFreePool(DriveLayout);
-            ExFreePoolWithTag(PartitionBuffer, TAG_IO);
-            ObDereferenceObject(FileObject);
+            DriveLayout = NULL;
             continue;
         }
 
-        ObDereferenceObject(FileObject);
-
-        /* Calculate checksum by adding data */
-        for (i = 0, CheckSum = 0; i < 512 / sizeof(ULONG) ; i++)
-        {
-            CheckSum += PartitionBuffer[i];
-        }
-
         /* Browse each ARC disk */
         for (NextEntry = ArcDiskInformation->DiskSignatureListHead.Flink;
              NextEntry != &ArcDiskInformation->DiskSignatureListHead;
@@ -800,29 +809,22 @@ IopCreateArcNamesDisk(IN PLOADER_PARAMETER_BLOCK LoaderBlock,
             }
         }
 
-        /* Release memory before jumping to next item */
+        /* Finally, release drive layout */
         ExFreePool(DriveLayout);
         DriveLayout = NULL;
-        ExFreePoolWithTag(PartitionBuffer, TAG_IO);
-        PartitionBuffer = NULL;
     }
 
     Status = STATUS_SUCCESS;
 
 Cleanup:
-    if (SymbolicLinkList)
-    {
-        ExFreePool(SymbolicLinkList);
-    }
-
     if (DriveLayout)
     {
         ExFreePool(DriveLayout);
     }
 
-    if (PartitionBuffer)
+    if (SymbolicLinkList)
     {
-        ExFreePoolWithTag(PartitionBuffer, TAG_IO);
+        ExFreePool(SymbolicLinkList);
     }
 
     return Status;