Re: [PATCH v3 09/10] cryptodisk: Rename offset in grub_cryptodisk_t to offset_sectors.

[Patrick Steinhardt]

On Mon, Oct 19, 2020 at 06:09:57PM -0500, Glenn Washburn wrote:
> This makes it clear that the offset represents sectors, not bytes, in order
> to improve readability.
> 
> Signed-off-by: Glenn Washburn <development@efficientek.com>

Reviewed-by: Patrick Steinhardt <ps@pks.im>

Patrick

> ---
>  grub-core/disk/cryptodisk.c | 10 +++++-----
>  grub-core/disk/geli.c       |  2 +-
>  grub-core/disk/luks.c       |  4 ++--
>  grub-core/disk/luks2.c      |  4 ++--
>  include/grub/cryptodisk.h   |  7 ++++++-
>  5 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index f5d098f65..64a51165c 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -766,10 +766,10 @@ grub_cryptodisk_read (grub_disk_t disk, 
> grub_disk_addr_t sector,
>    grub_dprintf ("cryptodisk",
>               "Reading %" PRIuGRUB_SIZE " sectors from sector 0x%"
>               PRIxGRUB_UINT64_T " with offset of %" PRIuGRUB_UINT64_T "\n",
> -             size, sector, dev->offset);
> +             size, sector, dev->offset_sectors);
>  
>    err = grub_disk_read (dev->source_disk,
> -                     grub_disk_from_native_sector (disk, sector + 
> dev->offset),
> +                     grub_disk_from_native_sector (disk, sector + 
> dev->offset_sectors),
>                       0, size << disk->log_sector_size, buf);
>    if (err)
>      {
> @@ -815,7 +815,7 @@ grub_cryptodisk_write (grub_disk_t disk, grub_disk_addr_t 
> sector,
>    grub_dprintf ("cryptodisk",
>               "Writing %" PRIuGRUB_SIZE " sectors to sector 0x%"
>               PRIxGRUB_UINT64_T " with offset of %" PRIuGRUB_UINT64_T "\n",
> -             size, sector, dev->offset);
> +             size, sector, dev->offset_sectors);
>  
>    gcry_err = grub_cryptodisk_endecrypt (dev, (grub_uint8_t *) tmp,
>                                       size << disk->log_sector_size,
> @@ -827,7 +827,7 @@ grub_cryptodisk_write (grub_disk_t disk, grub_disk_addr_t 
> sector,
>      }
>  
>    /* Since ->write was called so disk.mod is loaded but be paranoid  */
> -  sector = sector + dev->offset;
> +  sector = sector + dev->offset_sectors;
>    if (grub_disk_write_weak)
>      err = grub_disk_write_weak (dev->source_disk,
>                               grub_disk_from_native_sector (disk, sector),
> @@ -1240,7 +1240,7 @@ luks_script_get (grub_size_t *sz)
>       ptr = grub_stpcpy (ptr, "luks_mount ");
>       ptr = grub_stpcpy (ptr, i->uuid);
>       *ptr++ = ' ';
> -     grub_snprintf (ptr, 21, "%" PRIuGRUB_UINT64_T " ", i->offset);
> +     grub_snprintf (ptr, 21, "%" PRIuGRUB_UINT64_T " ", i->offset_sectors);
>       while (*ptr)
>         ptr++;
>       for (iptr = i->cipher->cipher->name; *iptr; iptr++)
> diff --git a/grub-core/disk/geli.c b/grub-core/disk/geli.c
> index 4ec875821..0175ce4c4 100644
> --- a/grub-core/disk/geli.c
> +++ b/grub-core/disk/geli.c
> @@ -361,7 +361,7 @@ configure_ciphers (grub_disk_t disk, const char 
> *check_uuid,
>      }
>    newdev->cipher = cipher;
>    newdev->secondary_cipher = secondary_cipher;
> -  newdev->offset = 0;
> +  newdev->offset_sectors = 0;
>    newdev->source_disk = NULL;
>    newdev->benbi_log = 0;
>    if (grub_le_to_cpu16 (header.alg) == 0x16)
> diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
> index 545734342..88f2da493 100644
> --- a/grub-core/disk/luks.c
> +++ b/grub-core/disk/luks.c
> @@ -122,10 +122,10 @@ configure_ciphers (grub_disk_t disk, const char 
> *check_uuid,
>    newdev = grub_zalloc (sizeof (struct grub_cryptodisk));
>    if (!newdev)
>        return NULL;
> -  newdev->offset = grub_be_to_cpu32 (header.payloadOffset);
> +  newdev->offset_sectors = grub_be_to_cpu32 (header.payloadOffset);
>    newdev->source_disk = NULL;
>    newdev->log_sector_size = LUKS1_LOG_SECTOR_SIZE;
> -  newdev->total_sectors = grub_disk_get_size (disk) - newdev->offset;
> +  newdev->total_sectors = grub_disk_get_size (disk) - newdev->offset_sectors;
>    grub_memcpy (newdev->uuid, uuid, sizeof (uuid));
>    newdev->modname = "luks";
>  
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index bae54936c..cd573208a 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -612,12 +612,12 @@ luks2_recover_key (grub_disk_t disk,
>        grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_UINT64_T"\n", 
> keyslot.slot_key);
>  
>        /* Set up disk according to keyslot's segment. */
> -      crypt->offset = grub_divmod64 (segment.offset, segment.sector_size, 
> NULL);
> +      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;
>        if (grub_strcmp (segment.size, "dynamic") == 0)
>       crypt->total_sectors = (grub_disk_get_size (disk) >> 
> (crypt->log_sector_size - disk->log_sector_size))
> -                            - crypt->offset;
> +                            - crypt->offset_sectors;
>        else
>       crypt->total_sectors = grub_strtoull (segment.size, NULL, 10) >> 
> crypt->log_sector_size;
>  
> diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> index 9e10ae927..c8eef2204 100644
> --- a/include/grub/cryptodisk.h
> +++ b/include/grub/cryptodisk.h
> @@ -72,7 +72,12 @@ struct grub_cryptodisk
>    struct grub_cryptodisk **prev;
>  
>    char *source;
> -  grub_disk_addr_t offset;
> +
> +  /*
> +   * The number of sectors the start of the encrypted data is offset into the
> +   * underlying disk, where sectors are the size noted by log_sector_size.
> +   */
> +  grub_disk_addr_t offset_sectors;
>    /* Total number of encrypted sectors of size (1<<log_sector_size) */
>    grub_disk_addr_t total_sectors;
>    grub_disk_t source_disk;
> -- 
> 2.27.0
>

signature.asc
Description: PGP signature