[hbelusca] 74512: [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 criter...

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.c...
==============================================================================
--- 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/interface...
==============================================================================
--- 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))