grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v2 1/7] biosdisk: autodetect hardware sector size (opt-in)


From: Mihai Moldovan
Subject: [PATCH v2 1/7] biosdisk: autodetect hardware sector size (opt-in)
Date: Sun, 24 May 2020 14:25:11 +0200

Some buggy (usually older) system firmware always reports a sector size
of 512 bytes - the default one for a very long time.

Newer disk drives diverge from that with longer sector sizes (typically
4096 bytes) and sometimes provide an 512-bytes-emulation layer. If the
latter is not provided, grub will get into trouble.

With 4Kn drives and buggy/older firmware a combination of things can
happen:
  - the system firmware reads exactly 512 bytes (only) from a sector
    and places it into memory, leaving the rest unaddress- and readable
  - the system firmware reads 512 bytes of the requested sector and
    additionally length * 512 bytes and places it into memory
  - the system firmware *thinks* it reads 512 bytes of the requested
    sector (only), but actually fetches more (usually the native sector
    size) and places all of that into memory

>From these behaviors, the first one is at least safe, but very
frustrating, the second one is tolerable and the third one is downright
harmful because it's essentially leading to undetected buffer
overflows.

Therefore, we actually need to determine *two* sizes:
  - the read size (i.e., how much of a physical sector will be put into
    memory by the system firmware) and
  - the actual disk sector size.

Determining the read size is relatively easy and goes like that:
  - poison scratch memory area with a known pattern
  - tell the system firmware to read exactly one sector
  - scan the scratch memory area for the first occurrence of the
    poisoning pattern.

The read size hence is the difference between the first poisoning
pattern occurrence and the start of the scratch memory area.

However, determining the physical sector size is a lot trickier and
requires a pretty nasty heuristic:
  - read one read size block from the *second* physical hardware sector
    into a buffer
  - read (MAX_HW_SECTOR_SIZE + read_size) / read_size blocks from the
    first physical hardware sector into a different buffer
  - scan the latter buffer until finding data from the former buffer.

Crucially, MAX_HW_SECTOR_SIZE must be small enough to not encompass the
whole usable scratch area (i.e., minus DAP in any case) and allow at
least one more read size block to be put into the buffer. That means
that the maximum supported hardware sector size is one order of
magnitude lower (pow-2-based) than the maximum supported read size.

Also, this heuristic is *dangerous* because it assumes and implies
that:
  - the first and second physical sectors on the drive have diverging
    content
  - the first physical sector does not contain the first read size
    block of the second physical sector.

This assumption usually holds for GPT-partitioned disks since GPT
starts at sector 2/LBA 1, but can easily fail for MBR-partitioned disks
with a standard post-MBR gap, especially if the disk has been wiped at
some point in time and nothing wrote to the post-MBR gap.

It will also trivially fail for completely blank disks.

This said, I see no other way to (more reliably) detect the physical
sector size.

If the sector size is detected correctly, almost everything should
work. There is one pretty big caveat: if the read block and sector
sizes do not match, then the last
pow (2, sector_bits - read_block_bits) sectors will not be fully
accessible, because:
  - the amount of sectors to be read must be given in as read size
    blocks
  - start sector + amount of (virtual, read-block-sized) sectors would
    overflow the total sectors count as returned by the drive
  - system firmware often checks this size constraints and errors out.

The point here is that the system firmware *might* check for this
overflow, but needs not.

So, for affected sectors, we will:
  - cancel any write requests since it's just too dangerous
  - try to opportunistically read.

If reading succeeded, we then need to make sure that the returned data
actually matches what we expect, i.e., identify truncated reads.

To do this, we employ the same scratch region poisoning scheme like we
do when determining the read block size.

Partial reads (and other errors) will be discarded.

This usually means not being able to access the last sectors of a disk,
but there's no way to work around that issue.

Disclaimer: this code is opt-in. It will not be triggered by default.
            Users must explicitly set the environment variable
            biosdisk_autodetect_sector_size to 1 (preferably) or any
            value different from zero.
---
 grub-core/disk/i386/pc/biosdisk.c | 626 +++++++++++++++++++++++++++++-
 include/grub/i386/pc/biosdisk.h   |   3 +
 2 files changed, 620 insertions(+), 9 deletions(-)

diff --git a/grub-core/disk/i386/pc/biosdisk.c 
b/grub-core/disk/i386/pc/biosdisk.c
index 8ca250c77..044bc48f3 100644
--- a/grub-core/disk/i386/pc/biosdisk.c
+++ b/grub-core/disk/i386/pc/biosdisk.c
@@ -28,11 +28,36 @@
 #include <grub/err.h>
 #include <grub/term.h>
 #include <grub/i18n.h>
+#include <grub/env.h>
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
 static int cd_drive = 0;
+/* Determined by fair dice roll. */
+static const grub_uint64_t grub_biosdisk_poison_pattern[64] =
+  {
+    0xF660FE98E09DF125, 0xD123C6FEDC70D7F8, 0x5B66A37CD24ADA47, 
0xB83E4A9DB0E12A04,
+    0x132DA014C28284AB, 0xF5C68B442B2AA5BD, 0xE21F60B0BFEFC1CF, 
0x5F976D7F7F5BD4DC,
+    0xFCB4F76ACD915CE8, 0x5C6E6DE8437CE13B, 0xF790D7BA4F629C2D, 
0x4EAB723F8B90F387,
+    0x6270892631C70107, 0xB4332B5DFB0A3E6D, 0x09FD5C6AA025EAB0, 
0xF39DCF68620BF1BE,
+    0x3B966D9C50A3A407, 0x3DA77872C8288688, 0xFBDA19DFB95940DD, 
0x4A136CFE2B3DD4A5,
+    0xF14B447C6CCB7EED, 0x92F2D74BD4CA99C0, 0x3CFF39AC8CE04880, 
0x14212C27A1106A50,
+    0x033EC0A7122519C8, 0xD0F7F9FA4A269434, 0x21A55CD807FF3F5E, 
0xE6A66A84650AD10B,
+    0x68E98E5DF15D9A6F, 0xD7DF78B2A3DEC128, 0xE364966B5EAAAEA7, 
0x96F19C0FF3121EEE,
+    0x78FD8D9EDDC9C358, 0xF9981071A4E0DA61, 0xCB5FB448E13EF785, 
0x872A0CD898AB4F17,
+    0x64574AF0E2CB75C4, 0xED72A3009C00746F, 0x43C9BBBA9283F3DB, 
0x3B5891BC9B0DE3F0,
+    0xD5F16F1BCDCD88B4, 0xED4D5C20229BB63E, 0x14ECB7F68803AF5E, 
0x8240D2555E729FA6,
+    0xECE77CA33E78F78D, 0x93772E94777226E0, 0xCC6C8BB2DDC8323B, 
0x5E13D0C413C5A7D3,
+    0x9799FD9136523F24, 0x02C942D7FFBD9BDC, 0xCEE49D0F6B7FFDB0, 
0x12C785E7DC01A725,
+    0x58E4F2D4353EDF6A, 0xB186F0BA1D8E9027, 0xD8C728D71BBCDD47, 
0xA187BE57A2E98EF6,
+    0x4D6568EC45CE249B, 0x5256CE99E4F812E2, 0x41CE1E1197D5987E, 
0x3A1076EFDE1D63EF,
+    0xE085586A0609FF7A, 0x58F5F23A289A1ABC, 0xF68B104B1D440019, 
0x13D1B4D7FDE31685
+  };
+
 static int grub_biosdisk_rw_int13_extensions (int ah, int drive, void *dap);
+static grub_err_t grub_biosdisk_read (grub_disk_t disk,
+                                     grub_disk_addr_t sector,
+                                     grub_size_t size, char *buf);
 
 static int grub_biosdisk_get_num_floppies (void)
 {
@@ -329,6 +354,93 @@ grub_biosdisk_iterate (grub_disk_dev_iterate_hook_t hook, 
void *hook_data,
   return 0;
 }
 
+static void
+grub_biosdisk_poison_scratch (const unsigned char dap_stop)
+{
+  /* Make sure that the poison pattern is a 512-bytes block. */
+  COMPILE_TIME_ASSERT (GRUB_DISK_SECTOR_SIZE == sizeof 
(grub_biosdisk_poison_pattern));
+
+  grub_size_t i = 0;
+  grub_size_t poison_pattern_size = sizeof (grub_biosdisk_poison_pattern);
+  grub_uint64_t *scratch_p = (grub_uint64_t *)
+                             GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
+
+  /* By default, poison the whole region. */
+  grub_size_t scratch_size = GRUB_MEMORY_MACHINE_SCRATCH_SIZE;
+
+  /*
+   * But if dap_stop has been provided, limit the poisoned area to everything
+   * before the DAP location.
+   */
+  if (dap_stop)
+    {
+      scratch_size = GRUB_BIOSDISK_DAP_OFFSET;
+    }
+
+  /* The size is actually the count of poisoning pattern blocks. */
+  scratch_size = grub_divmod64 (scratch_size, poison_pattern_size, 0);
+
+  /* Actually poison region. */
+  for (i = 0; i < scratch_size; ++i)
+    {
+      grub_memmove (scratch_p + (i * ARRAY_SIZE 
(grub_biosdisk_poison_pattern)),
+                   grub_biosdisk_poison_pattern, poison_pattern_size);
+    }
+}
+
+static grub_err_t
+grub_biosdisk_find_poison_pattern (const unsigned char dap_stop,
+                                  unsigned char *found,
+                                  grub_size_t *block_offset)
+{
+  grub_err_t ret = GRUB_ERR_BAD_ARGUMENT;
+
+  if ((!found) || (!block_offset))
+    {
+      return ret;
+    }
+
+  ret = GRUB_ERR_NONE;
+
+  /* Make sure that the poison pattern is a 512-bytes block. */
+  COMPILE_TIME_ASSERT (GRUB_DISK_SECTOR_SIZE == sizeof 
(grub_biosdisk_poison_pattern));
+
+  grub_size_t i = 0;
+  grub_size_t poison_pattern_size = sizeof (grub_biosdisk_poison_pattern);
+  grub_uint64_t *scratch_p = (grub_uint64_t *)
+                             GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
+
+  /* By default, scan the whole region. */
+  grub_size_t scratch_size = GRUB_MEMORY_MACHINE_SCRATCH_SIZE;
+
+  /*
+   * But if dap_stop has been provided, limit the scanned area to everything
+   * before the DAP location.
+   */
+  if (dap_stop)
+    {
+      scratch_size = GRUB_BIOSDISK_DAP_OFFSET;
+    }
+
+  /* The size is actually the count of poisoning pattern blocks. */
+  scratch_size = grub_divmod64 (scratch_size, poison_pattern_size, 0);
+
+  /* Actually scan the region. */
+  *found = 0;
+  for (i = 0; i < scratch_size; ++i)
+    {
+      if (0 == grub_memcmp (scratch_p + (i * ARRAY_SIZE 
(grub_biosdisk_poison_pattern)),
+                           grub_biosdisk_poison_pattern, poison_pattern_size))
+       {
+         *found = 1;
+         *block_offset = i;
+         break;
+       }
+    }
+
+  return ret;
+}
+
 static grub_err_t
 grub_biosdisk_open (const char *name, grub_disk_t disk)
 {
@@ -360,6 +472,27 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
     {
       /* HDD */
       int version;
+      unsigned int detected_log_read_size = 0;
+      unsigned int detected_read_size = 0;
+      unsigned int detected_sector_size = 0;
+      unsigned int detected_log_sector_size = 0;
+      grub_size_t i = 0;
+      grub_size_t max_reads = 0;
+      unsigned char found = 0;
+      int autodetect = 0;
+      const char *autodetect_str = grub_env_get 
("biosdisk_autodetect_sector_size");
+
+      char *buf_lhs = grub_malloc (GRUB_BIOSDISK_DAP_OFFSET); /* Max supported 
buffer size. */
+      if (!buf_lhs)
+       {
+         return grub_errno;
+       }
+
+      char *buf_rhs = grub_malloc (GRUB_BIOSDISK_DAP_OFFSET >> 1); /* Max 
supported sector size. */
+      if (!buf_rhs)
+       {
+         return grub_errno;
+       }
 
       disk->log_sector_size = 9;
 
@@ -395,6 +528,287 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
                }
            }
        }
+
+      if (autodetect_str)
+        {
+          grub_errno = 0;
+          autodetect = !!(grub_strtoul (autodetect_str, 0, 0));
+
+          if (grub_errno)
+            {
+              /*
+               * Env variable set, but not recognized as a number.
+               * Assume that the user intended to enable this feature.
+               */
+              autodetect = 1;
+            }
+        }
+
+      /* Initialize read block size to queried/defaulted sector size. */
+      data->log_read_size = disk->log_sector_size;
+
+      if (autodetect)
+        {
+         /*
+          * We now either have a sector size as returned by the firmware
+          * or hardcoded/initialized it to 512 bytes.
+          *
+          * Unfortunately, some old/buggy firmware versions always return
+          * a sector size of 512 bytes, regardless of the actual disk hardware
+          * sector size.
+          *
+          * Using a smaller sector size than actually supported by the
+          * hardware will lead to nasty memory corruptions, because:
+          *   - we have a hardcoded scratch memory region used for read and
+          *     write operations
+          *   - this memory region is bounded
+          *   - a DAP struct, containing read/write parameters for the
+          *     firmware, is put at an offset of 32 KByte within the scratch
+          *     region
+          *
+          * This will generally work fine if the sector size returned by the
+          * firmware matches the hardware sector size (and we make sure that
+          * (SAFE_SECTORS * SECTOR_SIZE) does not overflow the scratch memory
+          * region or reaches into the DAP) or the hardware sector size is
+          * smaller than the sector size reported by the firmware, but is very
+          * dangerous if the hardware sector size is bigger than the sector
+          * size returned by the firmware.
+          *
+          * Additionally, sometimes read and sector sizes are decoupled.
+          * For example, there is firmware that wrongfully reports a fixed
+          * sector size of 512 bytes AND writes at most 512 bytes to the
+          * buffer for a one-sector read. In such a case, the read length must
+          * be specified as a read-block multiple, while sector addressing is
+          * hardware-sector-size based (because the firmware just passes the
+          * start sector value right to the actual disk).
+          *
+          * To counter this, try to detect the actual read block size and
+          * hardware sector size by:
+          *   - poisoning the whole scratch memory area with a specific value
+          *   - set safe sector size to 1
+          *   - read a block off the disk drive (and make sure this succeeds)
+          *   - determine the read block size by scanning for the poisoning
+          *     value
+          *   - fill two buffers with reads of LBAs 0 and 1 (taking read
+          *     block size into account)
+          *   - determine the hardware sector size by searching for the first
+          *     matching block (hoping that both sectors contain different
+          *     data)
+          */
+
+         /* Poison region. */
+         grub_biosdisk_poison_scratch (0);
+
+         /*
+          * Set safe sector size to one sector for upcoming read operations.
+          */
+         data->sectors = 1;
+
+         /* Set disk->data temporarily to our data object. */
+         disk->data = data;
+
+         /* Read one block. In case of failure, error out. */
+         found = 0;
+         if (GRUB_ERR_NONE == grub_biosdisk_read (disk, 0, 1, buf_rhs))
+           {
+             /* Compare the data with our poison pattern. */
+             if (grub_biosdisk_find_poison_pattern (1, &found, &i))
+               {
+                 return grub_error (GRUB_ERR_BUG,
+                                    "%s: unable to search for poisoning 
pattern "
+                                    "in sector size autodetection - bad RAM?",
+                                    disk->name);
+               }
+           }
+         else
+           {
+             disk->data = NULL;
+             grub_free (buf_lhs);
+             grub_free (buf_rhs);
+             grub_free (data);
+             return grub_error (GRUB_ERR_BAD_DEVICE,
+                                "%s: read failure while determining INT13H 
read size",
+                                disk->name);
+           }
+
+         detected_read_size = sizeof (grub_biosdisk_poison_pattern) * i;
+
+         if (!found || (detected_read_size < 512) || (detected_read_size > 
16384))
+           {
+             disk->data = NULL;
+             grub_free (buf_lhs);
+             grub_free (buf_rhs);
+             grub_free (data);
+             if (!found)
+               {
+                 return grub_error (GRUB_ERR_BAD_DEVICE, "%s: no INT13H read 
size "
+                                    "detected",
+                                    disk->name);
+               }
+             else
+               {
+                 return grub_error (GRUB_ERR_BAD_DEVICE, "%s: INT13H read size 
has "
+                                    "unsupported length %u",
+                                    disk->name, detected_read_size);
+               }
+           }
+
+         /* Convert detected read size to log2 value ... */
+         for (detected_log_read_size = 0;
+              (1U << detected_log_read_size) < detected_read_size;
+              ++detected_log_read_size);
+
+         /* ... and check that it's a sane power-of-two read block size. */
+         if ((1U << detected_log_read_size) == detected_read_size)
+           {
+             data->log_read_size = detected_log_read_size;
+           }
+         else
+           {
+             grub_free (buf_lhs);
+             grub_free (buf_rhs);
+             grub_free (data);
+             return grub_error (GRUB_ERR_BAD_DEVICE,
+                                "%s: unable to determine INT13H read size",
+                                disk->name);
+           }
+
+         /*
+          * Determining the hardware sector size is also tricky.
+          * Buggy firmware versions read less data than the hardware sector
+          * size (typically 512 bytes), so we will have to tell the firmware
+          * to read (MAX_HW_SECTOR_SIZE + read_size) / read_size blocks to
+          * fetch enough data to detect sector sizes up to the maximum
+          * supported.
+          *
+          * Hence:
+          *   - read read_size bytes from second HW sector into buf_rhs
+          *   - set safe sector count to
+          *     (MAX_HW_SECTOR_SIZE + read_size) / read_size and read the same
+          *     amount of blocks from first HW sector into buf_lhs
+          *   - scan for read_size-sized data in buf_rhs in buf_lhs
+          *
+          * Assuming that the actual hardware sector size is less than or
+          * equal to the maximum supported sector size, buf_lhs should always
+          * contain the first block of the second HW sector somewhere. The
+          * difference between buf_lhs' start and the second HW sector block
+          * must thus be the HW sector size.
+          *
+          * NOTE: this heuristic is inherently unsafe because it relies on
+          *       disk data. It should work fine with GPT-partitioned disks,
+          *       since the GPT always starts at the second HW sector, but can
+          *       otherwise return wrong results. For instance, the HW sector
+          *       size would be determined as 512 bytes for a zero-filled disk
+          *       with MBR data in the first 512 bytes of the first HW sector,
+          *       even though the actual HW sector size might differ.
+          */
+
+         /* Fill buffers with data from hard disk. */
+         i = grub_divmod64 (GRUB_BIOSDISK_DAP_OFFSET >> 1, detected_read_size, 
0);
+
+         /*
+          * Setting disk->log_sector_size to the read size will correctly fill
+          * the provided buffer without us needing to manually copy data from
+          * the memory scratch area.
+          */
+         disk->log_sector_size = detected_log_read_size;
+
+         if (GRUB_ERR_NONE != grub_biosdisk_read (disk, 1, 1, buf_rhs))
+           {
+             disk->data = NULL;
+             grub_free (buf_lhs);
+             grub_free (buf_rhs);
+             grub_free (data);
+             return grub_error (GRUB_ERR_BAD_DEVICE,
+                                "%s: read failure for sector 1 while "
+                                "determining sector size",
+                                disk->name);
+           }
+
+         /* Set data->sectors to (MAX_HW_SECTOR_SIZE + read_size) / read_size. 
*/
+         data->sectors = i + 1;
+
+         if (GRUB_ERR_NONE != grub_biosdisk_read (disk, 0, i + 1, buf_lhs))
+           {
+             disk->data = NULL;
+             grub_free (buf_lhs);
+             grub_free (buf_rhs);
+             grub_free (data);
+             return grub_error (GRUB_ERR_BAD_DEVICE,
+                                "%s: read failure for sector 0 while "
+                                "determining sector size",
+                                disk->name);
+           }
+
+         /*
+          * No more reads necessary, so reset disk->data.
+          * Will be set correctly at a later time.
+          */
+         disk->data = NULL;
+
+         /*
+          * Try to find the first matching read block in both buf_lhs and
+          * buf_rhs.
+          * Assuming that both sectors contain different data, stumbling
+          * across the same block value means that we have crossed sector
+          * boundaries in the first buffer.
+          */
+         max_reads = (2 * i);
+         found = 0;
+         for (i = 1; i < max_reads; ++i)
+           {
+             if (0 == grub_memcmp (buf_lhs + (i * detected_read_size), buf_rhs,
+                                   detected_read_size))
+               {
+                 found = 1;
+                 break;
+               }
+           }
+
+         detected_sector_size = i * detected_read_size;
+
+         /* Free buffers, we don't need them any longer. */
+         grub_free (buf_lhs);
+         grub_free (buf_rhs);
+
+         if (!found || (detected_sector_size < 512)
+             || (detected_sector_size > 16348)
+             || (detected_sector_size < detected_read_size))
+           {
+             grub_free (data);
+             if (!found)
+               {
+                 return grub_error (GRUB_ERR_BAD_DEVICE, "%s: no sector size "
+                                    "detected",
+                                    disk->name);
+               }
+             else
+               {
+                 return grub_error (GRUB_ERR_BAD_DEVICE, "%s: sector size has "
+                                    "unsupported length %u",
+                                    disk->name, detected_sector_size);
+               }
+           }
+
+         /* Convert detected sector size to log2 value ... */
+         for (detected_log_sector_size = 0;
+              (1U << detected_log_sector_size) < detected_sector_size;
+              ++detected_log_sector_size);
+
+         /* ... and check that it's a sane power-of-two sector size. */
+         if ((1U << detected_log_sector_size) == detected_sector_size)
+           {
+             disk->log_sector_size = detected_log_sector_size;
+           }
+         else
+           {
+             return grub_error (GRUB_ERR_BAD_DEVICE,
+                                "%s: unable to determine sector size, "
+                                "detected value %u is not a power-of-two "
+                                "value",
+                                disk->name, detected_sector_size);
+           }
+       }
     }
 
   if (! (data->flags & GRUB_BIOSDISK_FLAG_CDROM))
@@ -470,13 +884,64 @@ grub_biosdisk_rw (int cmd, grub_disk_t disk,
                       N_("attempt to read or write outside of disk `%s'"),
                       disk->name);
 
+  /*
+   * If the native sector size doesn't match the read-block size, we're working
+   * with old, buggy system firmware and newer drives.
+   *
+   * We want to handle operations on sectors at the end in a special way, so
+   * check if this operation would be affected.
+   */
+  unsigned char past_end = 0;
+  if ((disk->log_sector_size != data->log_read_size)
+      && ((sector + size) > disk->total_sectors))
+    {
+      past_end = 1;
+
+      if (cmd)
+       {
+         /*
+          * Write operations are especially scary.
+          *
+          * If the system firmware writes only some of the data (e.g., because
+          * it copies only part of the buffer), we'll end up with
+          * partially updated sectors.
+          *
+          * Needless to say, we should definitely avoid this.
+          */
+         return grub_error (GRUB_ERR_WRITE_ERROR, N_ ("refusing to write 
partial "
+                                                      "data to disk sector 
%llu on `%s'"),
+                            (unsigned long long) sector,
+                            disk->name);
+       }
+      else
+       {
+         /*
+          * Read operations aren't much better.
+          *
+          * Reads could either succeed and return the correct amount of data
+          * (if the system firmware is lenient, doesn't check bounds, fetches
+          * data into a buffer big enough and hands over the data correctly
+          * sized), XOR suceed and return partial data, XOR error out.
+          *
+          * We will go ahead and try to read the data opportunistically,
+          * but will also have to differentiate partial read sectors from
+          * fully read sectors.
+          *
+          * To do this, we'll use about the same poisoning technique we're
+          * also using for determining the read block size in
+          * grub_biosdisk_open.
+          */
+         grub_biosdisk_poison_scratch (1);
+       }
+    }
+
   if (data->flags & GRUB_BIOSDISK_FLAG_LBA)
     {
       struct grub_biosdisk_dap *dap;
 
+      /* Put the DAP at a 32 KByte offset from scratch memory start. */
       dap = (struct grub_biosdisk_dap *) (GRUB_MEMORY_MACHINE_SCRATCH_ADDR
-                                         + (data->sectors
-                                            << disk->log_sector_size));
+                                         + GRUB_BIOSDISK_DAP_OFFSET);
       dap->length = sizeof (*dap);
       dap->reserved = 0;
       dap->blocks = size;
@@ -503,10 +968,101 @@ grub_biosdisk_rw (int cmd, grub_disk_t disk,
       else
         if (grub_biosdisk_rw_int13_extensions (cmd + 0x42, data->drive, dap))
          {
-           /* Fall back to the CHS mode.  */
+           if (past_end)
+             {
+               /*
+                * Reading "past the end" failed, which likely means that the
+                * system firmware checks the length and start sector against
+                * the total sectors.
+                *
+                * It doesn't make sense to retry.
+                */
+               return grub_error (GRUB_ERR_READ_ERROR, N_ ("opportunistically 
tried to read "
+                                                           "end sectors from 
%llu on `%s', "
+                                                           "but system 
firmware does not seem "
+                                                           "to support this"),
+                                  (unsigned long long) sector,
+                                  disk->name);
+             }
+
+           /*
+            * If the operation failed and we didn't try to read the end
+            * sectors, fall back to the CHS mode ...
+            */
+           grub_err_t chs_read = GRUB_ERR_NONE;
            data->flags &= ~GRUB_BIOSDISK_FLAG_LBA;
            disk->total_sectors = data->cylinders * data->heads * data->sectors;
-           return grub_biosdisk_rw (cmd, disk, sector, size, segment);
+
+           chs_read = grub_biosdisk_rw (cmd, disk, sector, size, segment);
+
+           /* Pass CHS operation result through. */
+           return chs_read;
+         }
+       else if (past_end)
+         {
+           /*
+            * An opportunistic past-the-end read operation completed
+            * successfully, which means that the system firmware didn't check
+            * for an out-of-device-bounds read.
+            *
+            * In turn, we'll have to check if the read data is actually
+            * complete.
+            */
+           grub_size_t real_size = size >> (disk->log_sector_size - 
data->log_read_size);
+           grub_size_t i = 0;
+           unsigned char found = 0;
+           if (grub_biosdisk_find_poison_pattern (1, &found, &i))
+             {
+               return grub_error (GRUB_ERR_BUG,
+                                  N_ ("%s: unable to search for poisoning 
pattern "
+                                      "in past-the-end read operation - bad 
RAM?"),
+                                  disk->name);
+             }
+
+           if (found)
+             {
+               /* Found pattern at a later position than expected. */
+               if (i > real_size)
+                 {
+                   return grub_error (GRUB_ERR_BUG,
+                                      N_ ("%s: poisoning pattern found at 
block offset %llu, "
+                                          "but expected at (at most) %llu, 
this should not happen"),
+                                      disk->name, i, real_size);
+                 }
+
+               /* Read partial data. */
+               if (i < real_size)
+                 {
+                   return grub_error (GRUB_ERR_READ_ERROR, N_ 
("opportunistically tried to read "
+                                                               "end sectors 
from %llu on `%s', "
+                                                               "but system 
firmware does not seem "
+                                                               "to support 
this"),
+                                      (unsigned long long) sector,
+                                      disk->name);
+                 }
+             }
+           else
+             {
+               /* Read past the original DAP. */
+               if ((real_size << disk->log_sector_size) > 
GRUB_BIOSDISK_DAP_OFFSET)
+                 {
+                   return grub_error (GRUB_ERR_BUG,
+                                      N_ ("%s: attempted to read more data 
(%llu bytes) than would "
+                                          "fit into the buffer (%llu bytes), 
this should not happen"),
+                                      disk->name, real_size << 
disk->log_sector_size,
+                                      GRUB_BIOSDISK_DAP_OFFSET);
+                 }
+
+               /* System firmware wrote more data to buffer than expected?! */
+               if ((real_size << disk->log_sector_size) < 
GRUB_BIOSDISK_DAP_OFFSET)
+                 {
+                   return grub_error (GRUB_ERR_BUG,
+                                      N_ ("%s: unable to find poisoning 
pattern after returned "
+                                          "data, did the system firmware 
return more data than "
+                                          "expected?"),
+                                      disk->name);
+                 }
+             }
          }
     }
   else
@@ -563,7 +1119,24 @@ get_safe_sectors (grub_disk_t disk, grub_disk_addr_t 
sector)
   grub_size_t size;
   grub_uint64_t offset;
   struct grub_biosdisk_data *data = disk->data;
-  grub_uint32_t sectors = data->sectors;
+
+  /*
+   * Calculate usable area, but leave a safety margin of one sector.
+   * This can leave about half of the scratch region (currently) unused, but
+   * data safety is arguably more important than a few additional read
+   * operations.
+   */
+  grub_uint32_t sectors = (GRUB_BIOSDISK_DAP_OFFSET >> disk->log_sector_size)
+                         - 1;
+
+  /*
+   * For CHS access, we want to read at most one head at a time,
+   * so limit the safe sector count if necessary.
+   */
+  if (data->sectors < sectors)
+    {
+      sectors = data->sectors;
+    }
 
   /* OFFSET = SECTOR % SECTORS */
   grub_divmod64 (sector, sectors, &offset);
@@ -577,16 +1150,48 @@ static grub_err_t
 grub_biosdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
                    grub_size_t size, char *buf)
 {
+  struct grub_biosdisk_data *data = disk->data;
+
   while (size)
     {
-      grub_size_t len;
+      grub_size_t len, raw_len;
 
       len = get_safe_sectors (disk, sector);
 
       if (len > size)
        len = size;
 
-      if (grub_biosdisk_rw (GRUB_BIOSDISK_READ, disk, sector, len,
+      /*
+       * Usually, we want to pass the amount of sectors to read or write to
+       * grub_biosdisk_rw.
+       *
+       * Unfortunately, for (old) buggy BIOS versions, we will have to pass
+       * a read-block sized value to actually get the desired data.
+       *
+       * Look up the autodetection code and comments in grub_biosdisk_open for
+       * a description of this stuff.
+       *
+       * Now, there is one caveat to all of this: system firmware *thinks*
+       * that we're passing a sector-based length value to it and will
+       * check sector + length to make sure it stays below (or equals) the
+       * total sector value as returned by the hardware.
+       *
+       * This is *bad*.
+       * Why?
+       *
+       * For instance, if we want to read the last sector, we'll only be able
+       * to pass 1 as the length and hence will get only one read-sized block
+       * (so, e.g., only the first 512 bytes of the sector). All the other data
+       * will be inaccessible.
+       *
+       * For the second-to-last sector, it would be at most two read-sized
+       * blocks etc.
+       *
+       * But even worse, we can't do anything to work around that issue.
+       */
+      raw_len = len << (disk->log_sector_size - data->log_read_size);
+
+      if (grub_biosdisk_rw (GRUB_BIOSDISK_READ, disk, sector, raw_len,
                            GRUB_MEMORY_MACHINE_SCRATCH_SEG))
        return grub_errno;
 
@@ -612,16 +1217,19 @@ grub_biosdisk_write (grub_disk_t disk, grub_disk_addr_t 
sector,
 
   while (size)
     {
-      grub_size_t len;
+      grub_size_t len, raw_len;
 
       len = get_safe_sectors (disk, sector);
       if (len > size)
        len = size;
 
+      /* Please check grub_biosdisk_read for an explanation of this magic. */
+      raw_len = len << (disk->log_sector_size - data->log_read_size);
+
       grub_memcpy ((void *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR, buf,
                   len << disk->log_sector_size);
 
-      if (grub_biosdisk_rw (GRUB_BIOSDISK_WRITE, disk, sector, len,
+      if (grub_biosdisk_rw (GRUB_BIOSDISK_WRITE, disk, sector, raw_len,
                            GRUB_MEMORY_MACHINE_SCRATCH_SEG))
        return grub_errno;
 
diff --git a/include/grub/i386/pc/biosdisk.h b/include/grub/i386/pc/biosdisk.h
index 3d8071684..ca01cb7ae 100644
--- a/include/grub/i386/pc/biosdisk.h
+++ b/include/grub/i386/pc/biosdisk.h
@@ -33,6 +33,8 @@
 
 #define GRUB_BIOSDISK_CDTYPE_MASK      0xF
 
+#define GRUB_BIOSDISK_DAP_OFFSET       (1 << 15)
+
 struct grub_biosdisk_data
 {
   int drive;
@@ -40,6 +42,7 @@ struct grub_biosdisk_data
   unsigned long heads;
   unsigned long sectors;
   unsigned long flags;
+  unsigned int  log_read_size;
 };
 
 /* Drive Parameters.  */
-- 
2.25.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]