https://git.reactos.org/?p=reactos.git;a=commitdiff;h=8d2fe54188616fa5869468ecf108bf4af9a8624f
commit 8d2fe54188616fa5869468ecf108bf4af9a8624f Author: Victor Perevertkin <[email protected]> AuthorDate: Mon Dec 7 14:43:34 2020 +0300 Commit: Victor Perevertkin <[email protected]> CommitDate: Mon Dec 7 14:43:34 2020 +0300 [FSTUB] Fix out of bounds access in IoReadDiskSignature - Convert PARTITION_TABLE_OFFSET to the number of bytes instead of (number of bytes) / 2. This avoids many confusing casts - Use a cache aligned buffer for MBR --- ntoskrnl/fstub/disksup.c | 31 ++++++++++++------------------- ntoskrnl/fstub/fstubex.c | 12 ++++++------ ntoskrnl/include/internal/hal.h | 7 ++++--- 3 files changed, 22 insertions(+), 28 deletions(-) diff --git a/ntoskrnl/fstub/disksup.c b/ntoskrnl/fstub/disksup.c index 71d51873f94..a685393b143 100644 --- a/ntoskrnl/fstub/disksup.c +++ b/ntoskrnl/fstub/disksup.c @@ -1712,8 +1712,7 @@ xHalExamineMBR(IN PDEVICE_OBJECT DeviceObject, } /* Get the partition entry */ - PartitionDescriptor = (PPARTITION_DESCRIPTOR) - &(((PUSHORT)Buffer)[PARTITION_TABLE_OFFSET]); + PartitionDescriptor = (PPARTITION_DESCRIPTOR)&Buffer[PARTITION_TABLE_OFFSET]; /* Make sure it's what the caller wanted */ if (PartitionDescriptor->PartitionType != MbrTypeIdentifier) @@ -1831,7 +1830,7 @@ xHalIoReadPartitionTable(IN PDEVICE_OBJECT DeviceObject, DiskGeometryEx.DiskSize, MaxSector); /* Allocate our buffer */ - Buffer = ExAllocatePoolWithTag(NonPagedPool, InputSize, TAG_FILE_SYSTEM); + Buffer = ExAllocatePoolWithTag(NonPagedPoolCacheAligned, InputSize, TAG_FILE_SYSTEM); if (!Buffer) { /* Fail, free the input buffer */ @@ -1901,12 +1900,11 @@ xHalIoReadPartitionTable(IN PDEVICE_OBJECT DeviceObject, if (!Offset.QuadPart) { /* Then read the signature off the disk */ - (*PartitionBuffer)->Signature = ((PULONG)Buffer)[PARTITION_TABLE_OFFSET / 2 - 1]; + (*PartitionBuffer)->Signature = *(PUINT32)&Buffer[DISK_SIGNATURE_OFFSET]; } /* Get the partition descriptor array */ - PartitionDescriptor = (PPARTITION_DESCRIPTOR) - &(((PUSHORT)Buffer)[PARTITION_TABLE_OFFSET]); + PartitionDescriptor = (PPARTITION_DESCRIPTOR)&Buffer[PARTITION_TABLE_OFFSET]; /* Start looping partitions */ j++; @@ -2091,8 +2089,7 @@ xHalIoReadPartitionTable(IN PDEVICE_OBJECT DeviceObject, Offset.QuadPart = 0; /* Go back to the descriptor array and loop it */ - PartitionDescriptor = (PPARTITION_DESCRIPTOR) - &(((PUSHORT)Buffer)[PARTITION_TABLE_OFFSET]); + PartitionDescriptor = (PPARTITION_DESCRIPTOR)&Buffer[PARTITION_TABLE_OFFSET]; for (Entry = 1; Entry <= NUM_PARTITION_TABLE_ENTRIES; Entry++, PartitionDescriptor++) { /* Check if this is a container partition, since we skipped them */ @@ -2237,7 +2234,7 @@ xHalIoSetPartitionInformation(IN PDEVICE_OBJECT DeviceObject, } /* Allocate our partition buffer */ - Buffer = ExAllocatePoolWithTag(NonPagedPool, PAGE_SIZE, TAG_FILE_SYSTEM); + Buffer = ExAllocatePoolWithTag(NonPagedPoolCacheAligned, PAGE_SIZE, TAG_FILE_SYSTEM); if (!Buffer) return STATUS_INSUFFICIENT_RESOURCES; /* Initialize the event we'll use and loop partitions */ @@ -2290,8 +2287,7 @@ xHalIoSetPartitionInformation(IN PDEVICE_OBJECT DeviceObject, } /* Get the partition descriptors and loop them */ - PartitionDescriptor = (PPARTITION_DESCRIPTOR) - &(((PUSHORT)Buffer)[PARTITION_TABLE_OFFSET]); + PartitionDescriptor = (PPARTITION_DESCRIPTOR)&Buffer[PARTITION_TABLE_OFFSET]; for (Entry = 1; Entry <= NUM_PARTITION_TABLE_ENTRIES; Entry++, PartitionDescriptor++) { /* Check if it's unused or a container partition */ @@ -2352,8 +2348,7 @@ xHalIoSetPartitionInformation(IN PDEVICE_OBJECT DeviceObject, if (Entry <= NUM_PARTITION_TABLE_ENTRIES) break; /* Nothing found yet, get the partition array again */ - PartitionDescriptor = (PPARTITION_DESCRIPTOR) - &(((PUSHORT)Buffer)[PARTITION_TABLE_OFFSET]); + PartitionDescriptor = (PPARTITION_DESCRIPTOR)&Buffer[PARTITION_TABLE_OFFSET]; for (Entry = 1; Entry <= NUM_PARTITION_TABLE_ENTRIES; Entry++, PartitionDescriptor++) { /* Check if this was a container partition (we skipped these) */ @@ -2469,7 +2464,7 @@ xHalIoWritePartitionTable(IN PDEVICE_OBJECT DeviceObject, DiskLayout->TableCount = (PartitionBuffer->PartitionCount + NUM_PARTITION_TABLE_ENTRIES - 1) / NUM_PARTITION_TABLE_ENTRIES; /* Allocate our partition buffer */ - Buffer = ExAllocatePoolWithTag(NonPagedPool, PAGE_SIZE, TAG_FILE_SYSTEM); + Buffer = ExAllocatePoolWithTag(NonPagedPoolCacheAligned, PAGE_SIZE, TAG_FILE_SYSTEM); if (!Buffer) return STATUS_INSUFFICIENT_RESOURCES; /* Loop the entries */ @@ -2520,7 +2515,7 @@ xHalIoWritePartitionTable(IN PDEVICE_OBJECT DeviceObject, if (!IsSuperFloppy) { /* Set the boot record signature */ - Buffer[BOOT_SIGNATURE_OFFSET] = BOOT_RECORD_SIGNATURE; + ((PUSHORT)Buffer)[BOOT_SIGNATURE_OFFSET] = BOOT_RECORD_SIGNATURE; /* By default, don't require a rewrite */ DoRewrite = FALSE; @@ -2529,12 +2524,10 @@ xHalIoWritePartitionTable(IN PDEVICE_OBJECT DeviceObject, if (!Offset.QuadPart) { /* Check if the signature doesn't match */ - if (((PULONG)Buffer)[PARTITION_TABLE_OFFSET / 2 - 1] != - PartitionBuffer->Signature) + if (*(PUINT32)&Buffer[PARTITION_TABLE_OFFSET] != PartitionBuffer->Signature) { /* Then write the signature and now we need a rewrite */ - ((PULONG)Buffer)[PARTITION_TABLE_OFFSET / 2 - 1] = - PartitionBuffer->Signature; + *(PUINT32)&Buffer[PARTITION_TABLE_OFFSET] = PartitionBuffer->Signature; DoRewrite = TRUE; } } diff --git a/ntoskrnl/fstub/fstubex.c b/ntoskrnl/fstub/fstubex.c index c1ef4cdd47c..5e3a819ae7c 100644 --- a/ntoskrnl/fstub/fstubex.c +++ b/ntoskrnl/fstub/fstubex.c @@ -674,7 +674,7 @@ FstubDetectPartitionStyle(IN PDISK_INFORMATION Disk, /* Get the partition descriptor array */ PartitionDescriptor = (PPARTITION_DESCRIPTOR) - &(Disk->Buffer[PARTITION_TABLE_OFFSET]); + &(Disk->Buffer[PARTITION_TABLE_OFFSET / sizeof(Disk->Buffer[0])]); /* If we have not the 0xAA55 then it's raw partition */ if (Disk->Buffer[BOOT_SIGNATURE_OFFSET] != BOOT_RECORD_SIGNATURE) { @@ -2140,7 +2140,7 @@ IoReadDiskSignature(IN PDEVICE_OBJECT DeviceObject, IN ULONG BytesPerSector, OUT PDISK_SIGNATURE Signature) { - PULONG Buffer; + PUCHAR Buffer; NTSTATUS Status; ULONG HeaderCRC32, i, CheckSum; PEFI_PARTITION_HEADER EFIHeader; @@ -2207,7 +2207,7 @@ IoReadDiskSignature(IN PDEVICE_OBJECT DeviceObject, /* Then zero the one in EFI header. This is needed to compute header checksum */ EFIHeader->HeaderCRC32 = 0; /* Compute header checksum and compare with the one present in partition table */ - if (RtlComputeCrc32(0, (PUCHAR)Buffer, sizeof(EFI_PARTITION_HEADER)) != HeaderCRC32) + if (RtlComputeCrc32(0, Buffer, sizeof(EFI_PARTITION_HEADER)) != HeaderCRC32) { Status = STATUS_DISK_CORRUPT_ERROR; goto Cleanup; @@ -2220,14 +2220,14 @@ IoReadDiskSignature(IN PDEVICE_OBJECT DeviceObject, else { /* Compute MBR checksum */ - for (i = 0, CheckSum = 0; i < 512 / sizeof(ULONG) ; i++) + for (i = 0, CheckSum = 0; i < BytesPerSector / sizeof(UINT32); i++) { - CheckSum += Buffer[i]; + CheckSum += *(PUINT32)&Buffer[i]; } /* Set partition table style to MBR and return signature (offset 440) and checksum */ Signature->PartitionStyle = PARTITION_STYLE_MBR; - Signature->Mbr.Signature = Buffer[PARTITION_TABLE_OFFSET / 2 - 1]; + Signature->Mbr.Signature = *(PUINT32)&Buffer[DISK_SIGNATURE_OFFSET]; Signature->Mbr.CheckSum = CheckSum; } diff --git a/ntoskrnl/include/internal/hal.h b/ntoskrnl/include/internal/hal.h index 1e61d138eac..b02b7575a60 100644 --- a/ntoskrnl/include/internal/hal.h +++ b/ntoskrnl/include/internal/hal.h @@ -240,9 +240,10 @@ xKdUnmapVirtualAddress( // // Various offsets in the boot record // -#define PARTITION_TABLE_OFFSET (0x1BE / 2) -#define BOOT_SIGNATURE_OFFSET ((0x200 / 2) - 1) -#define BOOT_RECORD_RESERVED 0x1BC +#define DISK_SIGNATURE_OFFSET 0x1B8 +#define PARTITION_TABLE_OFFSET 0x1BE +#define BOOT_SIGNATURE_OFFSET ((0x200 / sizeof(INT16)) - 1) + #define BOOT_RECORD_SIGNATURE 0xAA55 #define NUM_PARTITION_TABLE_ENTRIES 4
