grub-devel
[Top][All Lists]
Advanced

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

[PATCH v9 0/6] Cryptodisk fixes for v2.06 redux


From: Glenn Washburn
Subject: [PATCH v9 0/6] Cryptodisk fixes for v2.06 redux
Date: Tue, 15 Dec 2020 17:31:05 -0600

This patch series has been rebased onto latest master. The first patch is new
and fixes issue where total_sectors was incorrectly converted from source
disk sized sectors, when it should've been converted from grub native sector
size. The second patch was separated from the previous patch 14. Patch 3 is
most of the previous patch 14 with suggestions incorporated and some minor
debug message changes.

Glenn

Glenn Washburn (6):
  luks2: Convert to crypt sectors from grub native sectors
  luks2: Do not handle disks of size GRUB_DISK_SIZE_UNKNOWN for now
  luks2: Better error handling when setting up the cryptodisk
  mips: Enable __clzdi2()
  misc: Add grub_log2ull macro for calculating log base 2 of 64-bit
    integers
  luks2: Use grub_log2ull to calculate log_sector_size and improve
    readability

 grub-core/disk/luks2.c       | 93 ++++++++++++++++++++++++++++++++++--
 grub-core/kern/compiler-rt.c |  2 +-
 include/grub/compiler-rt.h   |  2 +-
 include/grub/disk.h          | 19 ++++++++
 include/grub/misc.h          |  3 ++
 5 files changed, 112 insertions(+), 7 deletions(-)

Range-diff against v8:
-:  --------- > 1:  2254f177d luks2: Convert to crypt sectors from grub native 
sectors
-:  --------- > 2:  1b52233d1 luks2: Do not handle disks of size 
GRUB_DISK_SIZE_UNKNOWN for now
1:  c32b542f2 ! 3:  ceaf95eb1 luks2: Better error handling when setting up the 
cryptodisk
    @@ Metadata
      ## Commit message ##
         luks2: Better error handling when setting up the cryptodisk
     
    -    First, check to make sure that source disk has a known size. If not, 
print
    -    debug message and return error. There are 4 cases where
    -    GRUB_DISK_SIZE_UNKNOWN is set (biosdisk, obdisk, ofdisk, and uboot), 
and in
    -    all those cases processing continues. So this is probably a bit
    -    conservative. However, 3 of the cases seem pathological, and the other,
    -    biosdisk, happens when booting from a cd. Since I doubt booting from a 
LUKS2
    -    volume on a cd is a big use case, we'll error until someone complains.
    -
         Do some sanity checking on data coming from the luks header. If 
segment.size
         is "dynamic", verify that the offset is not past the end of disk. 
Otherwise,
         check for errors from grub_strtoull when converting segment size from
    @@ Commit message
     
      ## grub-core/disk/luks2.c ##
     @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source,
    -       goto err;
    -     }
    - 
    -+  if (source->total_sectors == GRUB_DISK_SIZE_UNKNOWN)
    -+    {
    -+      /* FIXME: Allow use of source disk, and maybe cause errors in read. 
*/
    -+      grub_dprintf ("luks2", "Source disk %s has an unknown size, "
    -+                       "conservatively returning error\n", source->name);
    -+      ret = grub_error (GRUB_ERR_BUG, "Unknown size of luks2 source 
device");
    -+      goto err;
    -+    }
    -+
        /* Try all keyslot */
        for (json_idx = 0; json_idx < size; json_idx++)
          {
    @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source,
            crypt->offset_sectors = grub_divmod64 (segment.offset, 
segment.sector_size, NULL);
            crypt->log_sector_size = sizeof (unsigned int) * 8
                - __builtin_clz ((unsigned int) segment.sector_size) - 1;
    -+      /* Set to the source disk size, which is the maximum we allow. */
    -+      max_crypt_sectors = grub_disk_convert_sector(source,
    -+                                             source->total_sectors,
    -+                                             crypt->log_sector_size);
    ++      /* Set to the source disk/partition size, which is the maximum we 
allow. */
    ++      max_crypt_sectors = grub_disk_native_sectors(source);
    ++      max_crypt_sectors = grub_convert_sector(max_crypt_sectors,
    ++                                        GRUB_DISK_SECTOR_BITS,
    ++                                        crypt->log_sector_size);
     +
     +      if (max_crypt_sectors < crypt->offset_sectors)
     +  {
    @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source,
     +  }
     +
            if (grub_strcmp (segment.size, "dynamic") == 0)
    --  crypt->total_sectors = (grub_disk_native_sectors (source) >> 
(crypt->log_sector_size - source->log_sector_size))
    +-  crypt->total_sectors = (grub_disk_native_sectors (source) >> 
(crypt->log_sector_size - GRUB_DISK_SECTOR_BITS))
     -                         - crypt->offset_sectors;
     +  crypt->total_sectors = max_crypt_sectors - crypt->offset_sectors;
            else
    @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source,
     +    grub_errno = GRUB_ERR_NONE;
     +    /* Convert segment.size to sectors, rounding up to nearest sector */
     +    crypt->total_sectors = grub_strtoull (segment.size, NULL, 10);
    -+    crypt->total_sectors = ALIGN_UP (crypt->total_sectors,
    -+                                     1 << crypt->log_sector_size);
    -+    crypt->total_sectors >>= crypt->log_sector_size;
     +
     +    if (grub_errno == GRUB_ERR_NONE)
    -+      ;
    ++      {
    ++        crypt->total_sectors = ALIGN_UP (crypt->total_sectors,
    ++                                         1 << crypt->log_sector_size);
    ++        crypt->total_sectors >>= crypt->log_sector_size;
    ++      }
     +    else if (grub_errno == GRUB_ERR_BAD_NUMBER)
     +      {
     +        grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" size"
    -+                               " \"%s\" is not a parsable number\n",
    ++                               " \"%s\" is not a parsable number,"
    ++                               " skipping keyslot\n",
     +                               segment.idx, segment.size);
     +        continue;
     +      }
    @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source,
     +         * There was an overflow in parsing segment.size, so disk must
     +         * be very large or the string is incorrect.
     +         */
    ++        /*
    ++         * TODO: Allow reading of at least up max_crypt_sectors. Really,
    ++         * its very unlikely one would be booting from such a large drive
    ++         * anyway. Use another smaller LUKS boot device.
    ++         */
     +        grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" size"
     +                               " %s overflowed 64-bit unsigned integer,"
    -+                               " the end of the crypto device will be"
    -+                               " inaccessible\n",
    ++                               " skipping keyslot\n",
     +                               segment.idx, segment.size);
    -+        if (crypt->total_sectors > max_crypt_sectors)
    -+          crypt->total_sectors = max_crypt_sectors;
    ++        continue;
     +      }
     +  }
     +
    @@ include/grub/disk.h: typedef struct grub_disk_memberlist 
*grub_disk_memberlist_t
      /* Return value of grub_disk_native_sectors() in case disk size is 
unknown. */
      #define GRUB_DISK_SIZE_UNKNOWN     0xffffffffffffffffULL
      
    -+/* Convert sector number from disk sized sectors to a log-size sized 
sector. */
    ++/* Convert sector number from one sector size to another. */
     +static inline grub_disk_addr_t
    -+grub_disk_convert_sector (grub_disk_t disk,
    -+                    grub_disk_addr_t sector,
    -+                    grub_size_t log_sector_size)
    ++grub_convert_sector (grub_disk_addr_t sector,
    ++               grub_size_t log_sector_size_from,
    ++               grub_size_t log_sector_size_to)
     +{
    -+  if (disk->log_sector_size < log_sector_size)
    ++  if (log_sector_size_from == log_sector_size_to)
    ++    return sector;
    ++  else if (log_sector_size_from < log_sector_size_to)
     +    {
    -+      sector = ALIGN_UP (sector, 1 << (log_sector_size / 
disk->log_sector_size));
    -+      return sector >> (log_sector_size - disk->log_sector_size);
    ++      sector = ALIGN_UP (sector, 1 << (log_sector_size_to - 
log_sector_size_from));
    ++      return sector >> (log_sector_size_to - log_sector_size_from);
     +    }
     +  else
    -+    return sector << (disk->log_sector_size - log_sector_size);
    ++    return sector << (log_sector_size_from - log_sector_size_to);
     +}
     +
      /* Convert to GRUB native disk sized sector from disk sized sector. */
2:  f53c505fc = 4:  62d4a6c2e mips: Enable __clzdi2()
3:  9ed86739f = 5:  02c702fbf misc: Add grub_log2ull macro for calculating log 
base 2 of 64-bit integers
4:  626377092 ! 6:  30dfc9ee5 luks2: Use grub_log2ull to calculate 
log_sector_size and improve readability
    @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source,
     -      crypt->log_sector_size = sizeof (unsigned int) * 8
     -          - __builtin_clz ((unsigned int) segment.sector_size) - 1;
     +      crypt->log_sector_size = grub_log2ull (segment.sector_size);
    -       /* Set to the source disk size, which is the maximum we allow. */
    -       max_crypt_sectors = grub_disk_convert_sector(source,
    -                                              source->total_sectors,
    +       /* Set to the source disk/partition size, which is the maximum we 
allow. */
    +       max_crypt_sectors = grub_disk_native_sectors(source);
    +       max_crypt_sectors = grub_convert_sector(max_crypt_sectors,
-- 
2.27.0




reply via email to

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