Author: hbelusca
Date: Tue May 9 15:31:53 2017
New Revision: 74512
URL: http://svn.reactos.org/svn/reactos?rev=74512&view=rev
Log:
[USETUP]: Improve the bootsector validity check performed in IsThereAValidBootSector:
- Check for the first 3 bytes (and not 4) of the bootsector to not be zero (that's our criterium for a "valid instruction"). Therefore, a bootsector starting with "00 00 00 xx" (with xx the first byte of a volume identifier) is detected as invalid (because the BIOS won't be able to run it anyways) and therefore, needs to be overwritten.
- Check that its last 2 bytes are the valid 0xAA55 signature.
These improvements were suggested by Serge Gautherie and Peter Hater.
CORE-4870 CORE-12672 CORE-13188
- Move a DPRINT1 around.
Modified:
trunk/reactos/base/setup/usetup/bootsup.c
trunk/reactos/base/setup/usetup/interface/usetup.c
Modified: trunk/reactos/base/setup/usetup/bootsup.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/setup/usetup/bootsup.…
==============================================================================
--- trunk/reactos/base/setup/usetup/bootsup.c [iso-8859-1] (original)
+++ trunk/reactos/base/setup/usetup/bootsup.c [iso-8859-1] Tue May 9 15:31:53 2017
@@ -630,12 +630,14 @@
IsThereAValidBootSector(PWSTR RootPath)
{
/*
- * Check the first DWORD (4 bytes) of the bootsector for a potential
- * "valid" instruction (the BIOS starts execution of the bootsector
- * at its beginning). Currently the criterium is that this DWORD must
- * be non-zero.
+ * We first demand that the bootsector has a valid signature at its end.
+ * We then check the first 3 bytes (as a ULONG) of the bootsector for a
+ * potential "valid" instruction (the BIOS starts execution of the bootsector
+ * at its beginning). Currently this criterium is that this ULONG must be
+ * non-zero. If both these tests pass, then the bootsector is valid; otherwise
+ * it is invalid and certainly needs to be overwritten.
*/
-
+ BOOLEAN IsValid = FALSE;
NTSTATUS Status;
UNICODE_STRING Name;
OBJECT_ATTRIBUTES ObjectAttributes;
@@ -666,10 +668,9 @@
0,
FILE_SYNCHRONOUS_IO_NONALERT);
if (!NT_SUCCESS(Status))
- {
- RtlFreeHeap(ProcessHeap, 0, BootSector);
- return FALSE; // Status;
- }
+ goto Quit;
+
+ RtlZeroMemory(BootSector, SECTORSIZE);
FileOffset.QuadPart = 0ULL;
Status = NtReadFile(FileHandle,
@@ -682,16 +683,20 @@
&FileOffset,
NULL);
NtClose(FileHandle);
-
- Instruction = *(PULONG)BootSector;
-
+ if (!NT_SUCCESS(Status))
+ goto Quit;
+
+ /* Check the instruction; we use a ULONG to read three bytes */
+ Instruction = (*(PULONG)BootSector) & 0x00FFFFFF;
+ IsValid = (Instruction != 0x00000000);
+
+ /* Check the bootsector signature */
+ IsValid &= (*(PUSHORT)(BootSector + 0x1fe) == 0xaa55);
+
+Quit:
/* Free the boot sector */
RtlFreeHeap(ProcessHeap, 0, BootSector);
-
- if (!NT_SUCCESS(Status))
- return FALSE; // Status;
-
- return (Instruction != 0x00000000);
+ return IsValid; // Status;
}
NTSTATUS
Modified: trunk/reactos/base/setup/usetup/interface/usetup.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/setup/usetup/interfac…
==============================================================================
--- trunk/reactos/base/setup/usetup/interface/usetup.c [iso-8859-1] (original)
+++ trunk/reactos/base/setup/usetup/interface/usetup.c [iso-8859-1] Tue May 9 15:31:53 2017
@@ -4442,9 +4442,6 @@
wcscpy(SourceMbrPathBuffer, SourceRootPath.Buffer);
wcscat(SourceMbrPathBuffer, L"\\loader\\dosmbr.bin");
- DPRINT1("Install MBR bootcode: %S ==> %S\n",
- SourceMbrPathBuffer, DestinationDevicePathBuffer);
-
if (IsThereAValidBootSector(DestinationDevicePathBuffer))
{
/* Save current MBR */
@@ -4460,6 +4457,8 @@
}
}
+ DPRINT1("Install MBR bootcode: %S ==> %S\n",
+ SourceMbrPathBuffer, DestinationDevicePathBuffer);
Status = InstallMbrBootCodeToDisk(SourceMbrPathBuffer,
DestinationDevicePathBuffer);
if (!NT_SUCCESS(Status))
Author: hbelusca
Date: Mon May 8 21:34:07 2017
New Revision: 74509
URL: http://svn.reactos.org/svn/reactos?rev=74509&view=rev
Log:
[DISK]: Small fixes:
- Check for malformed disk identifier, which must be at least 9 WCHARs long (as done by disk_new);
- Prevent possible memory leaks (missing ExFreePool's) + closing registry key.
Investigated by Lesan Ilie during tests; as part of CORE-13131.
- Make one of our hacks more readable (by me).
(We also note that this driver uses the ExAllocate/FreePool functions in the old-school way).
Modified:
trunk/reactos/drivers/storage/class/disk/disk.c
Modified: trunk/reactos/drivers/storage/class/disk/disk.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/storage/class/disk…
==============================================================================
--- trunk/reactos/drivers/storage/class/disk/disk.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/storage/class/disk/disk.c [iso-8859-1] Mon May 8 21:34:07 2017
@@ -2286,7 +2286,7 @@
outputBuffer->PartitionType = diskData->PartitionType;
outputBuffer->StartingOffset = deviceExtension->StartingOffset;
outputBuffer->PartitionLength.QuadPart = (diskData->PartitionNumber) ?
- deviceExtension->PartitionLength.QuadPart : 2305843009213693951LL; // HACK
+ deviceExtension->PartitionLength.QuadPart : 0x1FFFFFFFFFFFFFFFLL; // HACK
outputBuffer->HiddenSectors = diskData->HiddenSectors;
outputBuffer->PartitionNumber = diskData->PartitionNumber;
outputBuffer->BootIndicator = diskData->BootIndicator;
@@ -3921,6 +3921,17 @@
ZwClose(targetKey);
if (!NT_SUCCESS(status)) {
+ ExFreePool(keyData);
+ continue;
+ }
+
+ if (keyData->DataLength < 9*sizeof(WCHAR)) {
+ //
+ // the data is too short to use (we subtract 9 chars in normal path)
+ //
+ DebugPrint((1, "EnumerateBusKey: Saved data was invalid, "
+ "not enough data in registry!\n"));
+ ExFreePool(keyData);
continue;
}
@@ -3943,6 +3954,7 @@
TRUE);
if (!NT_SUCCESS(status)) {
+ ExFreePool(keyData);
continue;
}
@@ -4124,6 +4136,7 @@
DebugPrint((1,
"SCSIDISK: ExtractBiosGeometry: Can't query configuration data (%x)\n",
status));
+ ZwClose(hardwareKey);
ExFreePool(keyData);
return;
}