Uploaded image for project: 'Core ReactOS'
  1. Core ReactOS
  2. CORE-4023

usetup does not install MBR code if partition table is present

    XMLWordPrintable

Details

    • Bug
    • Resolution: Fixed
    • Major
    • 0.3.10
    • Setup
    • None
    • Operating System: ReactOS
      Platform: x86 Hardware

    Description

      The usetup installer does not fill in the MBR code if there is a partition table present but, but there is no code in the MBR. This can happen, for example, with a new hard disk that is partitioned with the Linux FDISK utility before starting usetup, or a new image file or logical volume partitioned the same way.

      The resulting symptom is that usetup will install ROS, and install the ROS bootloader code to the partition's boot sector, but the system will be unbootable because there will be no code in the MBR to start the bootloader from the active partition. I was able to fix this by compiling dosmbr.asm and copying the first 272 bytes to the MBR. I only copied 272 bytes because the rest (other than the signature) were NUL bytes in the resulting binary, and I already had a partition table and MBR signature.

      To fix this bug, usetup needs to check the first 440 bytes of the MBR. If these bytes are NUL (0x00), then the first 440 bytes of dosmbr.asm's compiled code need to be placed there instead; this lets dosmbr.asm's code size grow in the future, while not disturbing any pre-existing partition table which starts at offset 446 of the MBR; see the MBR article on Wikipedia for the layout and format of the MBR for additional detail.[1]

      Currently usetup appears to look at the entire 512 bytes of the MBR; this check fails if the code area is empty but there is a partition table and/or an MBR signature (say, a partitioning utility created partitions but did not install code to the MBR when initially partitioning a new, null-filled device); additionally it seems to install all of the 'dosmbr.bin' file, which of course is 512 bytes in length; instead it should only install the first 440 bytes, and if necessary, install the boot signature at offset 511. This installation process should be generic so that it works both for a brand-new device that has never been partitioned as well as a drive that has been partitioned but never had MBR code installed to it.

      === usetup: Check for new drive (near line 880 in partlist.c) ===

      if (LayoutBuffer->PartitionCount == 0)

      { DiskEntry->NewDisk = TRUE; }

      I can't patch this without a lot of learning (I know next to nothing about Win32 API so I can read it more or less, but not write it), but the check should change to essentially say:

      byte mbr[512];
      ReadDriveSector(/* drive / 0, / sector */ 0);

      int firstNonNullByteSeen = 0;
      int currentByte = 0;
      foreach(byte b in mbr) {
      if(b != 0)

      { firstNonNullByteSeen = currentByte++; break; }

      }

      if(firstNonNullByteSeen < 440)

      { DiskEntry->NewDisk = TRUE; }

      Then, when the check for newDisk == TRUE is done later near line 2517 in partlist.c, only the first 440 bytes of the MBR from dosmbr.bin should be installed to the MBR, and the disk signature of 0xAA55 (byte[]

      { 0x55, 0xAA }

      , to be exact).

      Interestingly enough, the boot code for the CD-ROM correctly detects this situation; if the partition table is filled in but there is no code in the hard disk's MBR, the CD boots without issuing a prompt. When I filled the code area in with 'dd' and booted again, the CD-ROM asked me to press a key to boot it. I don't know if that looks at all 440 bytes of the code segment or just the first several. If 0x00 is an invalid opcode on x86/x86-64 systems (I assume it is, but I don't actually know anything of x86 assembly at the opcode level), then just looking to see if the first byte is 0x00 should satisfy the check and that would simplify the above code from looping over 440 bytes to just inspecting a single byte.

      [1] http://en.wikipedia.org/wiki/Master_boot_record

      Attachments

        Activity

          People

            fireball fireball
            mtrausch mtrausch
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: