https://git.reactos.org/?p=reactos.git;a=commitdiff;h=4b4ffa92f564a334ad919bdda2404561aa1ab10f
commit 4b4ffa92f564a334ad919bdda2404561aa1ab10f Author: Hermès Bélusca-Maïto <[email protected]> AuthorDate: Thu Jun 10 19:44:59 2021 +0200 Commit: Hermès Bélusca-Maïto <[email protected]> CommitDate: Fri Jun 11 02:21:48 2021 +0200 [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 | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 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,20 +668,26 @@ IopCreateArcNamesDisk(IN PLOADER_PARAMETER_BLOCK LoaderBlock, KeWaitForSingleObject(&Event, Executive, KernelMode, FALSE, NULL); Status = IoStatusBlock.Status; } - if (!NT_SUCCESS(Status)) + + /* If reading succeeded, calculate checksum by adding data */ + if (NT_SUCCESS(Status)) { - ExFreePool(DriveLayout); - ExFreePoolWithTag(PartitionBuffer, TAG_IO); - ObDereferenceObject(FileObject); - continue; + for (i = 0, CheckSum = 0; i < 512 / sizeof(ULONG); i++) + { + CheckSum += PartitionBuffer[i]; + } } + /* Release now unnecessary resources */ + ExFreePoolWithTag(PartitionBuffer, TAG_IO); ObDereferenceObject(FileObject); - /* Calculate checksum by adding data */ - for (i = 0, CheckSum = 0; i < 512 / sizeof(ULONG) ; i++) + /* If we failed, release drive layout before going to next disk */ + if (!NT_SUCCESS(Status)) { - CheckSum += PartitionBuffer[i]; + ExFreePool(DriveLayout); + DriveLayout = NULL; + continue; } /* Browse each ARC disk */ @@ -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;
