Re: [PATCH v3] disk/cryptodisk: When cheatmounting, use the sector info of the cheat device

[Glenn Washburn]

On Tue, 14 Jun 2022 15:55:21 +0200
Fabian Vogt <fvogt@suse.de> wrote:

> When using grub-probe with cryptodisk, the mapped block device from the host
> is used directly instead of decrypting the source device in GRUB code.
> In that case, the sector size and count of the host device needs to be used.
> This is especially important when using luks2, which does not assign
> total_sectors and log_sector_size when scanning, but only later when the
> segments in the JSON area are evaluated. With an unset log_sector_size,
> grub_open_device complains.
> 
> This fixes grub-probe failing with
> "error: sector sizes of 1 bytes aren't supported yet."
> 
> Signed-off-by: Fabian Vogt <fvogt@suse.de>

Reviewed-by: Glenn Washburn <development@efficientek.com>

Glenn

> ---
> v2: Moved new code from grub_cryptodisk_cheat_mount to grub_cryptodisk_open,
>     which allowed to simplify the code a bit. Also improved error handling.
> v3: More liberal placement of variable declarations.
> 
>  grub-core/disk/cryptodisk.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index c80cf9907..b3c76ef57 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -709,16 +709,31 @@ grub_cryptodisk_open (const char *name, grub_disk_t 
> disk)
>    if (!dev)
>      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "No such device");
>  
> -  disk->log_sector_size = dev->log_sector_size;
> -
>  #ifdef GRUB_UTIL
>    if (dev->cheat)
>      {
> +      grub_uint64_t cheat_dev_size;
> +      unsigned int cheat_log_sector_size;
> +
>        if (!GRUB_UTIL_FD_IS_VALID (dev->cheat_fd))
>       dev->cheat_fd = grub_util_fd_open (dev->cheat, GRUB_UTIL_FD_O_RDONLY);
>        if (!GRUB_UTIL_FD_IS_VALID (dev->cheat_fd))
>       return grub_error (GRUB_ERR_IO, N_("cannot open `%s': %s"),
>                          dev->cheat, grub_util_fd_strerror ());
> +
> +      /* Use the sector size and count of the cheat device */
> +      cheat_dev_size = grub_util_get_fd_size (dev->cheat_fd, dev->cheat, 
> &cheat_log_sector_size);
> +      if (cheat_dev_size == -1)
> +        {
> +          const char *errmsg = grub_util_fd_strerror ();
> +          grub_util_fd_close (dev->cheat_fd);
> +          dev->cheat_fd = GRUB_UTIL_FD_INVALID;
> +          return grub_error (GRUB_ERR_IO, N_("failed to query size of device 
> `%s': %s"),
> +                             dev->cheat, errmsg);
> +        }
> +
> +      dev->log_sector_size = cheat_log_sector_size;
> +      dev->total_sectors = cheat_dev_size >> cheat_log_sector_size;
>      }
>  #endif
>  
> @@ -732,6 +747,7 @@ grub_cryptodisk_open (const char *name, grub_disk_t disk)
>      }
>  
>    disk->data = dev;
> +  disk->log_sector_size = dev->log_sector_size;
>    disk->total_sectors = dev->total_sectors;
>    disk->max_agglomerate = GRUB_DISK_MAX_MAX_AGGLOMERATE;
>    disk->id = dev->id;