7 Dec
2020
7 Dec
'20
11:51 a.m.
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=8d2fe54188616fa586946…
commit 8d2fe54188616fa5869468ecf108bf4af9a8624f
Author: Victor Perevertkin <victor.perevertkin(a)reactos.org>
AuthorDate: Mon Dec 7 14:43:34 2020 +0300
Commit: Victor Perevertkin <victor.perevertkin(a)reactos.org>
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